From 5d0788457cc24ec63aae889daa3eb21578b8143f Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Sun, 10 Dec 2023 20:33:28 -0500
Subject: [PATCH 01/10] linter

---
 tools/checkorder/checkorder.py                | 150 ++++++++----------
 .../isledecomp/isledecomp/parser/__init__.py  |   1 +
 tools/isledecomp/isledecomp/parser/error.py   |  26 +++
 tools/isledecomp/isledecomp/parser/linter.py  |  96 +++++++++++
 tools/isledecomp/isledecomp/parser/node.py    |   6 -
 tools/isledecomp/isledecomp/parser/parser.py  |  13 +-
 tools/isledecomp/tests/test_linter.py         |  70 ++++++++
 tools/isledecomp/tests/test_parser.py         |  17 ++
 8 files changed, 285 insertions(+), 94 deletions(-)
 create mode 100644 tools/isledecomp/isledecomp/parser/linter.py
 create mode 100644 tools/isledecomp/tests/test_linter.py

diff --git a/tools/checkorder/checkorder.py b/tools/checkorder/checkorder.py
index 0cdba670..2263ebbe 100644
--- a/tools/checkorder/checkorder.py
+++ b/tools/checkorder/checkorder.py
@@ -1,97 +1,87 @@
 import os
 import sys
 import argparse
+import colorama
 from isledecomp.dir import walk_source_dir, is_file_cpp
-from isledecomp.parser import DecompParser
+from isledecomp.parser import DecompLinter
+
+colorama.init()
 
 
-def sig_truncate(sig: str) -> str:
-    """Helper to truncate function names to 50 chars and append ellipsis
-    if needed. Goal is to stay under 80 columns for tool output."""
-    return f"{sig[:47]}{'...' if len(sig) >= 50 else ''}"
+def display_errors(alerts, filename):
+    sorted_alerts = sorted(alerts, key=lambda a: a.line_number)
+
+    for alert in sorted_alerts:
+        error_type = (
+            f"{colorama.Fore.RED}error: "
+            if alert.is_error()
+            else f"{colorama.Fore.YELLOW}warning: "
+        )
+        components = [
+            colorama.Fore.LIGHTWHITE_EX,
+            filename,
+            ":",
+            str(alert.line_number),
+            " : ",
+            error_type,
+            colorama.Fore.LIGHTWHITE_EX,
+            alert.code.name.lower(),
+        ]
+        print("".join(components))
+
+        if alert.line is not None:
+            print(f"{colorama.Fore.WHITE}  {alert.line}")
 
 
-def check_file(filename: str, verbose: bool = False) -> bool:
-    """Open and read the given file, then check whether the code blocks
-    are in order. If verbose, print each block."""
-
-    parser = DecompParser()
-    with open(filename, "r", encoding="utf-8") as f:
-        parser.read_lines(f)
-
-    just_offsets = [block.offset for block in parser.functions]
-    sorted_offsets = sorted(just_offsets)
-    file_out_of_order = just_offsets != sorted_offsets
-
-    # TODO: When we add parser error severity, actual errors that obstruct
-    # parsing should probably be shown here regardless of verbose mode
-
-    # If we detect inexact comments, don't print anything unless we are
-    # in verbose mode. If the file is out of order, we always print the
-    # file name.
-    should_report = (len(parser.alerts) > 0 and verbose) or file_out_of_order
-
-    if not should_report and not file_out_of_order:
-        return False
-
-    # Else: we are alerting to some problem in this file
-    print(filename)
-    if verbose:
-        if file_out_of_order:
-            order_lookup = {k: i for i, k in enumerate(sorted_offsets)}
-            prev_offset = 0
-
-            for fun in parser.functions:
-                msg = " ".join(
-                    [
-                        " " if fun.offset > prev_offset else "!",
-                        f"{fun.offset:08x}",
-                        f"{fun.end_line - fun.line_number:4} lines",
-                        f"{order_lookup[fun.offset]:3}",
-                        "    ",
-                        sig_truncate(fun.name),
-                    ]
-                )
-                print(msg)
-                prev_offset = fun.offset
-
-        for alert in parser.alerts:
-            print(f"* line {alert.line_number:4} {alert.code} ({alert.line})")
-
-        print()
-
-    return file_out_of_order
-
-
-def parse_args(test_args: list | None = None) -> dict:
+def parse_args() -> dict:
     p = argparse.ArgumentParser(
-        description="Checks the source files to make sure the function offset comments are in order",
+        description="Syntax checking and linting for decomp annotation markers."
     )
     p.add_argument("target", help="The file or directory to check.")
     p.add_argument(
         "--enforce",
         action=argparse.BooleanOptionalAction,
         default=False,
-        help="Fail with error code if target is out of order.",
+        help="Fail if syntax errors are found.",
     )
     p.add_argument(
-        "--verbose",
+        "--module",
+        required=False,
+        type=str,
+        help="If present, run targeted checks for markers from the given module.",
+    )
+    p.add_argument(
+        "--warnfail",
         action=argparse.BooleanOptionalAction,
         default=False,
-        help=(
-            "Display each code block in the file and show "
-            "where each consecutive run of blocks is broken."
-        ),
+        help="Fail if syntax warnings are found and enforce is enabled.",
     )
 
-    if test_args is None:
-        args = p.parse_args()
-    else:
-        args = p.parse_args(test_args)
-
+    args = p.parse_args()
     return vars(args)
 
 
+def process_files(files, module=None):
+    warning_count = 0
+    error_count = 0
+
+    linter = DecompLinter()
+    for filename in files:
+        success = linter.check_file(filename, module)
+
+        warnings = [a for a in linter.alerts if a.is_warning()]
+        errors = [a for a in linter.alerts if a.is_error()]
+
+        error_count += len(errors)
+        warning_count += len(warnings)
+
+        if not success:
+            display_errors(linter.alerts, filename)
+            print()
+
+    return (warning_count, error_count)
+
+
 def main():
     args = parse_args()
 
@@ -102,24 +92,12 @@ def main():
     else:
         sys.exit("Invalid target")
 
-    files_out_of_order = 0
+    (warning_count, error_count) = process_files(files_to_check, module=args["module"])
 
-    for file in files_to_check:
-        is_jumbled = check_file(file, args["verbose"])
-        if is_jumbled:
-            files_out_of_order += 1
+    print(colorama.Style.RESET_ALL, end="")
 
-    if files_out_of_order > 0:
-        error_message = " ".join(
-            [
-                str(files_out_of_order),
-                "files are" if files_out_of_order > 1 else "file is",
-                "out of order",
-            ]
-        )
-        print(error_message)
-
-    if files_out_of_order > 0 and args["enforce"]:
+    would_fail = error_count > 0 or (warning_count > 0 and args["warnfail"])
+    if args["enforce"] and would_fail:
         sys.exit(1)
 
 
diff --git a/tools/isledecomp/isledecomp/parser/__init__.py b/tools/isledecomp/isledecomp/parser/__init__.py
index c9394d4a..3034a562 100644
--- a/tools/isledecomp/isledecomp/parser/__init__.py
+++ b/tools/isledecomp/isledecomp/parser/__init__.py
@@ -1 +1,2 @@
 from .parser import DecompParser
+from .linter import DecompLinter
diff --git a/tools/isledecomp/isledecomp/parser/error.py b/tools/isledecomp/isledecomp/parser/error.py
index c18e3e29..6de98c56 100644
--- a/tools/isledecomp/isledecomp/parser/error.py
+++ b/tools/isledecomp/isledecomp/parser/error.py
@@ -1,6 +1,8 @@
 from enum import Enum
+from dataclasses import dataclass
 
 
+# TODO: poorly chosen name, should be AlertType or AlertCode or something
 class ParserError(Enum):
     # WARN: Stub function exceeds some line number threshold
     UNLIKELY_STUB = 100
@@ -29,6 +31,13 @@ class ParserError(Enum):
     # and the start of the function. We can ignore it, but the line shouldn't be there
     UNEXPECTED_BLANK_LINE = 107
 
+    # WARN: We called the finish() method for the parser but had not reached the starting
+    # state of SEARCH
+    UNEXPECTED_END_OF_FILE = 108
+
+    # This code or higher is an error, not a warning
+    DECOMP_ERROR_START = 200
+
     # ERROR: We found a marker unexpectedly
     UNEXPECTED_MARKER = 200
 
@@ -39,3 +48,20 @@ class ParserError(Enum):
 
     # ERROR: The line following a synthetic marker was not a comment
     BAD_SYNTHETIC = 202
+
+    # ERROR: This function offset comes before the previous offset from the same module
+    # This hopefully gives some hint about which functions need to be rearranged.
+    FUNCTION_OUT_OF_ORDER = 203
+
+
+@dataclass
+class ParserAlert:
+    code: ParserError
+    line_number: int
+    line: str | None = None
+
+    def is_warning(self) -> bool:
+        return self.code.value < ParserError.DECOMP_ERROR_START.value
+
+    def is_error(self) -> bool:
+        return self.code.value >= ParserError.DECOMP_ERROR_START.value
diff --git a/tools/isledecomp/isledecomp/parser/linter.py b/tools/isledecomp/isledecomp/parser/linter.py
new file mode 100644
index 00000000..21666539
--- /dev/null
+++ b/tools/isledecomp/isledecomp/parser/linter.py
@@ -0,0 +1,96 @@
+from .parser import DecompParser
+from .error import ParserAlert, ParserError
+
+
+# TODO: You get an error for each function out of order.
+# Same as the verbose display but just as main line errors.
+# TODO: Sort alerts by line number at the end?
+# TODO: No more verbose mode. This is a linter and you get warnings always.
+# TODO: --warnfail flag?
+
+
+def get_checkorder_filter(module):
+    """Return a filter function on implemented functions in the given module"""
+    return lambda fun: fun.module == module and not fun.lookup_by_name
+
+
+class DecompLinter:
+    def __init__(self):
+        self.alerts = []
+        self._parser = DecompParser()
+        self._filename = ""
+        self._module = None
+
+    def reset(self):
+        self.alerts = []
+        self._parser.reset()
+        self._filename = ""
+        self._module = None
+
+    def file_is_header(self):
+        return self._filename.lower().endswith(".h")
+
+    def _check_function_order(self):
+        """Rules:
+        1. Only markers that are implemented in the file are considered. This means we
+        only look at markers that are cross-referenced with cvdump output by their line
+        number. Markers with the lookup_by_name flag set are ignored because we cannot
+        directly influence their order.
+
+        2. Order should be considered for a single module only. If we have multiple
+        markers for a single function (i.e. for LEGO1 functions linked statically to
+        ISLE) then the virtual address space will be very different. If we don't check
+        for one module only, we would incorrectly report that the file is out of order.
+        """
+
+        if self._module is None:
+            return
+
+        checkorder_filter = get_checkorder_filter(self._module)
+        last_offset = None
+        for fun in filter(checkorder_filter, self._parser.functions):
+            if last_offset is not None:
+                if fun.offset < last_offset:
+                    self.alerts.append(
+                        ParserAlert(
+                            code=ParserError.FUNCTION_OUT_OF_ORDER,
+                            line_number=fun.line_number,
+                        )
+                    )
+
+            last_offset = fun.offset
+
+    def _check_symbol_uniqueness(self):
+        # TODO
+        pass
+
+    def _check_byname_allowed(self):
+        # TODO
+        pass
+
+    def check_lines(self, lines, filename, module=None):
+        """`lines` is a generic iterable to allow for testing with a list of strings.
+        We assume lines has the entire contents of the compilation unit."""
+
+        self.reset()
+        self._filename = filename
+        self._module = module
+
+        self._parser.read_lines(lines)
+
+        self._parser.finish()
+        self.alerts = self._parser.alerts[::]
+
+        if self._module is not None:
+            self._check_byname_allowed()
+            self._check_symbol_uniqueness()
+
+            if not self.file_is_header():
+                self._check_function_order()
+
+        return len(self.alerts) == 0
+
+    def check_file(self, filename, module=None):
+        """Convenience method for decomplint cli tool"""
+        with open(filename, "r", encoding="utf-8") as f:
+            return self.check_lines(f, filename, module)
diff --git a/tools/isledecomp/isledecomp/parser/node.py b/tools/isledecomp/isledecomp/parser/node.py
index 0ee87000..721a24b9 100644
--- a/tools/isledecomp/isledecomp/parser/node.py
+++ b/tools/isledecomp/isledecomp/parser/node.py
@@ -6,12 +6,6 @@ class ParserNode:
     line_number: int
 
 
-@dataclass
-class ParserAlert(ParserNode):
-    code: int
-    line: str
-
-
 @dataclass
 class ParserSymbol(ParserNode):
     module: str
diff --git a/tools/isledecomp/isledecomp/parser/parser.py b/tools/isledecomp/isledecomp/parser/parser.py
index 89b79928..d133014c 100644
--- a/tools/isledecomp/isledecomp/parser/parser.py
+++ b/tools/isledecomp/isledecomp/parser/parser.py
@@ -12,12 +12,11 @@
     remove_trailing_comment,
 )
 from .node import (
-    ParserAlert,
     ParserFunction,
     ParserVariable,
     ParserVtable,
 )
-from .error import ParserError
+from .error import ParserAlert, ParserError
 
 
 class ReaderState(Enum):
@@ -29,6 +28,7 @@ class ReaderState(Enum):
     IN_GLOBAL = 5
     IN_FUNC_GLOBAL = 6
     IN_VTABLE = 7
+    DONE = 100
 
 
 def marker_is_stub(marker: DecompMarker) -> bool:
@@ -306,6 +306,9 @@ def _handle_marker(self, marker: DecompMarker):
             self._syntax_warning(ParserError.BOGUS_MARKER)
 
     def read_line(self, line: str):
+        if self.state == ReaderState.DONE:
+            return
+
         self.last_line = line  # TODO: Useful or hack for error reporting?
         self.line_number += 1
 
@@ -392,3 +395,9 @@ def read_line(self, line: str):
     def read_lines(self, lines: Iterable):
         for line in lines:
             self.read_line(line)
+
+    def finish(self):
+        if self.state != ReaderState.SEARCH:
+            self._syntax_warning(ParserError.UNEXPECTED_END_OF_FILE)
+
+        self.state = ReaderState.DONE
diff --git a/tools/isledecomp/tests/test_linter.py b/tools/isledecomp/tests/test_linter.py
new file mode 100644
index 00000000..6560e957
--- /dev/null
+++ b/tools/isledecomp/tests/test_linter.py
@@ -0,0 +1,70 @@
+import pytest
+from isledecomp.parser import DecompLinter
+from isledecomp.parser.error import ParserError
+
+
+@pytest.fixture(name="linter")
+def fixture_linter():
+    return DecompLinter()
+
+
+def test_simple_in_order(linter):
+    lines = [
+        "// FUNCTION: TEST 0x1000",
+        "void function1() {}",
+        "// FUNCTION: TEST 0x2000",
+        "void function2() {}",
+        "// FUNCTION: TEST 0x3000",
+        "void function3() {}",
+    ]
+    assert linter.check_lines(lines, "test.cpp", "TEST") is True
+
+
+def test_simple_not_in_order(linter):
+    lines = [
+        "// FUNCTION: TEST 0x1000",
+        "void function1() {}",
+        "// FUNCTION: TEST 0x3000",
+        "void function3() {}",
+        "// FUNCTION: TEST 0x2000",
+        "void function2() {}",
+    ]
+    assert linter.check_lines(lines, "test.cpp", "TEST") is False
+    assert len(linter.alerts) == 1
+
+    assert linter.alerts[0].code == ParserError.FUNCTION_OUT_OF_ORDER
+    # N.B. Line number given is the start of the function, not the marker
+    assert linter.alerts[0].line_number == 6
+
+
+def test_byname_ignored(linter):
+    """Should ignore lookup-by-name markers when checking order."""
+    lines = [
+        "// FUNCTION: TEST 0x1000",
+        "void function1() {}",
+        "// FUNCTION: TEST 0x3000",
+        "// MyClass::MyMethod",
+        "// FUNCTION: TEST 0x2000",
+        "void function2() {}",
+    ]
+    # TODO: This will fail after enforcing that bynames belong in headers
+    assert linter.check_lines(lines, "test.cpp", "TEST") is True
+    assert len(linter.alerts) == 0
+
+
+def test_module_isolation(linter):
+    """Should check the order of markers from a single module only."""
+    lines = [
+        "// FUNCTION: ALPHA 0x0001",
+        "// FUNCTION: TEST 0x1000",
+        "void function1() {}",
+        "// FUNCTION: ALPHA 0x0002",
+        "// FUNCTION: TEST 0x2000",
+        "void function2() {}",
+        "// FUNCTION: ALPHA 0x0003",
+        "// FUNCTION: TEST 0x3000",
+        "void function3() {}",
+    ]
+
+    assert linter.check_lines(lines, "test.cpp", "TEST") is True
+    assert linter.check_lines(lines, "test.cpp", "ALPHA") is True
diff --git a/tools/isledecomp/tests/test_parser.py b/tools/isledecomp/tests/test_parser.py
index 30092a97..c31602f1 100644
--- a/tools/isledecomp/tests/test_parser.py
+++ b/tools/isledecomp/tests/test_parser.py
@@ -358,3 +358,20 @@ def test_function_is_commented(parser):
     )
 
     assert len(parser.functions) == 0
+
+
+def test_unexpected_eof(parser):
+    """If a decomp marker finds its way to the last line of the file,
+    report that we could not get anything from it."""
+    parser.read_lines(
+        [
+            "// FUNCTION: TEST 0x1234",
+            "// Cls::Method",
+            "// FUNCTION: TEST 0x5555",
+        ]
+    )
+    parser.finish()
+
+    assert len(parser.functions) == 1
+    assert len(parser.alerts) == 1
+    assert parser.alerts[0].code == ParserError.UNEXPECTED_END_OF_FILE

From 07c0753a0125d3135d62a487b3e79aefd0b8ff71 Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Sun, 10 Dec 2023 20:47:12 -0500
Subject: [PATCH 02/10] Comment markers in headers only, rename script, update
 github actions

---
 .github/workflows/order.yml                   |  6 +++---
 .../decomplint.py}                            |  0
 tools/isledecomp/isledecomp/parser/error.py   |  3 +++
 tools/isledecomp/isledecomp/parser/linter.py  | 20 ++++++++++---------
 tools/isledecomp/tests/test_linter.py         | 12 +++++++++++
 5 files changed, 29 insertions(+), 12 deletions(-)
 rename tools/{checkorder/checkorder.py => decomplint/decomplint.py} (100%)

diff --git a/.github/workflows/order.yml b/.github/workflows/order.yml
index d7ae315c..0e0dda82 100644
--- a/.github/workflows/order.yml
+++ b/.github/workflows/order.yml
@@ -13,7 +13,7 @@ jobs:
       run: |
         pip install -r tools/requirements.txt
 
-    - name: Run checkorder.py
+    - name: Run decomplint.py
       run: |
-        python3 tools/checkorder/checkorder.py --verbose --enforce ISLE
-        python3 tools/checkorder/checkorder.py --verbose --enforce LEGO1
+        python3 tools/decomplint/decomplint.py --enforce ISLE --module ISLE
+        python3 tools/decomplint/decomplint.py --enforce LEGO1 --module LEGO1
diff --git a/tools/checkorder/checkorder.py b/tools/decomplint/decomplint.py
similarity index 100%
rename from tools/checkorder/checkorder.py
rename to tools/decomplint/decomplint.py
diff --git a/tools/isledecomp/isledecomp/parser/error.py b/tools/isledecomp/isledecomp/parser/error.py
index 6de98c56..58482296 100644
--- a/tools/isledecomp/isledecomp/parser/error.py
+++ b/tools/isledecomp/isledecomp/parser/error.py
@@ -35,6 +35,9 @@ class ParserError(Enum):
     # state of SEARCH
     UNEXPECTED_END_OF_FILE = 108
 
+    # WARN: We found a marker to be referenced by name outside of a header file.
+    BYNAME_FUNCTION_IN_CPP = 109
+
     # This code or higher is an error, not a warning
     DECOMP_ERROR_START = 200
 
diff --git a/tools/isledecomp/isledecomp/parser/linter.py b/tools/isledecomp/isledecomp/parser/linter.py
index 21666539..ea24c69c 100644
--- a/tools/isledecomp/isledecomp/parser/linter.py
+++ b/tools/isledecomp/isledecomp/parser/linter.py
@@ -2,13 +2,6 @@
 from .error import ParserAlert, ParserError
 
 
-# TODO: You get an error for each function out of order.
-# Same as the verbose display but just as main line errors.
-# TODO: Sort alerts by line number at the end?
-# TODO: No more verbose mode. This is a linter and you get warnings always.
-# TODO: --warnfail flag?
-
-
 def get_checkorder_filter(module):
     """Return a filter function on implemented functions in the given module"""
     return lambda fun: fun.module == module and not fun.lookup_by_name
@@ -65,8 +58,17 @@ def _check_symbol_uniqueness(self):
         pass
 
     def _check_byname_allowed(self):
-        # TODO
-        pass
+        if self.file_is_header():
+            return
+
+        for fun in self._parser.functions:
+            if fun.lookup_by_name:
+                self.alerts.append(
+                    ParserAlert(
+                        code=ParserError.BYNAME_FUNCTION_IN_CPP,
+                        line_number=fun.line_number,
+                    )
+                )
 
     def check_lines(self, lines, filename, module=None):
         """`lines` is a generic iterable to allow for testing with a list of strings.
diff --git a/tools/isledecomp/tests/test_linter.py b/tools/isledecomp/tests/test_linter.py
index 6560e957..6fca10fb 100644
--- a/tools/isledecomp/tests/test_linter.py
+++ b/tools/isledecomp/tests/test_linter.py
@@ -68,3 +68,15 @@ def test_module_isolation(linter):
 
     assert linter.check_lines(lines, "test.cpp", "TEST") is True
     assert linter.check_lines(lines, "test.cpp", "ALPHA") is True
+
+
+def test_byname_headers_only(linter):
+    """Markers that ar referenced by name with cvdump belong in header files only."""
+    lines = [
+        "// FUNCTION: TEST 0x1000",
+        "// MyClass::~MyClass",
+    ]
+
+    assert linter.check_lines(lines, "test.h", "TEST") is True
+    assert linter.check_lines(lines, "test.cpp", "TEST") is False
+    assert linter.alerts[0].code == ParserError.BYNAME_FUNCTION_IN_CPP

From 775256a142b094dc94b123eed8b215b205df6343 Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Sun, 10 Dec 2023 22:27:49 -0500
Subject: [PATCH 03/10] type hint compat

---
 tools/isledecomp/isledecomp/parser/error.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/isledecomp/isledecomp/parser/error.py b/tools/isledecomp/isledecomp/parser/error.py
index 58482296..34962eb0 100644
--- a/tools/isledecomp/isledecomp/parser/error.py
+++ b/tools/isledecomp/isledecomp/parser/error.py
@@ -1,3 +1,4 @@
+from __future__ import annotations  # python <3.10 compatibility
 from enum import Enum
 from dataclasses import dataclass
 

From 38b44e124644a4c2b7d943c1ebf74ed10ff7373f Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Mon, 11 Dec 2023 22:40:38 -0500
Subject: [PATCH 04/10] Rename github action, better argparse for linter

---
 .github/workflows/order.yml    |  4 ++--
 tools/decomplint/decomplint.py | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/order.yml b/.github/workflows/order.yml
index 0e0dda82..be766ad2 100644
--- a/.github/workflows/order.yml
+++ b/.github/workflows/order.yml
@@ -1,9 +1,9 @@
-name: Check order
+name: Decomp marker linting
 
 on: [push, pull_request]
 
 jobs:
-  checkorder:
+  decomplint:
     runs-on: ubuntu-latest
 
     steps:
diff --git a/tools/decomplint/decomplint.py b/tools/decomplint/decomplint.py
index 2263ebbe..83d65a2b 100644
--- a/tools/decomplint/decomplint.py
+++ b/tools/decomplint/decomplint.py
@@ -33,7 +33,7 @@ def display_errors(alerts, filename):
             print(f"{colorama.Fore.WHITE}  {alert.line}")
 
 
-def parse_args() -> dict:
+def parse_args() -> argparse.Namespace:
     p = argparse.ArgumentParser(
         description="Syntax checking and linting for decomp annotation markers."
     )
@@ -57,8 +57,8 @@ def parse_args() -> dict:
         help="Fail if syntax warnings are found and enforce is enabled.",
     )
 
-    args = p.parse_args()
-    return vars(args)
+    (args, _) = p.parse_known_args()
+    return args
 
 
 def process_files(files, module=None):
@@ -85,19 +85,19 @@ def process_files(files, module=None):
 def main():
     args = parse_args()
 
-    if os.path.isdir(args["target"]):
-        files_to_check = list(walk_source_dir(args["target"]))
-    elif os.path.isfile(args["target"]) and is_file_cpp(args["target"]):
-        files_to_check = [args["target"]]
+    if os.path.isdir(args.target):
+        files_to_check = list(walk_source_dir(args.target))
+    elif os.path.isfile(args.target) and is_file_cpp(args.target):
+        files_to_check = [args.target]
     else:
         sys.exit("Invalid target")
 
-    (warning_count, error_count) = process_files(files_to_check, module=args["module"])
+    (warning_count, error_count) = process_files(files_to_check, module=args.module)
 
     print(colorama.Style.RESET_ALL, end="")
 
-    would_fail = error_count > 0 or (warning_count > 0 and args["warnfail"])
-    if args["enforce"] and would_fail:
+    would_fail = error_count > 0 or (warning_count > 0 and args.warnfail)
+    if args.enforce and would_fail:
         sys.exit(1)
 
 

From 70f2a672ee21b1a293732eca9c7c34783908d011 Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Mon, 11 Dec 2023 22:41:03 -0500
Subject: [PATCH 05/10] Type hints, working test for byname ignore

---
 tools/isledecomp/isledecomp/parser/error.py  |  4 ++--
 tools/isledecomp/isledecomp/parser/linter.py | 13 +++++++------
 tools/isledecomp/isledecomp/parser/parser.py |  4 ++--
 tools/isledecomp/tests/test_linter.py        |  9 ++++++---
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/tools/isledecomp/isledecomp/parser/error.py b/tools/isledecomp/isledecomp/parser/error.py
index 34962eb0..638a806e 100644
--- a/tools/isledecomp/isledecomp/parser/error.py
+++ b/tools/isledecomp/isledecomp/parser/error.py
@@ -1,5 +1,5 @@
-from __future__ import annotations  # python <3.10 compatibility
 from enum import Enum
+from typing import Optional
 from dataclasses import dataclass
 
 
@@ -62,7 +62,7 @@ class ParserError(Enum):
 class ParserAlert:
     code: ParserError
     line_number: int
-    line: str | None = None
+    line: Optional[str] = None
 
     def is_warning(self) -> bool:
         return self.code.value < ParserError.DECOMP_ERROR_START.value
diff --git a/tools/isledecomp/isledecomp/parser/linter.py b/tools/isledecomp/isledecomp/parser/linter.py
index ea24c69c..99c8e272 100644
--- a/tools/isledecomp/isledecomp/parser/linter.py
+++ b/tools/isledecomp/isledecomp/parser/linter.py
@@ -1,3 +1,4 @@
+from typing import List, Optional
 from .parser import DecompParser
 from .error import ParserAlert, ParserError
 
@@ -8,11 +9,11 @@ def get_checkorder_filter(module):
 
 
 class DecompLinter:
-    def __init__(self):
-        self.alerts = []
+    def __init__(self) -> None:
+        self.alerts: List[ParserAlert] = []
         self._parser = DecompParser()
-        self._filename = ""
-        self._module = None
+        self._filename: str = ""
+        self._module: Optional[str] = None
 
     def reset(self):
         self.alerts = []
@@ -53,7 +54,7 @@ def _check_function_order(self):
 
             last_offset = fun.offset
 
-    def _check_symbol_uniqueness(self):
+    def _check_offset_uniqueness(self):
         # TODO
         pass
 
@@ -85,7 +86,7 @@ def check_lines(self, lines, filename, module=None):
 
         if self._module is not None:
             self._check_byname_allowed()
-            self._check_symbol_uniqueness()
+            self._check_offset_uniqueness()
 
             if not self.file_is_header():
                 self._check_function_order()
diff --git a/tools/isledecomp/isledecomp/parser/parser.py b/tools/isledecomp/isledecomp/parser/parser.py
index d133014c..336d4b0c 100644
--- a/tools/isledecomp/isledecomp/parser/parser.py
+++ b/tools/isledecomp/isledecomp/parser/parser.py
@@ -56,7 +56,7 @@ def marker_is_vtable(marker: DecompMarker) -> bool:
 
 
 class MarkerDict:
-    def __init__(self):
+    def __init__(self) -> None:
         self.markers: dict = {}
 
     def insert(self, marker: DecompMarker) -> bool:
@@ -80,7 +80,7 @@ class DecompParser:
     # pylint: disable=too-many-instance-attributes
     # Could combine output lists into a single list to get under the limit,
     # but not right now
-    def __init__(self):
+    def __init__(self) -> None:
         # The lists to be populated as we parse
         self.functions: List[ParserFunction] = []
         self.vtables: List[ParserVtable] = []
diff --git a/tools/isledecomp/tests/test_linter.py b/tools/isledecomp/tests/test_linter.py
index 6fca10fb..cb3b8f27 100644
--- a/tools/isledecomp/tests/test_linter.py
+++ b/tools/isledecomp/tests/test_linter.py
@@ -47,9 +47,12 @@ def test_byname_ignored(linter):
         "// FUNCTION: TEST 0x2000",
         "void function2() {}",
     ]
-    # TODO: This will fail after enforcing that bynames belong in headers
-    assert linter.check_lines(lines, "test.cpp", "TEST") is True
-    assert len(linter.alerts) == 0
+    # This will fail because byname lookup does not belong in the cpp file
+    assert linter.check_lines(lines, "test.cpp", "TEST") is False
+    # but it should not fail for function order.
+    assert all(
+        alert.code != ParserError.FUNCTION_OUT_OF_ORDER for alert in linter.alerts
+    )
 
 
 def test_module_isolation(linter):

From 7c1821a0e898dee28e02b78f770ab5236e2e88d6 Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Tue, 12 Dec 2023 13:33:54 -0500
Subject: [PATCH 06/10] CI rename and enable warnfail, enforce mode always on

---
 .github/workflows/{order.yml => analyze.yml} |  8 ++++----
 tools/decomplint/decomplint.py               | 10 ++--------
 2 files changed, 6 insertions(+), 12 deletions(-)
 rename .github/workflows/{order.yml => analyze.yml} (55%)

diff --git a/.github/workflows/order.yml b/.github/workflows/analyze.yml
similarity index 55%
rename from .github/workflows/order.yml
rename to .github/workflows/analyze.yml
index be766ad2..d3817d81 100644
--- a/.github/workflows/order.yml
+++ b/.github/workflows/analyze.yml
@@ -1,9 +1,9 @@
-name: Decomp marker linting
+name: Analyze
 
 on: [push, pull_request]
 
 jobs:
-  decomplint:
+  decomp-lint:
     runs-on: ubuntu-latest
 
     steps:
@@ -15,5 +15,5 @@ jobs:
 
     - name: Run decomplint.py
       run: |
-        python3 tools/decomplint/decomplint.py --enforce ISLE --module ISLE
-        python3 tools/decomplint/decomplint.py --enforce LEGO1 --module LEGO1
+        python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail
+        python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail
diff --git a/tools/decomplint/decomplint.py b/tools/decomplint/decomplint.py
index 83d65a2b..2ceb42c2 100644
--- a/tools/decomplint/decomplint.py
+++ b/tools/decomplint/decomplint.py
@@ -38,12 +38,6 @@ def parse_args() -> argparse.Namespace:
         description="Syntax checking and linting for decomp annotation markers."
     )
     p.add_argument("target", help="The file or directory to check.")
-    p.add_argument(
-        "--enforce",
-        action=argparse.BooleanOptionalAction,
-        default=False,
-        help="Fail if syntax errors are found.",
-    )
     p.add_argument(
         "--module",
         required=False,
@@ -54,7 +48,7 @@ def parse_args() -> argparse.Namespace:
         "--warnfail",
         action=argparse.BooleanOptionalAction,
         default=False,
-        help="Fail if syntax warnings are found and enforce is enabled.",
+        help="Fail if syntax warnings are found.",
     )
 
     (args, _) = p.parse_known_args()
@@ -97,7 +91,7 @@ def main():
     print(colorama.Style.RESET_ALL, end="")
 
     would_fail = error_count > 0 or (warning_count > 0 and args.warnfail)
-    if args.enforce and would_fail:
+    if would_fail:
         sys.exit(1)
 
 

From ef31d3eadbf1025f01019558b0794c821aeca6ed Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Tue, 12 Dec 2023 13:36:36 -0500
Subject: [PATCH 07/10] Two step linting

---
 .github/workflows/analyze.yml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml
index d3817d81..83df3130 100644
--- a/.github/workflows/analyze.yml
+++ b/.github/workflows/analyze.yml
@@ -3,7 +3,7 @@ name: Analyze
 on: [push, pull_request]
 
 jobs:
-  decomp-lint:
+  decomplint:
     runs-on: ubuntu-latest
 
     steps:
@@ -13,7 +13,10 @@ jobs:
       run: |
         pip install -r tools/requirements.txt
 
-    - name: Run decomplint.py
+    - name: Lint ISLE
       run: |
         python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail
+
+    - name: Lint LEGO1
+      run: |
         python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail

From bb528991ae322a736dba0eb52071de73f4ad7220 Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Tue, 12 Dec 2023 13:56:50 -0500
Subject: [PATCH 08/10] or one step

---
 .github/workflows/analyze.yml | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml
index 83df3130..ad14714d 100644
--- a/.github/workflows/analyze.yml
+++ b/.github/workflows/analyze.yml
@@ -13,10 +13,7 @@ jobs:
       run: |
         pip install -r tools/requirements.txt
 
-    - name: Lint ISLE
-      run: |
-        python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail
-
-    - name: Lint LEGO1
+    - name: Lint codebase
       run: |
+        python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail ||
         python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail

From 8e9977f38b9d6fa342c2fa4eaa56a46f4df455a9 Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Tue, 12 Dec 2023 14:06:53 -0500
Subject: [PATCH 09/10] continue on error

---
 .github/workflows/analyze.yml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml
index ad14714d..83141969 100644
--- a/.github/workflows/analyze.yml
+++ b/.github/workflows/analyze.yml
@@ -13,7 +13,11 @@ jobs:
       run: |
         pip install -r tools/requirements.txt
 
-    - name: Lint codebase
+    - name: Lint ISLE
+      continue-on-error: true
       run: |
-        python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail ||
-        python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail
+        python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail
+
+    - name: Lint LEGO1
+      run: |
+        python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail
\ No newline at end of file

From 99deb5d88671fe8ca93b533edea621af4141189d Mon Sep 17 00:00:00 2001
From: disinvite <disinvite@users.noreply.github.com>
Date: Tue, 12 Dec 2023 14:19:25 -0500
Subject: [PATCH 10/10] two jobs instead

---
 .github/workflows/analyze.yml | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml
index 83141969..e139b8bf 100644
--- a/.github/workflows/analyze.yml
+++ b/.github/workflows/analyze.yml
@@ -3,7 +3,7 @@ name: Analyze
 on: [push, pull_request]
 
 jobs:
-  decomplint:
+  decomplint-isle:
     runs-on: ubuntu-latest
 
     steps:
@@ -13,11 +13,20 @@ jobs:
       run: |
         pip install -r tools/requirements.txt
 
-    - name: Lint ISLE
-      continue-on-error: true
+    - name: Run decomplint.py
       run: |
         python3 tools/decomplint/decomplint.py ISLE --module ISLE --warnfail
 
-    - name: Lint LEGO1
+  decomplint-lego1:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Install python libraries
       run: |
-        python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail
\ No newline at end of file
+        pip install -r tools/requirements.txt
+
+    - name: Run decomplint.py
+      run: |
+        python3 tools/decomplint/decomplint.py LEGO1 --module LEGO1 --warnfail