pylint cleanup.

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

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 nontrivial part of this is the compilation of the YAML for
-# the dungeon and outputs a dungeon.h and dungeon.c pair of C code files.
+movement rules to the travel array that's actually used by
-#
+playermove().
-# 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
 
+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,21 +159,21 @@ 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
         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
         changes_str = ""
-        if attr.get("changes") == None:
+        if attr.get("changes") is None:
             changes_str = " " * 12 + "NULL,"
         else:
             for l_msg in attr["changes"]:
@@ -182,7 +182,7 @@ def get_objects(obj):
         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))
@@ -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,

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"
@@ -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:

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
+The default HTML output is appropriate for use with Gitlab CI.
-# consumes a YAML description of the dungeon and determines whether the
+You can override it with a command-line argument.
-# various strings contained are present within the test check files.
+
-#
+The DANGLING list is for actions that should be considered always found
-# The default HTML output is appropriate for use with Gitlab CI.
+even if the checkfile search doesn't find them. Typically this will because
-# You can override it with a command-line argument.
+they emit a templated message that can't be regression-tested by equality.
-#
+"""
-# 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"
@@ -64,7 +65,7 @@ def search(needle, haystack):
     # 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
 
@@ -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))
+        print('ERROR: could not load %s (%s)' % (YAML_PATH, e.strerror))
-        exit(-1)
+        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: