Add linter for Python scripts

.github/workflows/build.yml

@@ -66,6 +66,20 @@ jobs:
           dirs="include source"
           find $dirs -regex '.*\.\(c\|h\|cc\|hh\)' | xargs clang-format-16 --dry-run -Werror
 
+  lint:
+    name: Lint Python scripts
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Install Pylint
+        run: pip install pylint discord dotenv
+      - name: Lint
+        run: |
+          dirs="./configure.py tools"
+          find $dirs -regex '.*\.py' | xargs python -m pylint -E
+          find $dirs -regex '.*\.py' | xargs python -m pylint --exit-zero
+
   verify:
     name: Verify
     runs-on: ubuntu-latest

.pylintrc

@@ -0,0 +1,14 @@
+[MAIN]
+output-format=colorized
+init-hook='import sys; sys.path.append("tools")'
+
+[MESSAGES CONTROL]
+disable=missing-module-docstring,
+        missing-class-docstring,
+        missing-function-docstring,
+        too-few-public-methods,
+        too-many-locals,
+        invalid-name
+
+[FORMAT]
+max-line-length=100

configure.py

@@ -27,7 +27,7 @@
 n.newline()
 
 common_ccflags = [
-    '-DREVOLUTION',
+    '-DREVOLUTION'
     '-fno-asynchronous-unwind-tables',
     '-fno-exceptions',
     '-fno-rtti',
@@ -75,7 +75,7 @@
     description='LD $out',
 )
 
-code_in_files = [file for file in glob('**/*.cc', recursive=True)]
+code_in_files = list(glob('**/*.cc', recursive=True))
 
 target_code_out_files = []
 debug_code_out_files = []
@@ -149,6 +149,6 @@
     ],
 )
 
-with open('build.ninja', 'w') as out_file:
+with open('build.ninja', 'w', encoding="utf-8") as out_file:
     out_file.write(out_buf.getvalue())
 n.close()

tools/discord/bot.py

@@ -1,8 +1,8 @@
+import os
+from typing import Optional
 import asyncio
 import discord
 from dotenv import load_dotenv
-import os
-from typing import Optional
 
 # Global variable management
 load_dotenv()
@@ -73,7 +73,7 @@ async def dolphin_ok() -> bool:
 async def dolphin_fail() -> Optional[str]:
     fail_path = os.path.join(NAND_PATH, "fail")
     if os.path.exists(fail_path):
-        with open(fail_path, "r") as f:
+        with open(fail_path, "r", encoding="utf-8") as f:
             return f.read()
     else:
         return None
@@ -113,7 +113,7 @@ async def dolphin_generate_krkg(ghost: bytes, interaction: discord.Interaction):
         await respond_fail_error(interaction, fail)
         return
     # Empty string is falsey, leading to two situations where fail is False
-    elif os.path.exists(os.path.join(NAND_PATH, "fail")):
+    if os.path.exists(os.path.join(NAND_PATH, "fail")):
         await respond_bug_error(
             interaction, "Dolphin returned fail, but there's no explanation"
         )
@@ -155,7 +155,7 @@ async def replay_run() -> int:
 async def replay_results() -> Optional[str]:
     results_path = os.path.join(KINOKO_PATH, "results.txt")
     if os.path.exists(results_path):
-        with open(results_path, "r") as f:
+        with open(results_path, "r", encoding="utf-8") as f:
             return f.read()
     else:
         return None
@@ -187,7 +187,7 @@ async def replay_exec(ghost: bytes, interaction: discord.Interaction):
         )
         return
     # Empty string is falsey, leading to two situations where fail is False
-    elif os.path.exists(os.path.join(KINOKO_PATH, "results.txt")):
+    if os.path.exists(os.path.join(KINOKO_PATH, "results.txt")):
         await respond_bug_error(
             interaction, "Kinoko closed, but there's no explanation"
         )
@@ -315,7 +315,7 @@ async def command_generate_krkg(
     if ghost.size < 0x8C or ghost.size > 0x2800:
         await respond_generic_error(
             interaction,
-            f"File is too {"small" if ghost.size < 0x8c else "big"} to be an RKG",
+            f"File is too {'small' if ghost.size < 0x8c else 'big'} to be an RKG",
         )
         return
 
@@ -360,7 +360,7 @@ async def command_replay_ghost(
     if ghost.size < 0x8C or ghost.size > 0x2800:
         await respond_generic_error(
             interaction,
-            f"File is too {"small" if ghost.size < 0x8c else "big"} to be an RKG",
+            f"File is too {'small' if ghost.size < 0x8c else 'big'} to be an RKG",
         )
         return
 

tools/generate_tests.py

@@ -14,7 +14,7 @@ def generate_tests(filename = 'testCases.json', out_filename = 'out/testCases.bi
     # Parse test cases from JSON
     tests = []
 
-    with open(filename) as f:
+    with open(filename, encoding="utf-8") as f:
         data = json.load(f)
 
         for key, value in data.items():

tools/progress.py

@@ -1,8 +1,6 @@
-import json
 import os
-import requests
-import struct
 import sys
+import requests
 
 BADGE_URL_PREFIX = "https://badgen.net/static/"
 
@@ -12,22 +10,22 @@
 if __name__ == '__main__':
     # Look through provided Kinoko output
     lines = b''
-    with open(sys.argv[1], "r") as f:
-        lines = f.readlines();
+    with open(sys.argv[1], "r", encoding="utf-8") as f:
+        lines = f.readlines()
     os.makedirs(os.path.dirname(OUT_BADGE_DIR), exist_ok=True)
     for name, sync, targetFrames, totalFrames in zip(*[iter(lines)]*4):
         name = name.strip()
         sync = int(sync)
         targetFrames = int(targetFrames)
         totalFrames = int(totalFrames)
 
-        if (sync == 0):
+        if sync == 0:
             print(name + " desynced! Exiting out...")
-            exit(1)
-        
+            sys.exit(1)
+
         percent = (targetFrames / totalFrames) * 100
-        
+
         url = BADGE_URL_PREFIX + name + "/" + f"{percent:.1f}" + "%/green"
-        r = requests.get(url)
+        r = requests.get(url, timeout=120)
         with open(OUT_BADGE_DIR + name + OUT_BADGE_EXT, 'wb') as f:
             f.write(r.content)

tools/status_check.py

@@ -1,13 +1,13 @@
-# Validate that the framecounts in STATUS.md are up-to-date. This is done by trying to run the entire test case and
-# asserting that the sync framecount is the highest it currently can be. This helps us capture when changes focused
-# on one test case actually benefit a different test case.
+# Validate that the framecounts in STATUS.md are up-to-date. This is done by trying to run the
+# entire test case and asserting that the sync framecount is the highest it currently can be. This
+# helps us capture when changes focused on one test case actually benefit a different test case.
 
-from generate_tests import generate_tests
 import json
 import os
 import subprocess
 import sys
 from typing import Dict, TypedDict
+from generate_tests import generate_tests
 
 STATUS_TEST_CASE_FILENAME = 'statusTestCases.json'
 
@@ -72,50 +72,57 @@ def __init__(self, stdout):
     def get_test_case_name(self) -> str:
         return self.current_line.split(':')[3].split(' ')[1]
 
-    # e.g. Get the "814" from [TestDirector.hh:55] REPORT: Test Case Failed: rr-ng-rta-2-24-281 [814 / 9060]
+    # e.g. Get the "814" from
+    # [TestDirector.hh:55] REPORT: Test Case Failed: rr-ng-rta-2-24-281 [814 / 9060]
     def get_test_case_frame_lower_bound(self) -> int:
         return int(self.current_line.split(':')[3].split('[')[1].split(' ')[0])
 
     def validate_test_case(self, test_case: TestCase) -> bool:
         desync_frame = self.test_cases[test_case.name]
 
+        case_name_just = test_case.name.ljust(20)
+
         if desync_frame == test_case.total_frames:
             if test_case.sync_frame == test_case.total_frames:
                 if test_case.emoji != '✔️':
                     # Wrong emoji. It should be a checkmark.
-                    print(f"{test_case.name.ljust(20)}\t❌\t->\t✔️\t[EMOJI]")
+                    print(f"{case_name_just}\t❌\t->\t✔️\t[EMOJI]")
                     return False
 
                 if test_case.has_description:
                     # There should be no desync reason for a synced test case.
                     print(
-                        f"{test_case.name.ljust(20)}\tSynchronized runs should not have a desync reason [REASON]")
+                        case_name_just + \
+                        "\tSynchronized runs should not have a desync reason [REASON]")
                     return False
                 return True
 
             # We have a synced test case, but the label is incorrect
-            print(
-                f"{test_case.name.ljust(20)}\t{test_case.sync_frame}\t->\t{test_case.total_frames}\t[LABEL]")
+            msg = [
+                case_name_just, test_case.sync_frame,
+                "->", test_case.total_frames, "[LABEL]"
+            ]
+            print("\t".join(msg))
             return False
 
         if desync_frame - 1 == test_case.sync_frame:
             if test_case.emoji != '❌':
                 # Wrong emoji. It should be an x.
-                print(f"{test_case.name.ljust(20)}\t✔️\t->\t❌\t[EMOJI]")
+                print(f"{case_name_just}\t✔️\t->\t❌\t[EMOJI]")
                 return False
 
             if not test_case.has_description:
                 # There should be a desync reason if we haven't synced the full test case.
                 print(
-                    f"{test_case.name.ljust(20)}\tDesynchronized runs should have a desync reason [REASON]")
+                    f"{case_name_just}\tDesynchronized runs should have a desync reason [REASON]")
                 return False
             return True
 
         # Our framecount isn't up-to-date - the test case desyncs either later or
         # earlier than captured in STATUS.md
         early_or_late = "EARLY" if (desync_frame - 1 < test_case.sync_frame) else "LATER"
         print(
-            f"{test_case.name.ljust(20)}\t{test_case.sync_frame}\t->\t{desync_frame - 1}\t[{early_or_late}]")
+            f"{case_name_just}\t{test_case.sync_frame}\t->\t{desync_frame - 1}\t[{early_or_late}]")
 
         return False
 
@@ -125,7 +132,7 @@ def get_test_cases_from_status_md() -> Dict[str, TestCase]:
     # Read STATUS.md and parse test cases to dictionary
     with open('STATUS.md', 'r', encoding='utf-8') as f:
         # Skip the first 4 lines to get to first test case line
-        for i in range(4):
+        for _ in range(4):
             next(f)
 
         for line in f:
@@ -151,7 +158,7 @@ def create_json_file(test_cases: Dict[str, TestCase]):
     for key, value in test_cases.items():
         test_case_dict[key] = value.to_json()
 
-    with open(STATUS_TEST_CASE_FILENAME, 'w') as f:
+    with open(STATUS_TEST_CASE_FILENAME, 'w', encoding="utf-8") as f:
         f.write(json.dumps(test_case_dict, indent=4))
 
 
@@ -165,15 +172,17 @@ def main():
     generate_tests(STATUS_TEST_CASE_FILENAME)
 
     # Run Kinoko. subprocess.run shell behavior varies between Windows and Linux
-    exec = os.path.join('.', 'kinoko')
+    exec = os.path.join('.', 'kinoko') # pylint: disable=redefined-builtin
     args = [
         exec,
         "-m",
         "test",
         "-s",
         "testCases.bin",
     ] if sys.platform.startswith('win32') else exec + " -m test -s testCases.bin"
-    result = subprocess.run(args, cwd='out', shell=True, capture_output=True, text=True)
+    result = subprocess.run(
+        args, cwd='out', shell=True, capture_output=True, text=True, check=False
+    )
 
     # Check each test case is up-to-date. If not, return non-zero exit code so
     # our build action is aware.