tools: improve checking game strings

.pre-commit-config.yaml

@@ -14,6 +14,13 @@ repos:
     language: python
     stages: [commit]
 
+  - id: additional-lint -a
+    name: Run additional linters (repo-wide)
+    entry: tools/additional_lint -a
+    language: python
+    stages: [commit]
+    pass_filenames: false
+
   - id: imports
     name: imports
     entry: tools/sort_imports

tools/additional_lint

@@ -6,7 +6,7 @@ from fnmatch import fnmatch
 from pathlib import Path
 
 from shared.files import find_versioned_files, is_binary_file
-from shared.linting import LintContext, lint_bulk_files, lint_file
+from shared.linting import LintContext, lint_repo, lint_bulk_files, lint_file
 from shared.paths import REPO_DIR
 
 IGNORED_PATTERNS = ["*.patch", "*.bin", "gl_core_3_3.h"]
@@ -16,6 +16,7 @@ def parse_args() -> argparse.Namespace:
     parser = argparse.ArgumentParser()
     parser.add_argument("path", type=Path, nargs="*")
     parser.add_argument("-D", "--debug", action="store_true")
+    parser.add_argument("-a", "--all", action="store_true")
     return parser.parse_args()
 
 
@@ -46,7 +47,7 @@ def main(root_dir: Path) -> None:
 
     context = LintContext(
         root_dir=root_dir,
-        versioned_files=find_versioned_files(root_dir=REPO_DIR),
+        versioned_files=list(find_versioned_files(root_dir=REPO_DIR)),
     )
     if args.path:
         files = args.path
@@ -72,6 +73,13 @@ def main(root_dir: Path) -> None:
         print(str(lint_warning), file=sys.stderr)
         exit_code = 1
 
+    if args.all:
+        if args.debug:
+            print(f"Checking for repository-wide warnings...", file=sys.stderr)
+        for lint_warning in lint_repo(context):
+            print(str(lint_warning), file=sys.stderr)
+            exit_code = 1
+
     exit(exit_code)
 
 

tools/shared/linting.py

@@ -1,10 +1,15 @@
 #!/usr/bin/env python3
 import json
 import re
+from collections import defaultdict
 from collections.abc import Callable, Iterable
 from dataclasses import dataclass
 from pathlib import Path
 
+PROJECTS = ["tr1", "tr2", "libtrx"]
+RE_GAME_STRING_DEFINE = re.compile(r"GS_DEFINE\(([A-Z_]+),.*\)")
+RE_GAME_STRING_USAGE = re.compile(r"GS(?:_ID)?\(([A-Z_]+)\)")
+
 
 @dataclass
 class LintContext:
@@ -68,42 +73,129 @@ def lint_const_primitives(
             yield LintWarning(path, "useless const", line=i)
 
 
-def lint_game_strings(
+def get_relevant_project(context: LintContext, path: Path) -> str:
+    for project, project_path in get_project_paths(context).items():
+        if path.absolute().is_relative_to(project_path.absolute()):
+            break
+    else:
+        raise RuntimeError(f"{path}: Unable to get project path")
+    return project
+
+
+def get_project_paths(context: LintContext) -> dict[str, Path]:
+    return {
+        project: context.root_dir / "src" / project for project in PROJECTS
+    }
+
+
+def get_project_game_strings_paths(
+    context: LintContext,
+) -> dict[str, list[Path]]:
+    return {
+        project: list(project_path.rglob("**/game_string.def"))
+        for project, project_path in get_project_paths(context).items()
+    }
+
+
+def get_project_game_string_maps(context: LintContext) -> dict[str, set[str]]:
+    return {
+        project: [
+            match.group(1)
+            for path in def_paths
+            for match in re.finditer(RE_GAME_STRING_DEFINE, path.read_text())
+        ]
+        for project, def_paths in get_project_game_strings_paths(
+            context
+        ).items()
+    }
+
+
+def lint_undefined_game_strings(
     context: LintContext, paths: list[Path]
 ) -> Iterable[LintWarning]:
-    def_paths = list(context.root_dir.rglob("**/game_string.def"))
-    defs = [
-        match.group(1)
-        for path in def_paths
-        for match in re.finditer(
-            r"GS_DEFINE\(([A-Z_]+),.*\)", path.read_text()
-        )
-    ]
-    if not defs:
+    project_paths = get_project_paths(context)
+    project_game_strings_paths = get_project_game_strings_paths(context)
+    def_string_map = get_project_game_string_maps(context)
+    if not def_string_map:
+        yield LintWarning("Unable to list game string definitions")
         return
 
-    path_hints = " or ".join(
-        str(path.relative_to(context.root_dir)) for path in def_paths
-    )
-
     for path in paths:
         if path.suffix != ".c":
             continue
+
+        relevant_projects = [get_relevant_project(context, path), "libtrx"]
+        relevant_paths = sum(
+            [
+                project_game_strings_paths[relevant_project]
+                for relevant_project in sorted(relevant_projects)
+            ],
+            [],
+        )
+        path_hint = " or ".join(
+            str(relevant_path.relative_to(context.root_dir))
+            for relevant_path in relevant_paths
+        )
+
         for i, line in enumerate(path.open("r"), 1):
-            for match in re.finditer(r"GS\(([A-Z_]+)\)", line):
+            for match in re.finditer(RE_GAME_STRING_USAGE, line):
                 def_ = match.group(1)
-                if def_ in defs:
+                if any(
+                    def_ in def_string_map[project]
+                    for project in relevant_projects
+                ):
                     continue
 
                 yield LintWarning(
                     path,
                     f"undefined game string: {def_}. "
-                    f"Make sure it's defined in {path_hints}.",
+                    f"Make sure it's defined in {path_hint}.",
                     i,
                 )
 
 
-ALL_LINTERS: list[Callable[[LintContext, Path], Iterable[LintWarning]]] = [
+def lint_unused_game_strings(context: LintContext) -> Iterable[LintWarning]:
+    project_paths = get_project_paths(context)
+    project_game_strings_paths = get_project_game_strings_paths(context)
+    project_game_strings_maps = get_project_game_string_maps(context)
+    if not project_game_strings_maps:
+        yield LintWarning("Unable to list game string definitions")
+        return
+
+    used_strings = defaultdict(set)
+    for path in context.versioned_files:
+        if path.suffix != ".c":
+            continue
+
+        relevant_project = get_relevant_project(context, path)
+        for i, line in enumerate(path.open("r"), 1):
+            for match in re.finditer(RE_GAME_STRING_USAGE, line):
+                used_strings[relevant_project].add(match.group(1))
+
+    for project, defs in project_game_strings_maps.items():
+        relevant_projects = {
+            "libtrx": PROJECTS,
+            "tr1": ["tr1", "libtrx"],
+            "tr2": ["tr2", "libtrx"],
+        }[project]
+        for def_ in defs:
+            used_projects = {rel_project
+                for rel_project in relevant_projects
+                if def_ in used_strings[rel_project]
+            }
+            if len(used_projects) == 0:
+                yield LintWarning(
+                    project_paths[project], f"unused game string: {def_}."
+                )
+            elif project == 'libtrx' and 'libtrx' not in used_projects and len(used_projects) == 1:
+                yield LintWarning(
+                    project_paths[project], f"game string used only in a single child project: {def_} ({used_projects!r}."
+                )
+
+
+ALL_FILE_LINTERS: list[
+    Callable[[LintContext, Path], Iterable[LintWarning]]
+] = [
     lint_json_validity,
     lint_newlines,
     lint_trailing_whitespace,
@@ -113,12 +205,18 @@ ALL_LINTERS: list[Callable[[LintContext, Path], Iterable[LintWarning]]] = [
 ALL_BULK_LINTERS: list[
     Callable[[LintContext, list[Path]], Iterable[LintWarning]]
 ] = [
-    lint_game_strings,
+    lint_undefined_game_strings,
+]
+
+ALL_REPO_LINTERS: list[
+    Callable[[LintContext, list[Path]], Iterable[LintWarning]]
+] = [
+    lint_unused_game_strings,
 ]
 
 
 def lint_file(context: LintContext, file: Path) -> Iterable[LintWarning]:
-    for linter_func in ALL_LINTERS:
+    for linter_func in ALL_FILE_LINTERS:
         yield from linter_func(context, file)
 
 
@@ -127,3 +225,8 @@ def lint_bulk_files(
 ) -> Iterable[LintWarning]:
     for linter_func in ALL_BULK_LINTERS:
         yield from linter_func(context, files)
+
+
+def lint_repo(context: LintContext) -> Iterable[LintWarning]:
+    for linter_func in ALL_REPO_LINTERS:
+        yield from linter_func(context)