From dee8809e3091a9e06f79fcc47f9a38c4183894e5 Mon Sep 17 00:00:00 2001 From: "Eric S. Raymond" Date: Thu, 14 Apr 2022 13:13:46 -0400 Subject: [PATCH] pylint cleanup. --- Makefile | 6 ++- make_dungeon.py | 88 ++++++++++++++++++++------------------- make_graph.py | 12 +++--- tests/coverage_dungeon.py | 61 ++++++++++++++------------- 4 files changed, 87 insertions(+), 80 deletions(-) diff --git a/Makefile b/Makefile index f8599ea..84039b4 100644 --- a/Makefile +++ b/Makefile @@ -136,6 +136,10 @@ debug: CCFLAGS += -fsanitize=address debug: CCFLAGS += -fsanitize=undefined debug: linty -CSUPPRESSIONS = --suppress=missingIncludeSystem --suppress=invalidscanf +PYSUPPRESSIONS = --suppress=missingIncludeSystem --suppress=invalidscanf cppcheck: cppcheck -I. --template gcc --enable=all $(CSUPPRESSIONS) *.[ch] + +PYSUPPRESSIONS = line-too-long,invalid-name,missing-function-docstring,too-many-lines,too-many-branches,global-statement,multiple-imports,too-many-locals,too-many-statements,too-many-nested-blocks,no-else-return,raise-missing-from,redefined-outer-name,consider-using-in +pylint: + @pylint --score=n --disable=$(PYSUPPRESSIONS) *.py */*.py diff --git a/make_dungeon.py b/make_dungeon.py index 724b941..f90ad32 100755 --- a/make_dungeon.py +++ b/make_dungeon.py @@ -1,15 +1,15 @@ #!/usr/bin/env python3 +""" +This is the open-adventure dungeon generator. It consumes a YAML description of +the dungeon and outputs a dungeon.h and dungeon.c pair of C code files. -# This is the open-adventure dungeon generator. It consumes a YAML description of -# the dungeon and outputs a dungeon.h and dungeon.c pair of C code files. -# -# The nontrivial part of this is the compilation of the YAML for -# movement rules to the travel array that's actually used by -# playermove(). -# -# Copyright (c) 2017 by Eric S. Raymond -# SPDX-License-Identifier: BSD-2-clause +The nontrivial part of this is the compilation of the YAML for +movement rules to the travel array that's actually used by +playermove(). +Copyright (c) 2017 by Eric S. Raymond +SPDX-License-Identifier: BSD-2-clause +""" import sys, yaml YAML_NAME = "adventure.yaml" @@ -24,7 +24,7 @@ statedefines = "" def make_c_string(string): """Render a Python string into C string literal format.""" - if string == None: + if string is None: return "NULL" string = string.replace("\n", "\\n") string = string.replace("\t", "\\t") @@ -143,7 +143,7 @@ def get_objects(obj): words_str = get_string_group([]) i_msg = make_c_string(attr["inventory"]) descriptions_str = "" - if attr["descriptions"] == None: + if attr["descriptions"] is None: descriptions_str = " " * 12 + "NULL," else: labels = [] @@ -159,30 +159,30 @@ def get_objects(obj): statedefines += "#define %s\t%d\n" % (label, n) statedefines += "\n" sounds_str = "" - if attr.get("sounds") == None: + if attr.get("sounds") is None: sounds_str = " " * 12 + "NULL," else: - for l_msg in attr["sounds"]: - sounds_str += " " * 12 + make_c_string(l_msg) + ",\n" - sounds_str = sounds_str[:-1] # trim trailing newline + for l_msg in attr["sounds"]: + sounds_str += " " * 12 + make_c_string(l_msg) + ",\n" + sounds_str = sounds_str[:-1] # trim trailing newline texts_str = "" - if attr.get("texts") == None: + if attr.get("texts") is None: texts_str = " " * 12 + "NULL," else: - for l_msg in attr["texts"]: - texts_str += " " * 12 + make_c_string(l_msg) + ",\n" - texts_str = texts_str[:-1] # trim trailing newline + for l_msg in attr["texts"]: + texts_str += " " * 12 + make_c_string(l_msg) + ",\n" + texts_str = texts_str[:-1] # trim trailing newline changes_str = "" - if attr.get("changes") == None: + if attr.get("changes") is None: changes_str = " " * 12 + "NULL," else: - for l_msg in attr["changes"]: - changes_str += " " * 12 + make_c_string(l_msg) + ",\n" - changes_str = changes_str[:-1] # trim trailing newline + for l_msg in attr["changes"]: + changes_str += " " * 12 + make_c_string(l_msg) + ",\n" + changes_str = changes_str[:-1] # trim trailing newline locs = attr.get("locations", ["LOC_NOWHERE", "LOC_NOWHERE"]) immovable = attr.get("immovable", False) try: - if type(locs) == str: + if isinstance(locs, str): locs = [locs, -1 if immovable else 0] except IndexError: sys.stderr.write("dungeon: unknown object location in %s\n" % locs) @@ -255,13 +255,13 @@ def get_motions(motions): mot_str = "" for motion in motions: contents = motion[1] - if contents["words"] == None: + if contents["words"] is None: words_str = get_string_group([]) else: words_str = get_string_group(contents["words"]) mot_str += template.format(words_str) global ignore - if contents.get("oldstyle", True) == False: + if not contents.get("oldstyle", True): for word in contents["words"]: if len(word) == 1: ignore += word.upper() @@ -278,24 +278,24 @@ def get_actions(actions): for action in actions: contents = action[1] - if contents["words"] == None: + if contents["words"] is None: words_str = get_string_group([]) else: words_str = get_string_group(contents["words"]) - if contents["message"] == None: + if contents["message"] is None: message = "NULL" else: message = make_c_string(contents["message"]) - if contents.get("noaction") == None: + if contents.get("noaction") is None: noaction = "false" else: noaction = "true" act_str += template.format(words_str, message, noaction) global ignore - if contents.get("oldstyle", True) == False: + if not contents.get("oldstyle", True): for word in contents["words"]: if len(word) == 1: ignore += word.upper() @@ -304,7 +304,7 @@ def get_actions(actions): def bigdump(arr): out = "" - for (i, entry) in enumerate(arr): + for (i, _) in enumerate(arr): if i % 10 == 0: if out and out[-1] == ' ': out = out[:-1] @@ -374,6 +374,7 @@ def buildtravel(locs, objs): return locnames.index(action[1]) except ValueError: sys.stderr.write("dungeon: unknown location %s in goto clause of %s\n" % (action[1], name)) + raise ValueError elif action[0] == "special": return 300 + action[1] elif action[0] == "speak": @@ -384,10 +385,11 @@ def buildtravel(locs, objs): else: print(cond) raise ValueError + return '' # Pacify pylint def cencode(cond, name): if cond is None: return 0 - elif cond == ["nodwarves"]: + if cond == ["nodwarves"]: return 100 elif cond[0] == "pct": return cond[1] @@ -401,24 +403,24 @@ def buildtravel(locs, objs): try: return 200 + objnames.index(cond[1]) except IndexError: - sys.stderr.write("dungeon: unknown object name %s in with clause of \n" % (cond[1], name)) + sys.stderr.write("dungeon: unknown object name %s in with clause of %s\n" % (cond[1], name)) sys.exit(1) elif cond[0] == "not": try: obj = objnames.index(cond[1]) - if type(cond[2]) == int: + if isinstance(cond[2], int): state = cond[2] elif cond[2] in objs[obj][1].get("states", []): state = objs[obj][1].get("states").index(cond[2]) else: for (i, stateclause) in enumerate(objs[obj][1]["descriptions"]): - if type(stateclause) == list: + if isinstance(stateclause, list): if stateclause[0] == cond[2]: state = i break else: sys.stderr.write("dungeon: unmatched state symbol %s in not clause of %s\n" % (cond[2], name)) - sys.exit(0); + sys.exit(0) return 300 + obj + 100 * state except ValueError: sys.stderr.write("dungeon: unknown object name %s in not clause of %s\n" % (cond[1], name)) @@ -451,9 +453,9 @@ def buildtravel(locs, objs): newloc = rule.pop(0) if loc != oldloc: tkey.append(len(travel)) - oldloc = loc + oldloc = loc elif travel: - travel[-1][-1] = "false" if travel[-1][-1] == "true" else "true" + travel[-1][-1] = "false" if travel[-1][-1] == "true" else "true" while rule: cond = newloc // 1000 nodwarves = (cond == 100) @@ -482,13 +484,13 @@ def buildtravel(locs, objs): condarg2 = (cond - 300) // 100. dest = newloc % 1000 if dest <= 300: - desttype = "dest_goto"; + desttype = "dest_goto" destval = locnames[dest] elif dest > 500: - desttype = "dest_speak"; + desttype = "dest_speak" destval = msgnames[dest - 500] else: - desttype = "dest_special"; + desttype = "dest_special" destval = locnames[dest - 300] travel.append([len(tkey)-1, locnames[len(tkey)-1], @@ -542,7 +544,7 @@ if __name__ == "__main__": c_template = DONOTEDIT_COMMENT + ctf.read() except IOError as e: print('ERROR: reading template failed ({})'.format(e.strerror)) - exit(-1) + sys.exit(-1) c = c_template.format( h_file = H_NAME, @@ -557,7 +559,7 @@ if __name__ == "__main__": motions = get_motions(db["motions"]), actions = get_actions(db["actions"]), tkeys = bigdump(tkey), - travel = get_travel(travel), + travel = get_travel(travel), ignore = ignore ) diff --git a/make_graph.py b/make_graph.py index fcf44fb..1cb975c 100755 --- a/make_graph.py +++ b/make_graph.py @@ -12,7 +12,7 @@ Make a DOT graph of Colossal Cave. # Copyright (c) 2017 by Eric S. Raymond # SPDX-License-Identifier: BSD-2-clause -import sys, yaml, getopt +import sys, getopt, yaml def allalike(loc): "Select out loci related to the Maze All Alike" @@ -112,7 +112,7 @@ if __name__ == "__main__": subset = surface else: sys.stderr.write(__doc__) - raise SystemExit(1) + raise SystemExit(1) startlocs = {} for obj in db["objects"]: @@ -135,7 +135,7 @@ if __name__ == "__main__": startlocs[location] = [objname] print("digraph G {") - + for (loc, attrs) in db["locations"]: if is_forwarder(loc): continue @@ -145,8 +145,8 @@ if __name__ == "__main__": if loc in startlocs: node_label += "\\n" + ",".join(startlocs[loc]).lower() print(' %s [shape=box,label="%s"]' % (loc[4:], node_label)) - - for (loc, attrs) in db["locations"]: + + for (loc, attrs) in db["locations"]: travel = attrs["travel"] if len(travel) > 0: for dest in travel: @@ -157,7 +157,7 @@ if __name__ == "__main__": if action[0] == "goto": dest = forward(action[1]) if not (subset(loc) or subset(dest)): - continue; + continue arc = "%s -> %s" % (loc[4:], dest[4:]) label=",".join(verbs).lower() if len(label) > 0: diff --git a/tests/coverage_dungeon.py b/tests/coverage_dungeon.py index 182930c..8bb1c46 100755 --- a/tests/coverage_dungeon.py +++ b/tests/coverage_dungeon.py @@ -1,20 +1,21 @@ #!/usr/bin/env python3 +""" +This is the open-adventure dungeon text coverage report generator. It +consumes a YAML description of the dungeon and determines whether the +various strings contained are present within the test check files. -# This is the open-adventure dungeon text coverage report generator. It -# consumes a YAML description of the dungeon and determines whether the -# various strings contained are present within the test check files. -# -# The default HTML output is appropriate for use with Gitlab CI. -# You can override it with a command-line argument. -# -# The DANGLING list is for actions that should be considered always found -# even if the checkfile search doesn't find them. Typically this will because -# they emit a templated message that can't be regression-tested by equality. +The default HTML output is appropriate for use with Gitlab CI. +You can override it with a command-line argument. + +The DANGLING list is for actions that should be considered always found +even if the checkfile search doesn't find them. Typically this will because +they emit a templated message that can't be regression-tested by equality. +""" import os import sys -import yaml import re +import yaml TEST_DIR = "." YAML_PATH = "../adventure.yaml" @@ -63,11 +64,11 @@ def search(needle, haystack): # Search for needle in haystack, first escaping needle for regex, then # replacing %s, %d, etc. with regex wildcards, so the variable messages # within the dungeon definition will actually match - - if needle == None or needle == "" or needle == "NO_MESSAGE": + + if needle is None or needle == "" or needle == "NO_MESSAGE": # if needle is empty, assume we're going to find an empty string return True - + needle_san = re.escape(needle) \ .replace("\\n", "\n") \ .replace("\\t", "\t") \ @@ -80,7 +81,7 @@ def search(needle, haystack): def obj_coverage(objects, text, report): # objects have multiple descriptions based on state - for i, objouter in enumerate(objects): + for _, objouter in enumerate(objects): (obj_name, obj) = objouter if obj["descriptions"]: for j, desc in enumerate(obj["descriptions"]): @@ -88,7 +89,7 @@ def obj_coverage(objects, text, report): if name not in report["messages"]: report["messages"][name] = {"covered" : False} report["total"] += 1 - if report["messages"][name]["covered"] != True and search(desc, text): + if not report["messages"][name]["covered"] and search(desc, text): report["messages"][name]["covered"] = True report["covered"] += 1 @@ -100,26 +101,26 @@ def loc_coverage(locations, text, report): if name not in report["messages"]: report["messages"][name] = {"long" : False, "short": False} report["total"] += 2 - if report["messages"][name]["long"] != True and search(desc["long"], text): + if not report["messages"][name]["long"] and search(desc["long"], text): report["messages"][name]["long"] = True report["covered"] += 1 - if report["messages"][name]["short"] != True and search(desc["short"], text): + if not report["messages"][name]["short"] and search(desc["short"], text): report["messages"][name]["short"] = True report["covered"] += 1 def hint_coverage(obituaries, text, report): # hints have a "question" where the hint is offered, followed # by the actual hint if the player requests it - for i, hintouter in enumerate(obituaries): + for _, hintouter in enumerate(obituaries): hint = hintouter["hint"] name = hint["name"] if name not in report["messages"]: report["messages"][name] = {"question" : False, "hint": False} report["total"] += 2 - if report["messages"][name]["question"] != True and search(hint["question"], text): + if not report["messages"][name]["question"] and search(hint["question"], text): report["messages"][name]["question"] = True report["covered"] += 1 - if report["messages"][name]["hint"] != True and search(hint["hint"], text): + if not report["messages"][name]["hint"] and search(hint["hint"], text): report["messages"][name]["hint"] = True report["covered"] += 1 @@ -130,10 +131,10 @@ def obit_coverage(obituaries, text, report): if name not in report["messages"]: report["messages"][name] = {"query" : False, "yes_response": False} report["total"] += 2 - if report["messages"][name]["query"] != True and search(obit["query"], text): + if not report["messages"][name]["query"] and search(obit["query"], text): report["messages"][name]["query"] = True report["covered"] += 1 - if report["messages"][name]["yes_response"] != True and search(obit["yes_response"], text): + if not report["messages"][name]["yes_response"] and search(obit["yes_response"], text): report["messages"][name]["yes_response"] = True report["covered"] += 1 @@ -144,7 +145,7 @@ def threshold_coverage(classes, text, report): if name not in report["messages"]: report["messages"][name] = {"covered" : "False"} report["total"] += 1 - if report["messages"][name]["covered"] != True and search(item["message"], text): + if not report["messages"][name]["covered"] and search(item["message"], text): report["messages"][name]["covered"] = True report["covered"] += 1 @@ -153,7 +154,7 @@ def arb_coverage(arb_msgs, text, report): if name not in report["messages"]: report["messages"][name] = {"covered" : False} report["total"] += 1 - if report["messages"][name]["covered"] != True and search(message, text): + if not report["messages"][name]["covered"] and search(message, text): report["messages"][name]["covered"] = True report["covered"] += 1 @@ -163,7 +164,7 @@ def actions_coverage(items, text, report): if name not in report["messages"]: report["messages"][name] = {"covered" : False} report["total"] += 1 - if report["messages"][name]["covered"] != True and (search(item["message"], text) or name in DANGLING): + if not report["messages"][name]["covered"] and (search(item["message"], text) or name in DANGLING): report["messages"][name]["covered"] = True report["covered"] += 1 @@ -199,8 +200,8 @@ if __name__ == "__main__": with open(YAML_PATH, "r") as f: db = yaml.safe_load(f) except IOError as e: - print('ERROR: could not load {} ({}})'.format(YAML_PATH, e.strerror)) - exit(-1) + print('ERROR: could not load %s (%s)' % (YAML_PATH, e.strerror)) + sys.exit(-1) # get contents of all the check files check_file_contents = [] @@ -235,7 +236,7 @@ if __name__ == "__main__": for message_id, covered in cat_messages: category_html_row = "" for key, value in covered.items(): - category_html_row += HTML_CATEGORY_COVERAGE_CELL.format("uncovered" if value != True else "covered") + category_html_row += HTML_CATEGORY_COVERAGE_CELL.format("uncovered" if not value else "covered") category_html += HTML_CATEGORY_ROW.format(id=message_id,colspan=colspan, cells=category_html_row) categories_html += HTML_CATEGORY_SECTION.format(id=name, rows=category_html) @@ -258,7 +259,7 @@ if __name__ == "__main__": html_template = f.read() except IOError as e: print('ERROR: reading HTML report template failed ({})'.format(e.strerror)) - exit(-1) + sys.exit(-1) # parse template with report and write it out try: -- 2.31.1