code_health: class-size and include-layering checks, base from upstream merge-base

devtools/code_health/README.md

@@ -6,9 +6,14 @@ From the checkout root, create a dedicated environment (Python 3.8+):
 python3 -m venv .venv-code-health
 .venv-code-health/bin/python -m pip install -r devtools/code_health/requirements.txt
 .venv-code-health/bin/python -m unittest discover -s devtools/code_health/tests -v
-.venv-code-health/bin/python -m devtools.code_health --base-ref origin/main
+.venv-code-health/bin/python -m devtools.code_health
 ```
 
+Without `--base-ref`, the base is `git merge-base HEAD upstream/main`, or
+`origin/main` when there is no `upstream` remote, and the command prints the
+base it used. A stale fork `main` therefore cannot report upstream changes
+as this branch's. CI passes the PR base explicitly.
+
 Only added/modified/renamed Git-tracked `.h .hpp .c .cpp .tpp` files under
 `Common/` and `SerialPrograms/` are analyzed. Directory components starting
 with `3rdParty` are excluded. Unchanged and deleted files are not read.
@@ -21,6 +26,9 @@ are included. Missing base revisions and analysis errors fail the command.
 | Function length | 50 physical lines, signature through closing brace; 25 recommended |
 | Nesting | 4 lexical brace levels inside the function body |
 | Parameters | 7 |
+| Public methods per class/struct | 7 (excluding ctors, dtors, operators, deleted and Qt signals); 6-7 advisory |
+| Public non-static data in a class with mutating methods | 0 (exempt: plain structs, value types with only const/static methods, const fields, `*Option`/`*Cell` fields, test files) |
+| Single-method interface (one pure virtual + optional virtual dtor, no bases) | not allowed: prefer a callable (`MoveOnlyFunction` / `std::function`) |
 
 New files/functions must meet these limits. In modified files, compare each
 metric with its value at the base revision: existing violations may remain or
@@ -42,6 +50,22 @@ ignores enclosing namespace/class braces, comments and string literals. This
 is lexical analysis, not a C++ compiler or clang-tidy replacement. Macro-generated
 functions and advanced syntax may require parser follow-up. CCN is not enforced.
 
+Class shapes come from a lexical member scan over lizard's tokens
+(`classes.py`), keyed by qualified class name and compared with the base like
+functions. libclang would need a compilation database with Qt include paths
+to parse these headers; the scan only has to tell access sections, member
+functions and data apart. Classes split across `#if` branches are skipped.
+
+## Layering
+
+Files under `Common/`, `SerialPrograms/Source/CommonFramework/` and
+`SerialPrograms/Source/CommonTools/` may not add an `#include` of a game
+module: `Pokemon/`, `PokemonBDSP/` or `VideoGames/` under
+`SerialPrograms/Source`. Consoles, controllers and frameworks (including
+`NintendoSwitch/`) are platform modules games build on, not games; `config.py`
+lists both groups and a test fails when a new top-level directory is in neither.
+Includes already present at the base are not reported.
+
 ## Build and native C++ tests in one command
 
 The existing `.github/workflows/cpp-ci-serial-programs-base.yml` already runs

devtools/code_health/__main__.py

@@ -1,25 +1,40 @@
-"""Lint changed C++: python -m devtools.code_health --base-ref origin/main."""
+"""Lint changed C++: python -m devtools.code_health [--base-ref REV]."""
 import argparse
 import subprocess
 import sys
 from pathlib import Path
 
-from .git_files import changed_files, read_revision
+from . import layering
+from .classes import measure_classes
+from .git_files import changed_files, default_base, read_revision
 from .metrics import measure
-from .policy import evaluate
+from .policy import advisories, evaluate
 
 
-def run(root, base_ref):
+def analyze(path, source):
+    return measure(path, source) + measure_classes(path, source)
+
+
+def run(root, base_ref=None):
+    if base_ref is None:
+        base_ref, how = default_base(root)
+        print(f'Base: {base_ref} ({how}).')
     changes = changed_files(root, base_ref)
-    failures = []
+    failures, notes = [], []
     advisory = 0
     for current, original in changes:
-        metrics = measure(current, (root / current).read_text(encoding='utf-8-sig'))
-        previous = measure(current, read_revision(root, base_ref, original)) if original else []
+        source = (root / current).read_text(encoding='utf-8-sig')
+        previous_source = read_revision(root, base_ref, original) if original else None
+        metrics = analyze(current, source)
+        previous = analyze(current, previous_source) if original else []
+        failures.extend(layering.check(current, source, previous_source))
         failures.extend(evaluate(metrics, previous))
+        notes.extend(advisories(metrics, previous))
         advisory += sum(m.check == 'file_lines' and 500 < m.value <= 1000 for m in metrics)
     print(f'Scanned {len(changes)} changed C++ files against {base_ref}.')
     print(f'Advisory: {advisory} changed files exceed 500 lines; prefer functions <=25 lines.')
+    for note in notes:
+        print('Advisory: ' + note)
     for failure in failures:
         print(failure, file=sys.stderr)
     print(f'Code health: {len(failures)} failures.')
@@ -29,7 +44,8 @@ def run(root, base_ref):
 def main():
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument('--root', type=Path, default=Path.cwd())
-    parser.add_argument('--base-ref', default='origin/main', help='Compare changed files with this revision')
+    parser.add_argument('--base-ref', help='Compare changed files with this revision (default: '
+                        'merge-base of HEAD and upstream/main, else origin/main)')
     args = parser.parse_args()
     try:
         return run(args.root.resolve(), args.base_ref)