From 5ab5f2c2fa5a46c057caaa9339914605d1874776 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 02:42:05 +0000 Subject: [PATCH 1/5] test(code-metrics): capture the any-exprs table mypy writes on a duplicate-module abort mypy 1.19.1 exits 2 on "Duplicate module named" before analysing anything and still writes an any-exprs.txt whose only data row is Total 0 0 100.00%. The collector fix and its tests replay this capture. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kn6LHhMJfke8gjy8kNdkHk --- .../scripts/fixtures/tool-output/mypy-any-exprs-aborted.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 plugins/code-metrics/scripts/fixtures/tool-output/mypy-any-exprs-aborted.txt diff --git a/plugins/code-metrics/scripts/fixtures/tool-output/mypy-any-exprs-aborted.txt b/plugins/code-metrics/scripts/fixtures/tool-output/mypy-any-exprs-aborted.txt new file mode 100644 index 0000000000..28b326ea38 --- /dev/null +++ b/plugins/code-metrics/scripts/fixtures/tool-output/mypy-any-exprs-aborted.txt @@ -0,0 +1,4 @@ + Name Anys Exprs Coverage +------------------------------- +------------------------------- +Total 0 0 100.00% From f29d37cb031d05f79a74ac208bfa646fd0b6d178 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 03:01:07 +0000 Subject: [PATCH 2/5] fix(code-metrics): stop the mypy collector reporting 100% typed over an aborted run mypy exits 2 on a blocking error (a duplicate module name, a usage or config error) before analysing anything and still writes a report whose only row is `Total 0 0 100.00%`. The collector read that row as a measurement labelled mypy-reported-errors, so a repository carrying sanctioned replication reported 100% typed over zero expressions. - exit 2 is now the adapter contract's exit 4: the Python row reads `unavailable` with mypy's own message (stdout and stderr, since mypy prints error lines to stdout) and the run continues; - a Total row with zero expressions reports type_coverage_pct null; - mypy runs with --explicit-package-bases, so same-named files under identifier-named directories no longer collide (two hyphenated directories still do, because mypy's module walk stops at a non-identifier directory, and reach the unavailable row); - mypy runs with --cache-dir os.devnull, mypy's documented value for disabling the cache, so no .mypy_cache is written into the tree; - the exit-2 test fixture now replays the empty table mypy really writes, plus cases for null on zero expressions, the passed flags, and a real-mypy run leaving no cache; the skill-level suite gains the aborted-run case. Facts verified against mypy's source and docs at v1.19.1 and v2.3.1. Plugin 0.3.0. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kn6LHhMJfke8gjy8kNdkHk --- .../code-metrics/.claude-plugin/plugin.json | 2 +- plugins/code-metrics/CHANGELOG.md | 21 +++ .../scripts/collectors/mypy-report.py | 63 ++++++++- .../scripts/collectors/test_mypy_report.py | 122 ++++++++++++++++-- .../skills/audit-type-debt/SKILL.md | 15 ++- .../scripts/audit-type-debt.test.sh | 33 +++++ 6 files changed, 238 insertions(+), 18 deletions(-) diff --git a/plugins/code-metrics/.claude-plugin/plugin.json b/plugins/code-metrics/.claude-plugin/plugin.json index ea6494395b..2e2645f28d 100644 --- a/plugins/code-metrics/.claude-plugin/plugin.json +++ b/plugins/code-metrics/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "code-metrics", - "version": "0.2.0", + "version": "0.3.0", "description": "Read-only code measures for a change, with cited references and no verdict: lines per file (audit-size), cyclomatic, cognitive, and Halstead complexity (audit-complexity), duplication with sanctioned-replication exclusions (audit-duplication), coverage per function with CRAP from existing lcov, Cobertura, coverage.py, or Go artifacts (audit-coverage), type debt for TypeScript and Python (audit-type-debt), the literacy router for what each number can and cannot say (principles), and a setup skill for the consumer's .claude/code-metrics.yaml. Runs external collectors only when they already resolve, never installs, never runs tests, never emits a finding.", "author": { "name": "Melodic Software", diff --git a/plugins/code-metrics/CHANGELOG.md b/plugins/code-metrics/CHANGELOG.md index a7a0f1402f..2c9089a53a 100644 --- a/plugins/code-metrics/CHANGELOG.md +++ b/plugins/code-metrics/CHANGELOG.md @@ -3,6 +3,27 @@ All notable changes to the `code-metrics` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.3.0] + +### Fixed + +- **`audit-type-debt`: an aborted mypy run no longer reads as 100% typed.** mypy exits 2 on a + blocking error (a duplicate module name, a usage or config error) before analysing anything and + still writes a report whose only row is `Total 0 0 100.00%`; the collector accepted that as a + measurement labelled `mypy-reported-errors`, so a repository carrying sanctioned replication read + as fully typed over zero expressions. Exit 2 is now the adapter contract's exit 4: the Python row + reads `unavailable` with mypy's own message and the run continues. A Total row with zero + expressions reports `type_coverage_pct: null`, never 100. +- **`audit-type-debt`: sanctioned replication measures instead of aborting.** The collector passes + `--explicit-package-bases`, so mypy names each module by its path (`plugins.a.lib.x`) and two + same-named files under identifier-named directories no longer collide. Same-named files under + two hyphenated directories still collide, because mypy's module walk stops at a directory whose + name is not a Python identifier; that case reaches the `unavailable` row above. +- **`audit-type-debt`: no `.mypy_cache/` in the consumer's tree.** The collector passes + `--cache-dir` with the platform's null device, mypy's documented value for disabling the cache; + a one-shot report gained nothing from it (6.8s without a cache against 8.2s with a warm one over + this repository's 179 Python files). + ## [0.2.0] ### Added diff --git a/plugins/code-metrics/scripts/collectors/mypy-report.py b/plugins/code-metrics/scripts/collectors/mypy-report.py index 9522912370..67de313407 100755 --- a/plugins/code-metrics/scripts/collectors/mypy-report.py +++ b/plugins/code-metrics/scripts/collectors/mypy-report.py @@ -11,14 +11,34 @@ report directory is a temporary one, created and removed here, because mypy overwrites the whole directory. -Two facts probed against mypy 1.19.1 in this repository on 2026-09-05, and -replayed by fixtures/tool-output/mypy-any-exprs.txt: +Facts probed against mypy 1.19.1 (its documentation and source at that tag, +unchanged at 2.3.1) and replayed by fixtures/tool-output/mypy-any-exprs.txt and +mypy-any-exprs-aborted.txt: - the table is whitespace-aligned and the Coverage column carries a trailing percent sign; -- mypy exits 1 on any type error and still writes the report (design T1), so a - non-zero exit with a readable report is exit 0 here, with the row labelled - `mypy-reported-errors`. Only an unwritten or unreadable report is exit 3. +- mypy exits 1 on any type error and still writes the report (design T1), so + exit 1 with a readable report is exit 0 here, with the row labelled + `mypy-reported-errors`; +- mypy exits 2 on a blocking error (a duplicate module name, a usage or config + error) before analysing anything, and still writes a report whose only row + is `Total 0 0 100.00%`. Nothing was measured, so exit 2 is the adapter + contract's exit 4 (the tool resolved but cannot run on these files) with + mypy's stderr relayed, never a 100% row. An unwritten or unreadable report on + any other exit is exit 3; +- a Total row with 0 expressions is `type_coverage_pct: null`, because nothing + was counted; mypy's 100.00% for an empty build is not a measurement; +- `--explicit-package-bases` derives each module name from its path relative + to the working directory (or a MYPYPATH entry), so two same-named files under + identifier-named directories (`a/foo.py`, `b/foo.py`) no longer collide. The + walk stops at a directory whose name is not a Python identifier, so + same-named files under two hyphenated directories still collide and reach the + exit-4 path with mypy's message; +- `--cache-dir os.devnull` is mypy's documented "disable caching" value (mypy + compares the option to os.devnull by string equality, `/dev/null` on POSIX + and `nul` on Windows), so no `.mypy_cache` is written into the consumer's + tree; over this repository's 179 files caching saved nothing (6.8s without a + cache against 8.2s with a warm one). The percentage is mypy's own Coverage figure over expressions, which is not the `type-coverage` identifier ratio the TypeScript lane reports. The two are never @@ -40,6 +60,9 @@ TOOL = "mypy" MEASURE = "type_coverage" LANE = "python" +# mypy's exit for a blocking error (duplicate module, usage or config error): +# it stops before analysing anything and its report carries no measurement. +FATAL_EXIT = 2 def probe() -> int: @@ -84,11 +107,37 @@ def collect(lane: str, measure: str, files: list[str]) -> int: report_dir = tempfile.mkdtemp(prefix="code-metrics-mypy-") try: result = subprocess.run( - [exe, "--any-exprs-report", report_dir, "--no-error-summary", *files], + [ + exe, + "--any-exprs-report", + report_dir, + "--no-error-summary", + "--explicit-package-bases", + "--cache-dir", + os.devnull, + *files, + ], capture_output=True, text=True, check=False, ) + if result.returncode == FATAL_EXIT: + # A blocking error stopped mypy before analysis; the report it still + # wrote is empty, so there is no measurement to read. The tool + # resolved but cannot run on these files: the dispatcher writes an + # `unavailable` row carrying this reason (exit 4). + # mypy prints its error lines to stdout and usage errors to + # stderr, so the reason carries both. + said = " ".join( + part.strip().replace("\n", " ") + for part in (result.stdout, result.stderr) + if part.strip() + ) + print( + f"mypy could not analyse these files (exit {FATAL_EXIT}): {said}", + file=sys.stderr, + ) + return 4 report = os.path.join(report_dir, "any-exprs.txt") try: with open(report, encoding="utf-8") as handle: @@ -114,7 +163,7 @@ def collect(lane: str, measure: str, files: list[str]) -> int: "values": { "any_expressions": anys, "expressions_total": exprs, - "type_coverage_pct": coverage, + "type_coverage_pct": coverage if exprs else None, }, "collector": NAME, "labels": ["mypy-reported-errors"] if result.returncode != 0 else [], diff --git a/plugins/code-metrics/scripts/collectors/test_mypy_report.py b/plugins/code-metrics/scripts/collectors/test_mypy_report.py index f0a7e23a79..da09a827e4 100755 --- a/plugins/code-metrics/scripts/collectors/test_mypy_report.py +++ b/plugins/code-metrics/scripts/collectors/test_mypy_report.py @@ -14,6 +14,7 @@ import importlib.util import json import os +import shutil import stat import subprocess import sys @@ -24,6 +25,9 @@ SCRIPT_DIR = Path(__file__).resolve().parent SCRIPT = SCRIPT_DIR / "mypy-report.py" CAPTURE = SCRIPT_DIR.parent / "fixtures" / "tool-output" / "mypy-any-exprs.txt" +# The table mypy 1.19.1 writes when a blocking error (a duplicate module name) +# aborts the build before analysis: no module rows and a Total of 0 over 0. +ABORTED = SCRIPT_DIR.parent / "fixtures" / "tool-output" / "mypy-any-exprs-aborted.txt" SOURCES = "plugins/code-metrics/scripts/fixtures/sources" REPO_ROOT = SCRIPT_DIR.parents[3] @@ -41,18 +45,25 @@ def make_stub( capture: Path | None = CAPTURE, stdout_line: str = "", stderr_line: str = "", + argv_log: Path | None = None, ) -> None: - """Write a `mypy` stub that replays the capture into the report directory.""" + """Write a `mypy` stub that replays the capture into the report directory. + + With `argv_log` the stub also records every argument it received, one per + line, so a test can assert on the flags the adapter passes. + """ copy = ( f'cp "{capture}" "$dir/any-exprs.txt"\n' if capture is not None else "# the report is never written\n" ) + log = f'printf \'%s\\n\' "$@" >"{argv_log}"\n' if argv_log is not None else "" stub = directory / "mypy" stub.write_text( "#!/usr/bin/env bash\n" 'if [[ "${1:-}" == "--version" ]]; then printf \'mypy 1.19.1 (compiled: yes)\\n\'; exit 0; fi\n' - 'dir=""\nprev=""\n' + + log + + 'dir=""\nprev=""\n' 'for arg in "$@"; do\n' ' [[ "$prev" == "--any-exprs-report" ]] && dir="$arg"\n' ' prev="$arg"\n' @@ -65,11 +76,16 @@ def make_stub( stub.chmod(stub.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) -def run(*args: str, path_prefix: Path | None = None) -> subprocess.CompletedProcess: +def run( + *args: str, + path_prefix: Path | None = None, + real_path: bool = False, + cwd: Path | None = None, +) -> subprocess.CompletedProcess: env = dict(os.environ) if path_prefix is not None: env["PATH"] = f"{path_prefix}{os.pathsep}{env.get('PATH', '')}" - else: + elif not real_path: env["PATH"] = str( Path(tempfile.gettempdir()) / "definitely-empty-path-for-mypy-tests" ) @@ -78,7 +94,7 @@ def run(*args: str, path_prefix: Path | None = None) -> subprocess.CompletedProc capture_output=True, text=True, env=env, - cwd=REPO_ROOT, + cwd=cwd or REPO_ROOT, check=False, ) @@ -162,13 +178,38 @@ def test_a_reporting_exit_code_still_yields_a_row_and_a_label(self) -> None: self.assertEqual(row["labels"], ["mypy-reported-errors"]) self.assertEqual(row["values"]["expressions_total"], 13) - def test_no_report_written_is_exit_3_with_the_tool_stderr_relayed(self) -> None: + def test_a_fatal_exit_is_exit_4_with_the_tool_stderr_relayed(self) -> None: + # mypy exits 2 on a blocking error (a duplicate module name, a usage or + # config error) before analysing anything, and still writes a report + # whose Total row reads 0 over 0. Nothing was measured: the tool + # resolved but cannot run on these files, which is the adapter + # contract's exit 4, an `unavailable` row carrying mypy's own message. with tempfile.TemporaryDirectory() as tmp: make_stub( Path(tmp), exit_code=2, + capture=ABORTED, + stderr_line='plugins/b/lib/x.py: error: Duplicate module named "lib.x"', + ) + result = run( + "collect", + "python", + "type_coverage", + f"{SOURCES}/cm_sample.py", + path_prefix=Path(tmp), + ) + self.assertEqual(result.returncode, 4, result.stderr) + self.assertIn("Duplicate module named", result.stderr) + self.assertIn("exit 2", result.stderr) + self.assertEqual(result.stdout, "") + + def test_no_report_written_is_exit_3_with_the_tool_stderr_relayed(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + make_stub( + Path(tmp), + exit_code=0, capture=None, - stderr_line="mypy: error: unrecognized arguments", + stderr_line="mypy: something ate the report directory", ) result = run( "collect", @@ -178,9 +219,74 @@ def test_no_report_written_is_exit_3_with_the_tool_stderr_relayed(self) -> None: path_prefix=Path(tmp), ) self.assertEqual(result.returncode, 3) - self.assertIn("unrecognized arguments", result.stderr) + self.assertIn("ate the report directory", result.stderr) self.assertEqual(result.stdout, "") + def test_zero_expressions_is_null_coverage_never_100(self) -> None: + # A Total of 0 over 0 is "nothing was counted", which the report + # contract renders as null, not as the 100.00% mypy prints. + with tempfile.TemporaryDirectory() as tmp: + make_stub(Path(tmp), exit_code=0, capture=ABORTED) + result = run( + "collect", + "python", + "type_coverage", + f"{SOURCES}/cm_sample.py", + path_prefix=Path(tmp), + ) + self.assertEqual(result.returncode, 0, result.stderr) + row = json.loads(result.stdout.splitlines()[0]) + self.assertEqual( + row["values"], + { + "any_expressions": 0, + "expressions_total": 0, + "type_coverage_pct": None, + }, + ) + + def test_collect_passes_explicit_package_bases_and_a_devnull_cache_dir( + self, + ) -> None: + # --explicit-package-bases derives module names from the path, so two + # same-named files under identifier-named directories no longer abort + # the build; --cache-dir os.devnull is mypy's documented "disable + # caching" value and keeps .mypy_cache out of the consumer's tree. + with tempfile.TemporaryDirectory() as tmp: + argv_log = Path(tmp) / "argv" + make_stub(Path(tmp), argv_log=argv_log) + result = run( + "collect", + "python", + "type_coverage", + f"{SOURCES}/cm_sample.py", + path_prefix=Path(tmp), + ) + self.assertEqual(result.returncode, 0, result.stderr) + argv = argv_log.read_text(encoding="utf-8").splitlines() + self.assertIn("--explicit-package-bases", argv) + self.assertIn("--cache-dir", argv) + self.assertEqual(argv[argv.index("--cache-dir") + 1], os.devnull) + self.assertEqual(argv[-1], f"{SOURCES}/cm_sample.py") + + @unittest.skipUnless(shutil.which("mypy"), "the real mypy is not on PATH") + def test_the_real_mypy_leaves_no_cache_in_the_working_directory(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + source = Path(tmp) / "cm_real.py" + source.write_text("def add(a: int, b: int) -> int:\n return a + b\n") + result = run( + "collect", + "python", + "type_coverage", + "cm_real.py", + real_path=True, + cwd=Path(tmp), + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertFalse((Path(tmp) / ".mypy_cache").exists()) + row = json.loads(result.stdout.splitlines()[0]) + self.assertGreater(row["values"]["expressions_total"], 0) + def test_an_unreadable_table_is_exit_3(self) -> None: with tempfile.TemporaryDirectory() as tmp: garbled = Path(tmp) / "garbled.txt" diff --git a/plugins/code-metrics/skills/audit-type-debt/SKILL.md b/plugins/code-metrics/skills/audit-type-debt/SKILL.md index 407204137a..66c460a182 100644 --- a/plugins/code-metrics/skills/audit-type-debt/SKILL.md +++ b/plugins/code-metrics/skills/audit-type-debt/SKILL.md @@ -56,8 +56,11 @@ your notes and compare by hand. - A value the tool did not produce is `null`, never `0`: `any_count` is `null` when `type-coverage` listed no locations, and `type_coverage_pct` is `null` when nothing was counted at all (a TypeScript project with no `tsconfig.json` reaches this). -- mypy exits non-zero on any type error and still writes its report; the row is kept and labelled - `mypy-reported-errors`, because a type error is not a missing measurement. +- mypy exits 1 on any type error and still writes its report; the row is kept and labelled + `mypy-reported-errors`, because a type error is not a missing measurement. mypy exits 2 when a + blocking error (a duplicate module name, a usage or config error) stops it before analysis; the + Python lane then reads `unavailable` with mypy's own message, never a percentage, and the run + continues. - Exit 0 whenever a report was produced, including a run that measured nothing; exit 2 for a usage error such as an explicitly named path that does not exist; exit 3 when a collector resolved but produced nothing parseable, with its stderr in the run table. @@ -98,3 +101,11 @@ the collectors. covers what mypy followed, not only the files in scope. Compare like-scoped runs. - The Python percentage moves when a dependency ships or drops type stubs, because an unfollowed import turns into `Any`. A drop with no local edit is usually that. +- mypy runs with `--explicit-package-bases`, so a file vendored into several plugins (sanctioned + replication) is named by its path (`plugins.a.lib.x`) and measured once per copy instead of + aborting the lane. mypy's module walk stops at a directory whose name is not a Python + identifier, so two same-named files under two hyphenated directories (`my-pkg/mod.py`, + `other-pkg/mod.py`) still collide; that run reads `unavailable` with the duplicate-module + message. +- mypy runs with its cache disabled (`--cache-dir` set to the platform's null device), so no + `.mypy_cache/` is written into the working tree. A one-shot report gains nothing from the cache. diff --git a/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh b/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh index b5ffa8b41d..4fb66e1ab0 100755 --- a/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh +++ b/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh @@ -20,6 +20,7 @@ PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" SOURCES="$PLUGIN_ROOT/scripts/fixtures/sources" TC_CAPTURE="$PLUGIN_ROOT/scripts/fixtures/tool-output/type-coverage.json" MYPY_CAPTURE="$PLUGIN_ROOT/scripts/fixtures/tool-output/mypy-any-exprs.txt" +MYPY_ABORTED="$PLUGIN_ROOT/scripts/fixtures/tool-output/mypy-any-exprs-aborted.txt" cd "$REPO_ROOT" || exit 2 PY=python3 command -v python3 >/dev/null 2>&1 || PY=python @@ -159,6 +160,38 @@ assert_doc "its reason states that type-coverage needs a resolvable typescript" assert_doc "the python lane still reports while typescript cannot" "$out" \ 'any(r["lane"]=="python" and r["status"]=="ok" for r in d["run"]) and d["status"]=="partial"' +# 3b. mypy resolves but a blocking error (a duplicate module name) stops it +# before analysis: it exits 2 and still writes an empty report. The adapter +# exits 4, the python row reads `unavailable` with mypy's own message rather +# than a 100% measurement, and the run is not a failure. +ABORT_STUBS="$WORK/abort-stubs" +mkdir -p "$ABORT_STUBS" +cp "$STUBS/type-coverage" "$ABORT_STUBS/type-coverage" +cat >"$ABORT_STUBS/mypy" <&2 +exit 2 +EOF +chmod +x "$ABORT_STUBS/mypy" +out="$(cd "$PROJECT" && PATH="$ABORT_STUBS:$EMPTY_PATH" CODE_METRICS_HOME="$HOME_DIR" bash "$SCRIPT" --json --all "$SOURCES")" +rc=$? +assert_eq "exit 0 when mypy aborts before analysis" 0 "$rc" +assert_doc "the python row is unavailable and no python measure is emitted" "$out" \ + 'next(r for r in d["run"] if r["lane"]=="python")["status"]=="unavailable" and not any(r["lane"]=="python" for r in d["measures"])' +assert_doc "its reason carries mypy's own duplicate-module message" "$out" \ + '"Duplicate module named" in next(r for r in d["run"] if r["lane"]=="python")["reason"]' +assert_doc "the typescript lane still reports while mypy cannot" "$out" \ + 'any(r["lane"]=="typescript" and r["status"]=="ok" for r in d["run"]) and d["status"]=="partial"' + # 4. Neither tool present: exit 0, nothing measured, the install hint is named. out="$(cd "$BARE" && PATH="$EMPTY_PATH" CODE_METRICS_HOME="$HOME_DIR" bash "$SCRIPT" --json --all "$SOURCES")" rc=$? From 72dfff25b433967caba5d3b0b160038cdc178981 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 16:18:25 +0000 Subject: [PATCH 3/5] feat(code-metrics): report type debt per file with a lane row and relay mypy's error count audit-type-debt emits one row per scope file plus a lane-total row per lane, so the summary counts files and a change-scoped run reports the scope's own coverage. mypy's module names are mapped back to scope files by re-deriving its explicit-package-bases naming (suffix-matched for a config base such as mypy_path); the TypeScript lane reads the tsconfig program through the project's typescript so a file outside it gets no row rather than a fabricated 0. When mypy exits 1 the run row's reason reads 'mypy reported N errors (M missing stubs)': the dispatcher now relays a succeeding adapter's stderr as the ok row's reason. The renderer keys joins by lane and leads the table with the lane row. Closes #4003 with #4002 in the same PR. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kn6LHhMJfke8gjy8kNdkHk --- plugins/code-metrics/CHANGELOG.md | 31 +++ plugins/code-metrics/README.md | 2 +- .../code-metrics/reference/report-schema.md | 8 +- .../scripts/collectors/mypy-report.py | 243 ++++++++++++++--- .../scripts/collectors/test_mypy_report.py | 246 ++++++++++++++++-- .../scripts/collectors/test_type_coverage.py | 147 ++++++++++- .../scripts/collectors/type-coverage.py | 167 ++++++++++-- plugins/code-metrics/scripts/dispatch.sh | 6 +- plugins/code-metrics/scripts/dispatch.test.sh | 28 ++ .../tool-output/mypy-any-exprs-modules.txt | 7 + .../tool-output/type-coverage-detail.json | 64 +++++ plugins/code-metrics/scripts/report.py | 11 +- plugins/code-metrics/scripts/test_report.py | 120 +++++++++ .../skills/audit-type-debt/SKILL.md | 57 ++-- .../scripts/audit-type-debt.sh | 4 +- .../scripts/audit-type-debt.test.sh | 85 ++++-- 16 files changed, 1108 insertions(+), 118 deletions(-) create mode 100644 plugins/code-metrics/scripts/fixtures/tool-output/mypy-any-exprs-modules.txt create mode 100644 plugins/code-metrics/scripts/fixtures/tool-output/type-coverage-detail.json diff --git a/plugins/code-metrics/CHANGELOG.md b/plugins/code-metrics/CHANGELOG.md index 5587a357de..5ef1c09eec 100644 --- a/plugins/code-metrics/CHANGELOG.md +++ b/plugins/code-metrics/CHANGELOG.md @@ -5,6 +5,37 @@ All notable changes to the `code-metrics` plugin are documented here. Format fol ## [0.3.0] +### Added + +- **`audit-type-debt` reports per file.** One row per scope file the tool listed (`function` + null) plus one row per lane labelled `lane-total`; the summary's `Files:` count is the file + rows, where it read 0 before. The Python lane row sums the file rows, so a change-scoped run + reports the scope's own coverage rather than everything mypy followed. mypy names modules, not + files, so the collector re-derives its `--explicit-package-bases` naming from each scope path + (checked against a real 186-file run of this repository, every listed name matched), matches + the shorter names a config base such as `mypy_path = src` gives by suffix, and, when nothing + matches, keeps mypy's own Total as the lane row and says so in the run row's reason. A + TypeScript file row carries `any_count` alone, the occurrences + `type-coverage --detail --show-relative-path` lists for the file, because the CLI exposes no + per-file denominator; the tsconfig program's file set is read through the project's own + `typescript`, so a scope file the program leaves out gets no row and is counted in the run + row's reason rather than reported as 0. In the raw rows the lane row comes first, and the + rendered table leads with it and never drops it under the row cap. +- **mypy's error count reaches the run table.** When mypy exits 1 the Python run row's reason + reads `mypy reported N errors (M missing stubs)`, the missing ones being the `import-untyped` + and `import-not-found` codes; `--show-error-codes` and `--no-pretty` are passed so a consumer + config that hides codes or wraps messages does not hide the count. + +### Changed + +- **An `ok` run row carries what its collector said on stderr.** The dispatcher dropped an + adapter's stderr on exit 0; it is now the row's reason (500 characters, newlines folded), and + null when the adapter said nothing. Every skill's run table gains this. +- **The renderer sorts a `file: null` row among file rows and joins rows per lane.** The + `lane-total` row and a file row can tie on every earlier sort key, which compared `None` with a + path; and two lanes' rows with the same values used to join into one line, because the join + key left the lane out. + ### Fixed - **`audit-type-debt`: an aborted mypy run no longer reads as 100% typed.** mypy exits 2 on a diff --git a/plugins/code-metrics/README.md b/plugins/code-metrics/README.md index 51da636ab5..3648139785 100644 --- a/plugins/code-metrics/README.md +++ b/plugins/code-metrics/README.md @@ -16,7 +16,7 @@ value to count against, not a bar. | `/code-metrics:audit-size` | Lines per file (total, blank, comment, code through `scc`; total and non-blank from a bundled counter otherwise) beside a cited reference; `size.mode: iso-8.2.115` adds the ISO function-percentage form. | | `/code-metrics:audit-duplication` | Clone groups (duplicated lines and tokens, every instance's range) from `jscpd`, `dupl`, or PMD CPD, minus the replication the repository declares in a sanctioned-replication registry, which is an exclusion, not a suppression. | | `/code-metrics:audit-coverage` | Line coverage per file and per function read from the artifacts a build already produced (lcov 1.x and 2.2, Cobertura, coverage.py JSON, Go cover profile), plus CRAP per function from the complexity rows; it never runs a test, a missing artifact is a visible warning, and a function with no executable lines reports `null`, never zero. | -| `/code-metrics:audit-type-debt` | The typed-code percentage per lane: `type-coverage` for TypeScript, mypy's `--any-exprs-report` for Python; no standard or CWE anchors the measure, so the reference is `null` by design. C# is reported as not applicable. | +| `/code-metrics:audit-type-debt` | The typed-code percentage per file and per lane: `type-coverage` for TypeScript, mypy's `--any-exprs-report` for Python; no standard or CWE anchors the measure, so the reference is `null` by design. C# is reported as not applicable. | | `/code-metrics:principles` | Metric literacy: what each measure can and cannot tell you, where every reference value came from, CRAP's corrected provenance, the cross-metric caveats (carried once, here), and gated pointers to the plugins that own mutation score, tautological tests, dead code, coupling, and lint. | | `/code-metrics:setup` | `check` probes the interpreter, every configuration layer, and every collector; `apply` writes the tracked team configuration per key, idempotently, and never installs a tool. | diff --git a/plugins/code-metrics/reference/report-schema.md b/plugins/code-metrics/reference/report-schema.md index 94221689d1..19e9186bda 100644 --- a/plugins/code-metrics/reference/report-schema.md +++ b/plugins/code-metrics/reference/report-schema.md @@ -45,7 +45,9 @@ different files whatever the registry says, and all of them stay. Clone-group ro ## `run[]` rows `lane`, `measure`, `collector` (the tool and version that produced the rows, or `null`), `status` -(`ok`, `partial`, `unavailable`, `not-applicable`, `deferred`), `reason` (`null` only when `ok`). A +(`ok`, `partial`, `unavailable`, `not-applicable`, `deferred`), `reason` (`null` only when `ok` and +the collector said nothing; an `ok` row whose collector wrote to stderr while succeeding carries +that text, such as mypy's `mypy reported 386 errors (349 missing stubs)`). A run whose scope holds no measurable file carries one row `*/*` with status `not-applicable` and a reason that opens with `no measurable files in scope` and, under `change`, says why: the branch is at its merge-base with a clean working tree (naming the ref and the `--all` alternative), or the @@ -62,7 +64,7 @@ read as complete while one of its own rows says `N of M`. Common fields: `file`, `function` (`null` for a per-file row), `lane`, `values` (measure name to number or `null`), `collector`, `labels` (strings such as `comment-agnostic`, `start-line-only`, -`file-level`, `replicated`), `over_reference` (the measures whose reference the row is at or +`file-level`, `replicated`, `lane-total`), `over_reference` (the measures whose reference the row is at or beyond), and `replicas` on a collapsed row only (see "Sanctioned replication"). Granularity by skill: @@ -72,7 +74,7 @@ skill: | `audit-complexity` | function (`start_line`, `end_line` when the collector reports them) | none | | `audit-coverage` | function | `cov_source` (`artifact-region`, `line-range`, `statement-ratio`, or `ambiguous`), `hit` (the artifact's function-hit flag or `null`), `reason` (why the join was refused; present only on an `ambiguous` row) | | `audit-duplication` | clone group | `instances[]` (`file`, `start_line`, `end_line`) replaces `file` and `function` | -| `audit-type-debt` | lane | `file` and `function` are `null` | +| `audit-type-debt` | file | one row per scope file the tool listed (`function` is `null`) plus one lane row per lane with `file` `null` and the label `lane-total`. The Python lane row sums its file rows, so a change-scoped run reports the scope's own coverage; when no listed module matched a scope file it is mypy's own Total and no file row is emitted. A TypeScript file row carries `any_count` alone (the occurrences `type-coverage --detail` listed for that file; the CLI gives no per-file denominator) with the other three values `null`, and the lane row carries all four | A value the collector did not produce is `null`, never `0`. diff --git a/plugins/code-metrics/scripts/collectors/mypy-report.py b/plugins/code-metrics/scripts/collectors/mypy-report.py index 36185059a1..889d7442de 100755 --- a/plugins/code-metrics/scripts/collectors/mypy-report.py +++ b/plugins/code-metrics/scripts/collectors/mypy-report.py @@ -6,20 +6,44 @@ mypy writes `any-exprs.txt` into the directory given to `--any-exprs-report`: a fixed-width table with the columns Name, Anys, Exprs, Coverage, one row per -module, and a `Total` row. This adapter reads the `Total` row and prints one -per-lane row (`file` and `function` are `null`, per the report contract). The -report directory is a temporary one, created and removed here, because mypy -overwrites the whole directory. +module it was given, and a `Total` row. This adapter prints one row per scope +file whose module mypy listed (`file` set, `function` null) and one lane row +(`file` null, label `lane-total`) whose values are the sum of those file rows, +so a change-scoped run reports the scope's own coverage. The report directory +is a temporary one, created and removed here, because mypy overwrites the +whole directory. Facts probed against mypy 1.19.1 (its documentation and source at that tag, -unchanged at 2.3.1) and replayed by fixtures/tool-output/mypy-any-exprs.txt and -mypy-any-exprs-aborted.txt: +unchanged at 2.3.1) and replayed by fixtures/tool-output/mypy-any-exprs.txt, +mypy-any-exprs-modules.txt and mypy-any-exprs-aborted.txt: - the table is whitespace-aligned and the Coverage column carries a trailing - percent sign; + percent sign; the Name column is the module name, never the path; +- the module name is what `mypy/find_sources.py` (`SourceFinder.crawl_up`) + derives with the working directory as the only explicit base: walking up + from the file's directory, a directory holding `__init__.py[i]` is a + package and its name joins the module; otherwise the walk continues only + while each directory name is a Python identifier and reaches the base, and + stops with no prefix at the first directory whose name is not one. So + `plugins/perf/lib/x.py` is `plugins.perf.lib.x` and `claude-ops/lib/x.py` + is the bare `x`. `module_name` re-derives that rule; checked against a + real 186-file run of this repository (182 of 182 listed names matched); - mypy exits 1 on any type error and still writes the report (design T1), so - exit 1 with a readable report is exit 0 here, with the row labelled - `mypy-reported-errors`; + exit 1 with a readable report is exit 0 here, with the lane row labelled + `mypy-reported-errors` and a note on stderr (which the dispatcher relays as + the run row's reason) counting the errors and the missing-stub ones, the + codes `import-untyped` and `import-not-found`. Error lines go to stdout as + `:: error: []`; `--show-error-codes` is passed so + a consumer config hiding the codes does not hide the count, and + `--no-pretty` so a consumer's `pretty = true` does not wrap the code onto a + continuation line; +- mypy lists only the files it was given: a module it follows through an + import is type-checked for their sake but has no table row, so every listed + module is a scope file under some naming. When the consumer's mypy config + adds a base (`mypy_path = src`, the src layout) mypy names `src/pkg/m.py` + `pkg.m` while the cwd-based derivation says `src.pkg.m`, so a listed name + no derived name equals is matched to the one scope file whose derived name + ends in `.` plus that name, and left out when that is ambiguous; - mypy exits 2 on a blocking error (a duplicate module name, a usage or config error) before analysing anything, and still writes a report whose only row is `Total 0 0 100.00%`. Nothing was measured, so exit 2 is the adapter @@ -54,6 +78,7 @@ import subprocess import sys import tempfile +from typing import Optional from adapter_paths import files_from @@ -65,6 +90,10 @@ # mypy's exit for a blocking error (duplicate module, usage or config error): # it stops before analysing anything and its report carries no measurement. FATAL_EXIT = 2 +# mypy's own order: a stub beside a module wins (find_sources.PY_EXTENSIONS). +PY_EXTENSIONS = (".pyi", ".py") +# The error codes mypy gives an import it found no stubs or implementation for. +MISSING_STUB_CODES = ("[import-untyped]", "[import-not-found]") def probe() -> int: @@ -84,18 +113,117 @@ def probe() -> int: return 0 -def parse_total(table: str) -> tuple[int, int, float]: - """Return (anys, exprs, coverage percent) from the table's `Total` row.""" +# (anys, exprs, coverage percent or None); Optional because this alias is +# evaluated at import time and the floor is Python 3.9. +Counts = tuple[int, int, Optional[float]] + + +def _counts(fields: list[str]) -> Counts: + anys, exprs = int(fields[1]), int(fields[2]) + # mypy prints 100.00% for an empty module; nothing was counted, so the + # percentage is null, never 100. + return anys, exprs, float(fields[3].rstrip("%")) if exprs else None + + +def parse_table(table: str) -> tuple[dict[str, Counts], Counts]: + """Return ({module: counts}, total counts) from the any-exprs table.""" + modules: dict[str, Counts] = {} + total: Counts | None = None for line in table.splitlines(): fields = line.split() - if len(fields) != 4 or fields[0] != "Total": + if len(fields) != 4 or fields[0] == "Name" or not fields[1].isdigit(): continue - return ( - int(fields[1]), - int(fields[2]), - float(fields[3].rstrip("%")), - ) - raise ValueError("the any-exprs report has no Total row") + if fields[0] == "Total": + total = _counts(fields) + else: + modules[fields[0]] = _counts(fields) + if total is None: + raise ValueError("the any-exprs report has no Total row") + return modules, total + + +def parse_total(table: str) -> Counts: + """The table's `Total` row alone.""" + return parse_table(table)[1] + + +def _has_init(directory: str) -> bool: + return any( + os.path.isfile(os.path.join(directory, "__init__" + ext)) + for ext in PY_EXTENSIONS + ) + + +def _join(prefix: str, name: str) -> str: + return f"{prefix}.{name}" if prefix else name + + +def _crawl_dir(directory: str, base: str) -> str | None: + """The module prefix a directory contributes, or None when the walk + reaches neither the base nor a package (mypy's `_crawl_up_helper`).""" + if os.path.normcase(directory) == base: + return "" + parent, name = os.path.split(directory) + if not name or parent == directory: + return None + if name.endswith("-stubs"): + name = name[:-6] + if _has_init(directory): + # A package is always named, whatever lies above it; mypy refuses a + # package whose directory name is not an identifier (exit 2, the + # exit-4 path here), so the file cannot reach this point. + return _join(_crawl_dir(parent, base) or "", name) + if not name.isidentifier(): + return None + prefix = _crawl_dir(parent, base) + return None if prefix is None else _join(prefix, name) + + +def module_name(path: str, base: str) -> str: + """The module mypy names `path` under `--explicit-package-bases` with + `base` (an absolute, normcased directory) as the only base.""" + parent, filename = os.path.split(os.path.abspath(path)) + stem = filename + for ext in PY_EXTENSIONS: + if filename.endswith(ext): + stem = filename[: -len(ext)] + break + prefix = _crawl_dir(parent, base) or "" + return prefix if stem == "__init__" else _join(prefix, stem) + + +def error_note(stdout: str) -> str: + """`mypy reported N errors (M missing stubs)` from mypy's error lines.""" + lines = [line for line in stdout.splitlines() if ": error:" in line] + stubs = [line for line in lines if line.rstrip().endswith(MISSING_STUB_CODES)] + return ( + f"mypy reported {len(lines)} error{'s' if len(lines) != 1 else ''} " + f"({len(stubs)} missing stub{'s' if len(stubs) != 1 else ''})" + ) + + +def _row(lane: str, file: str | None, counts: Counts, labels: list[str]) -> dict: + anys, exprs, coverage = counts + return { + "file": file, + "function": None, + "lane": lane, + "values": { + "any_expressions": anys, + "expressions_total": exprs, + "type_coverage_pct": coverage, + }, + "collector": NAME, + "labels": labels, + } + + +def _summed(rows: list[Counts]) -> Counts: + anys = sum(r[0] for r in rows) + exprs = sum(r[1] for r in rows) + # mypy's own formatting of the percentage, so a lane row over every + # listed module reads exactly as mypy's Total row does. + return anys, exprs, float(f"{(exprs - anys) / exprs * 100:.2f}") if exprs else None def collect(lane: str, measure: str, files: list[str]) -> int: @@ -114,6 +242,8 @@ def collect(lane: str, measure: str, files: list[str]) -> int: "--any-exprs-report", report_dir, "--no-error-summary", + "--show-error-codes", + "--no-pretty", "--explicit-package-bases", "--cache-dir", os.devnull, @@ -154,26 +284,75 @@ def collect(lane: str, measure: str, files: list[str]) -> int: finally: shutil.rmtree(report_dir, ignore_errors=True) try: - anys, exprs, coverage = parse_total(table) + modules, total = parse_table(table) except ValueError as exc: print(f"{NAME}.py: unparsable report ({exc})", file=sys.stderr) return 3 - row = { - "file": None, - "function": None, - "lane": lane, - "values": { - "any_expressions": anys, - "expressions_total": exprs, - "type_coverage_pct": coverage if exprs else None, - }, - "collector": NAME, - "labels": ["mypy-reported-errors"] if result.returncode != 0 else [], - } - print(json.dumps(row)) + matched = match_modules(modules, files) + notes: list[str] = [] + if result.returncode != 0: + notes.append(error_note(result.stdout)) + labels = ["lane-total"] + (["mypy-reported-errors"] if result.returncode else []) + by_path = {path: name for name, path in matched.items()} + # File rows in scope order, whatever order the names matched in. + file_rows = [_row(lane, p, modules[by_path[p]], []) for p in files if p in by_path] + if file_rows: + lane_counts = _summed([modules[name] for name in matched]) + left_out = len(modules) - len(matched) + if left_out: + notes.append( + f"{left_out} listed module(s) matched no scope file and are not " + "counted in the lane row" + ) + else: + # Nothing listed matched (mypy named its modules from a base this + # adapter cannot recover), or nothing was listed at all: the lane row + # is mypy's own Total. + lane_counts = total + if modules: + notes.append( + f"none of the {len(modules)} listed module(s) matched a scope " + "file; the lane row is mypy's own Total and no file rows are emitted" + ) + unlisted = len(files) - len(matched) + if unlisted and modules: + notes.append(f"{unlisted} scope file(s) mypy did not list") + # The lane row leads, so the lane's figure is the first row a reader of + # the raw rows meets. + for row in [_row(lane, None, lane_counts, labels), *file_rows]: + print(json.dumps(row)) + if notes: + print("; ".join(notes), file=sys.stderr) return 0 +def match_modules(modules: dict[str, Counts], files: list[str]) -> dict[str, str]: + """Map each listed module to the scope file it names: by the cwd-based + derivation first, then, for a listed name no derived name equals, the one + scope file whose derived name ends in `.` plus it (a base the consumer's + config added, such as `mypy_path = src`); an ambiguous suffix matches + nothing.""" + base = os.path.normcase(os.path.abspath(os.getcwd())) + derived = [(module_name(path, base), path) for path in files] + matched: dict[str, str] = {} + for name, path in derived: + if name in modules and name not in matched: + matched[name] = path + taken = set(matched.values()) + for listed in modules: + if listed in matched: + continue + candidates = [ + path + for name, path in derived + if path not in taken and name.endswith("." + listed) + ] + if len(candidates) == 1: + matched[listed] = candidates[0] + taken.add(candidates[0]) + return matched + + def main(argv: list[str]) -> int: if not argv: print( diff --git a/plugins/code-metrics/scripts/collectors/test_mypy_report.py b/plugins/code-metrics/scripts/collectors/test_mypy_report.py index da09a827e4..7dc0d24d42 100755 --- a/plugins/code-metrics/scripts/collectors/test_mypy_report.py +++ b/plugins/code-metrics/scripts/collectors/test_mypy_report.py @@ -28,6 +28,13 @@ # The table mypy 1.19.1 writes when a blocking error (a duplicate module name) # aborts the build before analysis: no module rows and a Total of 0 over 0. ABORTED = SCRIPT_DIR.parent / "fixtures" / "tool-output" / "mypy-any-exprs-aborted.txt" +# A real mypy 1.19.1 table over `pkg/a.py`, `pkg-x/b.py` and `c.py` run from +# their parent directory: a dotted module, a bare stem under a hyphenated +# directory (the walk stops there), and a plain top-level module. +MODULES = SCRIPT_DIR.parent / "fixtures" / "tool-output" / "mypy-any-exprs-modules.txt" +STUB_ERROR = ( + 'pkg-x/b.py:1: error: Library stubs not installed for "yaml" [import-untyped]' +) SOURCES = "plugins/code-metrics/scripts/fixtures/sources" REPO_ROOT = SCRIPT_DIR.parents[3] @@ -112,8 +119,15 @@ def test_probe_prints_the_version_when_the_stub_resolves(self) -> None: self.assertEqual((result.returncode, result.stdout.strip()), (0, "1.19.1")) +def rows_of(result: subprocess.CompletedProcess) -> list[dict]: + return [json.loads(line) for line in result.stdout.splitlines()] + + class MypyReportCollectTests(unittest.TestCase): - def test_collect_reads_the_total_row_into_one_per_lane_row(self) -> None: + def test_collect_prints_a_row_per_scope_file_and_the_lane_row(self) -> None: + # The capture lists `cm_sample`, the bare stem mypy gives a file under + # a hyphenated directory (`code-metrics`): a file row for the scope + # path plus the lane row summing it. with tempfile.TemporaryDirectory() as tmp: make_stub(Path(tmp)) result = run( @@ -124,21 +138,126 @@ def test_collect_reads_the_total_row_into_one_per_lane_row(self) -> None: path_prefix=Path(tmp), ) self.assertEqual(result.returncode, 0, result.stderr) - rows = [json.loads(line) for line in result.stdout.splitlines()] - self.assertEqual(len(rows), 1) - row = rows[0] - self.assertEqual((row["file"], row["function"]), (None, None)) - self.assertEqual(row["lane"], "python") - self.assertEqual(row["collector"], "mypy-report") + rows = rows_of(result) + self.assertEqual(len(rows), 2) + # The lane row leads: the lane's figure is the first row. + lane_row, file_row = rows + values = { + "any_expressions": 0, + "expressions_total": 13, + "type_coverage_pct": 100.0, + } + self.assertEqual( + (file_row["file"], file_row["function"], file_row["labels"]), + (f"{SOURCES}/cm_sample.py", None, []), + ) + self.assertEqual(file_row["values"], values) + self.assertEqual((lane_row["file"], lane_row["function"]), (None, None)) + self.assertEqual(lane_row["lane"], "python") + self.assertEqual(lane_row["collector"], "mypy-report") + self.assertEqual(lane_row["values"], values) + self.assertEqual(lane_row["labels"], ["lane-total"]) + self.assertEqual(result.stderr, "") + + def _collect_modules( + self, tmp: str, *files: str, exit_code: int = 1 + ) -> subprocess.CompletedProcess: + make_stub( + Path(tmp), + exit_code=exit_code, + capture=MODULES, + stdout_line=STUB_ERROR if exit_code else "", + ) + return run( + "collect", + "python", + "type_coverage", + *files, + path_prefix=Path(tmp), + cwd=Path(tmp), + ) + + def test_module_rows_map_to_scope_files_and_the_lane_row_sums_them(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + result = self._collect_modules(tmp, "pkg/a.py", "pkg-x/b.py", "c.py") + self.assertEqual(result.returncode, 0, result.stderr) + rows = rows_of(result) self.assertEqual( - row["values"], + [(r["file"], tuple(r["values"].values())) for r in rows], + [ + (None, (8, 26, 69.23)), + ("pkg/a.py", (5, 9, 44.44)), + ("pkg-x/b.py", (3, 11, 72.73)), + ("c.py", (0, 6, 100.0)), + ], + ) + self.assertEqual(rows[0]["labels"], ["lane-total", "mypy-reported-errors"]) + self.assertEqual([r["labels"] for r in rows[1:]], [[], [], []]) + + def test_a_src_layout_matches_the_shorter_names_a_config_base_gives(self) -> None: + # `mypy_path = src` makes mypy name src/pkg/a.py `pkg.a` while the + # cwd-based derivation says `src.pkg.a`: the listed name is matched to + # the one scope file whose derived name ends in it. + with tempfile.TemporaryDirectory() as tmp: + result = self._collect_modules( + tmp, "src/pkg/a.py", "src/pkg-x/b.py", "src/c.py", exit_code=0 + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual( + [r["file"] for r in rows_of(result)], + [None, "src/pkg/a.py", "src/pkg-x/b.py", "src/c.py"], + ) + self.assertEqual(result.stderr, "") + + def test_the_lane_row_covers_the_scope_and_names_what_it_left_out(self) -> None: + # A change scope of two files: the lane row is those two files' + # coverage, and the module the table lists for neither is noted. + with tempfile.TemporaryDirectory() as tmp: + result = self._collect_modules(tmp, "pkg/a.py", "c.py", exit_code=0) + self.assertEqual(result.returncode, 0, result.stderr) + rows = rows_of(result) + self.assertEqual([r["file"] for r in rows], [None, "pkg/a.py", "c.py"]) + self.assertEqual( + rows[0]["values"], { - "any_expressions": 0, - "expressions_total": 13, - "type_coverage_pct": 100.0, + "any_expressions": 5, + "expressions_total": 15, + "type_coverage_pct": 66.67, }, ) - self.assertEqual(row["labels"], []) + self.assertEqual(rows[0]["labels"], ["lane-total"]) + self.assertEqual( + result.stderr.strip(), + "1 listed module(s) matched no scope file and are not counted in the lane row", + ) + + def test_a_scope_file_mypy_did_not_list_is_counted_in_the_note(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + result = self._collect_modules( + tmp, "pkg/a.py", "pkg-x/b.py", "c.py", "extra/z.py", exit_code=0 + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(len(rows_of(result)), 4) + self.assertEqual(result.stderr.strip(), "1 scope file(s) mypy did not list") + + def test_no_match_at_all_falls_back_to_the_total_row_and_says_so(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + result = self._collect_modules(tmp, "elsewhere/z.py", exit_code=0) + self.assertEqual(result.returncode, 0, result.stderr) + rows = rows_of(result) + self.assertEqual(len(rows), 1) + self.assertEqual( + (rows[0]["file"], tuple(rows[0]["values"].values())), + (None, (8, 26, 69.23)), + ) + self.assertIn("none of the 3 listed module(s) matched", result.stderr) + + def test_an_exit_1_note_counts_the_errors_and_the_missing_stubs(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + result = self._collect_modules(tmp, "pkg/a.py", "pkg-x/b.py", "c.py") + self.assertEqual( + result.stderr.strip(), "mypy reported 1 error (1 missing stub)" + ) def test_collect_removes_the_report_directory_it_asked_mypy_for(self) -> None: with tempfile.TemporaryDirectory() as tmp: @@ -174,9 +293,12 @@ def test_a_reporting_exit_code_still_yields_a_row_and_a_label(self) -> None: path_prefix=Path(tmp), ) self.assertEqual(result.returncode, 0, result.stderr) - row = json.loads(result.stdout.splitlines()[0]) - self.assertEqual(row["labels"], ["mypy-reported-errors"]) - self.assertEqual(row["values"]["expressions_total"], 13) + lane_row = rows_of(result)[0] + self.assertEqual(lane_row["labels"], ["lane-total", "mypy-reported-errors"]) + self.assertEqual(lane_row["values"]["expressions_total"], 13) + self.assertEqual( + result.stderr.strip(), "mypy reported 1 error (0 missing stubs)" + ) def test_a_fatal_exit_is_exit_4_with_the_tool_stderr_relayed(self) -> None: # mypy exits 2 on a blocking error (a duplicate module name, a usage or @@ -235,9 +357,11 @@ def test_zero_expressions_is_null_coverage_never_100(self) -> None: path_prefix=Path(tmp), ) self.assertEqual(result.returncode, 0, result.stderr) - row = json.loads(result.stdout.splitlines()[0]) + rows = rows_of(result) + # An empty table lists no module, so the lane row is the only row. + self.assertEqual(len(rows), 1) self.assertEqual( - row["values"], + rows[0]["values"], { "any_expressions": 0, "expressions_total": 0, @@ -265,6 +389,8 @@ def test_collect_passes_explicit_package_bases_and_a_devnull_cache_dir( self.assertEqual(result.returncode, 0, result.stderr) argv = argv_log.read_text(encoding="utf-8").splitlines() self.assertIn("--explicit-package-bases", argv) + self.assertIn("--show-error-codes", argv) + self.assertIn("--no-pretty", argv) self.assertIn("--cache-dir", argv) self.assertEqual(argv[argv.index("--cache-dir") + 1], os.devnull) self.assertEqual(argv[-1], f"{SOURCES}/cm_sample.py") @@ -284,8 +410,10 @@ def test_the_real_mypy_leaves_no_cache_in_the_working_directory(self) -> None: ) self.assertEqual(result.returncode, 0, result.stderr) self.assertFalse((Path(tmp) / ".mypy_cache").exists()) - row = json.loads(result.stdout.splitlines()[0]) - self.assertGreater(row["values"]["expressions_total"], 0) + rows = rows_of(result) + self.assertEqual([r["file"] for r in rows], [None, "cm_real.py"]) + self.assertGreater(rows[1]["values"]["expressions_total"], 0) + self.assertEqual(rows[0]["values"], rows[1]["values"]) def test_an_unreadable_table_is_exit_3(self) -> None: with tempfile.TemporaryDirectory() as tmp: @@ -315,12 +443,92 @@ def test_the_total_row_is_read_from_a_table_with_several_modules(self) -> None: " Total 3 16 81.25%\n" ) self.assertEqual(module.parse_total(table), (3, 16, 81.25)) + modules, total = module.parse_table(table) + self.assertEqual(total, (3, 16, 81.25)) + self.assertEqual( + modules, {"cm_first": (3, 4, 25.0), "cm_second": (0, 12, 100.0)} + ) def test_a_table_without_a_total_row_is_a_value_error(self) -> None: module = load_module() with self.assertRaises(ValueError): module.parse_total(" Name Anys Exprs Coverage\n") + def test_the_lane_row_percentage_is_formatted_as_mypy_formats_it(self) -> None: + module = load_module() + # This repository's whole-tree Total: mypy prints 89.96% for it. + self.assertEqual( + module._summed([(14961, 149042, 89.96)]), (14961, 149042, 89.96) + ) + self.assertEqual(module._summed([(0, 0, None)]), (0, 0, None)) + + +class MypyReportModuleNameTests(unittest.TestCase): + """`module_name` re-derives mypy's crawl with the cwd as the only base.""" + + def test_names_follow_mypy_under_explicit_package_bases(self) -> None: + module = load_module() + with tempfile.TemporaryDirectory() as tmp: + base = os.path.normcase(os.path.realpath(tmp)) + for rel in ( + "plugins/perf/lib/x.py", + "claude-ops/lib/x.py", + "pkg/sub/__init__.py", + "pkg/sub/c.pyi", + "top.py", + "dir-with-dash/deeper/__init__.py", + "dir-with-dash/deeper/leaf.py", + ): + path = Path(base) / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.touch() + cases = { + # every directory up to the base is an identifier: the dotted path + "plugins/perf/lib/x.py": "plugins.perf.lib.x", + # the walk stops at a non-identifier directory: the stem alone + "claude-ops/lib/x.py": "x", + # a package names its directory; a stub keeps its stem + "pkg/sub/__init__.py": "pkg.sub", + "pkg/sub/c.pyi": "pkg.sub.c", + "top.py": "top", + # a package under a non-identifier directory is still a package + "dir-with-dash/deeper/__init__.py": "deeper", + "dir-with-dash/deeper/leaf.py": "deeper.leaf", + } + for rel, expected in cases.items(): + with self.subTest(rel=rel): + self.assertEqual( + module.module_name(os.path.join(base, rel), base), expected + ) + + def test_a_file_outside_the_base_is_its_stem(self) -> None: + module = load_module() + with tempfile.TemporaryDirectory() as tmp: + outside = Path(tmp) / "somewhere" / "else" / "mod.py" + outside.parent.mkdir(parents=True) + outside.touch() + base = os.path.normcase(os.path.realpath(Path(tmp) / "base")) + self.assertEqual(module.module_name(str(outside), base), "mod") + + +class MypyReportErrorNoteTests(unittest.TestCase): + def test_the_note_counts_error_lines_and_missing_stub_codes(self) -> None: + module = load_module() + stdout = "\n".join( + [ + 'a.py:1: error: Library stubs not installed for "yaml" [import-untyped]', + 'a.py:1: note: Hint: "python3 -m pip install types-PyYAML"', + 'b.py:2: error: Cannot find implementation or library stub for module named "x" [import-not-found]', + "c.py:3: error: Incompatible return value type [return-value]", + ] + ) + self.assertEqual( + module.error_note(stdout), "mypy reported 3 errors (2 missing stubs)" + ) + self.assertEqual( + module.error_note(""), "mypy reported 0 errors (0 missing stubs)" + ) + class MypyReportVerbTests(unittest.TestCase): def test_other_verbs(self) -> None: diff --git a/plugins/code-metrics/scripts/collectors/test_type_coverage.py b/plugins/code-metrics/scripts/collectors/test_type_coverage.py index 23e48d45fb..c393ef4a6d 100755 --- a/plugins/code-metrics/scripts/collectors/test_type_coverage.py +++ b/plugins/code-metrics/scripts/collectors/test_type_coverage.py @@ -23,23 +23,55 @@ SCRIPT_DIR = Path(__file__).resolve().parent SCRIPT = SCRIPT_DIR / "type-coverage.py" CAPTURE = SCRIPT_DIR.parent / "fixtures" / "tool-output" / "type-coverage.json" +# A real `--detail --json-output --show-relative-path` capture (2.30.1, +# typescript 5.9) over a scratch project: src/a.ts and src/sub/b.ts carry +# any-typed identifiers, src/clean.ts none, and other/outside.ts sits outside +# the tsconfig's `include`, so the tool never names it. +DETAIL = SCRIPT_DIR.parent / "fixtures" / "tool-output" / "type-coverage-detail.json" +DETAIL_SCOPE = ("src/a.ts", "src/sub/b.ts", "src/clean.ts", "other/outside.ts") SOURCES = "plugins/code-metrics/scripts/fixtures/sources" REPO_ROOT = SCRIPT_DIR.parents[3] NO_TYPESCRIPT = "type-coverage needs a resolvable typescript (the probe found none)" -def write_stub(path: Path, capture: Path = CAPTURE, exit_code: int = 0) -> None: +def write_stub( + path: Path, + capture: Path = CAPTURE, + exit_code: int = 0, + argv_log: Path | None = None, +) -> None: + """A `type-coverage` stub replaying `capture`; with `argv_log` it also + records every argument it received, one per line.""" path.parent.mkdir(parents=True, exist_ok=True) + log = f'printf \'%s\\n\' "$@" >"{argv_log}"\n' if argv_log is not None else "" path.write_text( "#!/usr/bin/env bash\n" 'if [[ "${1:-}" == "--version" ]]; then printf \'Version: 2.30.1\\n\'; exit 0; fi\n' - f'cat "{capture}"\n' + + log + + f'cat "{capture}"\n' f"exit {exit_code}\n", encoding="utf-8", ) path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) +def write_node_stub( + path: Path, program: list[str] | None, exit_code: int = 0, stderr: str = "" +) -> None: + """A `node` stub standing in for the tsconfig-program listing: prints + `program` as JSON (`null` for no tsconfig), or fails with `stderr`.""" + path.parent.mkdir(parents=True, exist_ok=True) + listing = json.dumps(program) if program is not None else "null" + path.write_text( + "#!/usr/bin/env bash\n" + f"printf '%s\\n' '{listing}'\n" + + (f"printf '%s\\n' '{stderr}' >&2\n" if stderr else "") + + f"exit {exit_code}\n", + encoding="utf-8", + ) + path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + + def make_project(root: Path, with_typescript: bool = True, local_stub: bool = False): """A scratch cwd: optional node_modules/typescript and a local binary.""" if with_typescript: @@ -115,6 +147,7 @@ class TypeCoverageCollectTests(unittest.TestCase): def _collect(self, tmp: str, capture: Path = CAPTURE, exit_code: int = 0): stubs = Path(tmp) / "bin" write_stub(stubs / "type-coverage", capture=capture, exit_code=exit_code) + write_node_stub(stubs / "node", [f"{SOURCES}/cm-sample.ts"]) return run( "collect", "typescript", @@ -123,18 +156,38 @@ def _collect(self, tmp: str, capture: Path = CAPTURE, exit_code: int = 0): path_prefix=stubs, ) - def test_collect_translates_the_capture_into_one_per_lane_row(self) -> None: + def test_collect_translates_the_capture_into_a_file_row_and_the_lane_row( + self, + ) -> None: with tempfile.TemporaryDirectory() as tmp: result = self._collect(tmp) self.assertEqual(result.returncode, 0, result.stderr) rows = [json.loads(line) for line in result.stdout.splitlines()] - self.assertEqual(len(rows), 1) - row = rows[0] - self.assertEqual((row["file"], row["function"]), (None, None)) - self.assertEqual(row["lane"], "typescript") - self.assertEqual(row["collector"], "type-coverage") + self.assertEqual(len(rows), 2) + # The lane row leads: the lane's figure is the first row. + lane_row, file_row = rows + self.assertEqual(result.stderr, "") self.assertEqual( - row["values"], + (file_row["file"], file_row["function"], file_row["labels"]), + (f"{SOURCES}/cm-sample.ts", None, []), + ) + # The CLI gives no per-file denominator: a file row carries the + # occurrences listed for it and nothing else. + self.assertEqual( + file_row["values"], + { + "type_coverage_pct": None, + "typed_identifiers": None, + "total_identifiers": None, + "any_count": 4, + }, + ) + self.assertEqual((lane_row["file"], lane_row["function"]), (None, None)) + self.assertEqual(lane_row["lane"], "typescript") + self.assertEqual(lane_row["collector"], "type-coverage") + self.assertEqual(lane_row["labels"], ["lane-total"]) + self.assertEqual( + lane_row["values"], { "type_coverage_pct": 55.55, "typed_identifiers": 5, @@ -143,6 +196,74 @@ def test_collect_translates_the_capture_into_one_per_lane_row(self) -> None: }, ) + def test_file_rows_group_the_listed_occurrences_by_scope_file(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + stubs = Path(tmp) / "bin" + argv_log = Path(tmp) / "argv" + write_stub(stubs / "type-coverage", capture=DETAIL, argv_log=argv_log) + # The program holds the three files under src/, as the real + # listing over the scratch project's tsconfig did. + write_node_stub( + stubs / "node", ["src/a.ts", "src/clean.ts", "src/sub/b.ts"] + ) + result = run( + "collect", + "typescript", + "type_coverage", + *DETAIL_SCOPE, + path_prefix=stubs, + ) + self.assertEqual(result.returncode, 0, result.stderr) + rows = [json.loads(line) for line in result.stdout.splitlines()] + self.assertEqual( + [(r["file"], r["values"]["any_count"]) for r in rows], + [ + (None, 9), + ("src/a.ts", 5), + ("src/sub/b.ts", 4), + # in the program with no listed occurrence: a measured 0 + ("src/clean.ts", 0), + # other/outside.ts is outside the program: no row + ], + ) + self.assertEqual(rows[0]["values"]["type_coverage_pct"], 57.14) + self.assertEqual(rows[0]["labels"], ["lane-total"]) + self.assertEqual( + result.stderr.strip(), + "1 scope file(s) are outside the tsconfig program and were not " + "measured: other/outside.ts", + ) + argv = argv_log.read_text(encoding="utf-8").splitlines() + self.assertEqual( + argv[:5], + ["--detail", "--json-output", "--show-relative-path", "--", "src/a.ts"], + ) + + def test_an_unreadable_program_keeps_only_the_listed_files(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + stubs = Path(tmp) / "bin" + write_stub(stubs / "type-coverage", capture=DETAIL) + write_node_stub( + stubs / "node", None, exit_code=1, stderr="Cannot find module" + ) + result = run( + "collect", + "typescript", + "type_coverage", + *DETAIL_SCOPE, + path_prefix=stubs, + ) + self.assertEqual(result.returncode, 0, result.stderr) + rows = [json.loads(line) for line in result.stdout.splitlines()] + self.assertEqual( + [r["file"] for r in rows], [None, "src/a.ts", "src/sub/b.ts"] + ) + self.assertEqual( + result.stderr.strip(), + "the tsconfig program could not be read (Cannot find module); " + "only scope files with a listed occurrence have a row", + ) + def test_a_reporting_exit_code_still_yields_a_row(self) -> None: # --at-least makes the tool exit non-zero while still printing its JSON. with tempfile.TemporaryDirectory() as tmp: @@ -182,9 +303,11 @@ def test_a_null_percent_stays_null_rather_than_becoming_zero(self) -> None: ) result = self._collect(tmp, capture=empty) self.assertEqual(result.returncode, 0, result.stderr) - row = json.loads(result.stdout.splitlines()[0]) - self.assertIsNone(row["values"]["type_coverage_pct"]) - self.assertEqual(row["values"]["any_count"], 0) + rows = [json.loads(line) for line in result.stdout.splitlines()] + # Nothing counted: no file row either, only the lane row. + self.assertEqual(len(rows), 1) + self.assertIsNone(rows[0]["values"]["type_coverage_pct"]) + self.assertEqual(rows[0]["values"]["any_count"], 0) def test_unparsable_output_is_exit_3(self) -> None: with tempfile.TemporaryDirectory() as tmp: diff --git a/plugins/code-metrics/scripts/collectors/type-coverage.py b/plugins/code-metrics/scripts/collectors/type-coverage.py index 2b903fa28e..a996d248f7 100755 --- a/plugins/code-metrics/scripts/collectors/type-coverage.py +++ b/plugins/code-metrics/scripts/collectors/type-coverage.py @@ -14,8 +14,29 @@ - `--json-output` is a boolean flag: the JSON goes to stdout, with `correctCount`, `totalCount`, `percent` (`null` when nothing was counted), and, only when `--detail` is also passed, `details[]`, one entry per `any` - location. So `collect` runs `type-coverage --detail --json-output -- ` - and `any_count` is `null` when the tool listed no locations at all. + location (`filePath`, 0-based `line` and `character`, the identifier + `text`). So `collect` runs + `type-coverage --detail --json-output --show-relative-path -- ` and + the lane row's `any_count` is `null` when the tool listed no locations at + all. +- `--show-relative-path` makes `filePath` relative to the working directory + (the tool resolves it against the cwd otherwise), which is how the + dispatcher's scope paths are written; both sides are compared as absolute, + normcased paths, so a Windows `filePath` with backslashes still matches. +- the CLI exposes no per-file denominator (its core's `fileCounts` option is + never passed), so a file row carries `any_count` alone, the occurrences + listed for that file, with the other three values `null`. +- the tool counts only the files of its `tsconfig.json` program and does not + say which those are (verified on 2.30.1: a file outside `include` + contributes nothing and is not named), so the program's file set is read + through the project's own `typescript` (one `node` call: `findConfigFile`, + `parseJsonConfigFileContent`, `createProgram`, the source files outside + `node_modules`; this is the set core's `lint` iterates). A scope file in the + program with no listed occurrence reads `any_count: 0`, a measured zero; a + scope file outside it gets no row and is counted in a stderr note the + dispatcher relays as the run row's reason. When that call fails, only scope + files with a listed occurrence get a row and the note says why. No file + rows are emitted when `totalCount` is 0. - files after `--` restrict the run to those files, so the row is the scope the dispatcher asked for rather than a project-wide figure. The tool still reads the project's `tsconfig.json`; without one it counts nothing and reports @@ -49,6 +70,19 @@ LANE = "typescript" LOCAL_BIN = os.path.join("node_modules", ".bin", "type-coverage") NO_TYPESCRIPT = "type-coverage needs a resolvable typescript (the probe found none)" +# Prints the tsconfig program's source files (absolute paths, node_modules +# excluded) as a JSON array, or `null` when no tsconfig.json resolves from the +# working directory, through the same `typescript` the tool itself uses. +PROGRAM_SCRIPT = """ +const ts = require('typescript'); +const path = require('path'); +const config = ts.findConfigFile(process.cwd(), ts.sys.fileExists); +if (!config) { console.log('null'); process.exit(0); } +const read = ts.readConfigFile(config, ts.sys.readFile); +const parsed = ts.parseJsonConfigFileContent(read.config || {}, ts.sys, path.dirname(config)); +const program = ts.createProgram(parsed.fileNames, parsed.options); +console.log(JSON.stringify(program.getSourceFiles().map(f => f.fileName).filter(f => !f.includes('node_modules')))); +""" def resolve_binary() -> str | None: @@ -99,23 +133,112 @@ def probe() -> int: return 0 -def translate(raw: str, lane: str) -> dict: - payload = json.loads(raw) - percent = payload.get("percent") - details = payload.get("details") +def _key(path: str) -> str: + return os.path.normcase(os.path.abspath(path)) + + +def _row(lane: str, file: str | None, values: dict, labels: list[str]) -> dict: return { - "file": None, + "file": file, "function": None, "lane": lane, - "values": { + "values": values, + "collector": NAME, + "labels": labels, + } + + +def program_files() -> tuple[set[str] | None, str]: + """The tsconfig program's files as match keys, or None and why not.""" + node = shutil.which("node") + if not node: + return None, "node is not on PATH" + try: + result = subprocess.run( + [node, "-e", PROGRAM_SCRIPT], capture_output=True, text=True, check=False + ) + except OSError as exc: + return None, f"node failed to start: {exc}" + if result.returncode != 0: + said = result.stderr.strip().splitlines() or [ + "node exited " + str(result.returncode) + ] + return None, said[-1][:200] + try: + listed = json.loads(result.stdout) + except json.JSONDecodeError: + return None, "the program listing was not JSON" + if not isinstance(listed, list): + return None, "no tsconfig.json resolves from the working directory" + return {_key(str(name)) for name in listed}, "" + + +def translate( + raw: str, + lane: str, + files: list[str] | None = None, + program: set[str] | None = None, + why_no_program: str = "", +) -> tuple[list[dict], list[str]]: + """The rows for one capture, the lane row first, and the notes for + stderr: a row per scope file in the tsconfig program when the tool counted + anything; scope files outside the program are named in a note, never + given a 0.""" + payload = json.loads(raw) + percent = payload.get("percent") + details = payload.get("details") + notes: list[str] = [] + file_rows: list[dict] = [] + if payload.get("totalCount") and isinstance(details, list): + listed: dict[str, int] = {} + for entry in details: + key = _key(str(entry.get("filePath", ""))) + listed[key] = listed.get(key, 0) + 1 + outside: list[str] = [] + for path in files or []: + key = _key(path) + if program is None: + if key not in listed: + continue + elif key not in program: + outside.append(path) + continue + file_rows.append( + _row( + lane, + path, + { + "type_coverage_pct": None, + "typed_identifiers": None, + "total_identifiers": None, + "any_count": listed.get(key, 0), + }, + [], + ) + ) + if program is None: + notes.append( + f"the tsconfig program could not be read ({why_no_program}); " + "only scope files with a listed occurrence have a row" + ) + elif outside: + shown = ", ".join(outside[:3]) + (", ..." if len(outside) > 3 else "") + notes.append( + f"{len(outside)} scope file(s) are outside the tsconfig program " + f"and were not measured: {shown}" + ) + lane_row = _row( + lane, + None, + { "type_coverage_pct": float(percent) if percent is not None else None, "typed_identifiers": payload.get("correctCount"), "total_identifiers": payload.get("totalCount"), "any_count": len(details) if isinstance(details, list) else None, }, - "collector": NAME, - "labels": [], - } + ["lane-total"], + ) + return [lane_row, *file_rows], notes def collect(lane: str, measure: str, files: list[str]) -> int: @@ -127,20 +250,34 @@ def collect(lane: str, measure: str, files: list[str]) -> int: print(f"{NAME} not on PATH", file=sys.stderr) return 3 result = subprocess.run( - [exe, "--detail", "--json-output", "--", *files], + [exe, "--detail", "--json-output", "--show-relative-path", "--", *files], capture_output=True, text=True, check=False, ) try: - row = translate(result.stdout, lane) - except (json.JSONDecodeError, ValueError, TypeError) as exc: + payload = json.loads(result.stdout) + except json.JSONDecodeError as exc: print( f"{NAME}.py: unparsable {NAME} output ({exc}); stderr: {result.stderr.strip()}", file=sys.stderr, ) return 3 - print(json.dumps(row)) + program: set[str] | None = None + why = "" + if isinstance(payload, dict) and payload.get("totalCount"): + # Only a run that counted something needs to know which files the + # program holds; the no-tsconfig case never reaches node. + program, why = program_files() + try: + rows, notes = translate(result.stdout, lane, files, program, why) + except (ValueError, TypeError, AttributeError) as exc: + print(f"{NAME}.py: unparsable {NAME} output ({exc})", file=sys.stderr) + return 3 + for row in rows: + print(json.dumps(row)) + if notes: + print("; ".join(notes), file=sys.stderr) return 0 diff --git a/plugins/code-metrics/scripts/dispatch.sh b/plugins/code-metrics/scripts/dispatch.sh index e8b0eef4e2..e98846e1b2 100755 --- a/plugins/code-metrics/scripts/dispatch.sh +++ b/plugins/code-metrics/scripts/dispatch.sh @@ -658,7 +658,11 @@ for slot in "${COLLECT_SLOTS[@]}"; do version="${S_VERSION[slot]}" if [[ "${rc:-1}" -eq 0 ]]; then cat "$WORK/out.$slot" >>"$ROWS" - run_row "$slot" "$lane" "$measure" "$tool $version" ok '' + # What an adapter said on stderr while succeeding (mypy's error count, + # a module the scope did not cover) is the ok row's reason; an adapter + # that said nothing leaves it null. + note="$(tr '\n' ' ' <"$WORK/err.$slot" | cut -c1-500)" + run_row "$slot" "$lane" "$measure" "$tool $version" ok "${note% }" progress "$lane/$measure: $tool finished in ${elapsed:-?}s, $(wc -l <"$WORK/out.$slot" | tr -d ' ') row(s)" elif [[ "${rc:-1}" -eq 4 ]]; then run_row "$slot" "$lane" "$measure" "$tool $version" unavailable "$(tr '\n' ' ' <"$WORK/err.$slot" | cut -c1-500)" diff --git a/plugins/code-metrics/scripts/dispatch.test.sh b/plugins/code-metrics/scripts/dispatch.test.sh index 944916e611..fc7a16d2fc 100755 --- a/plugins/code-metrics/scripts/dispatch.test.sh +++ b/plugins/code-metrics/scripts/dispatch.test.sh @@ -86,6 +86,8 @@ assert_doc "every fixture is measured exactly once" "$out" \ 'sorted(r["file"].rsplit("/",1)[1] for r in d["measures"])==["CmSample.cs","cm-notes.md","cm-sample.go","cm-sample.sh","cm-sample.ts","cm_sample.py","shared-utils.sh","shared-utils.sh"]' assert_doc "threshold carries the plugin-default provenance" "$out" \ 'd["thresholds"][0]["measure"]=="file_lines" and d["thresholds"][0]["reference"]==1000 and "not normative" in d["thresholds"][0]["provenance"]' +assert_doc "a collector that said nothing leaves its ok row's reason null" "$out" \ + 'all(r["reason"] is None for r in d["run"] if r["status"]=="ok")' # 2. scc present: the ladder prefers it and comment counts appear. out="$(PATH="$STUBS:$EMPTY_PATH" bash "$SCRIPT" audit-size --measures file_lines --all "$SOURCES")" @@ -157,6 +159,32 @@ assert_doc "collect failure row is unavailable with the stderr relayed" "$out" \ 'd["run"][0]["status"]=="unavailable" and d["run"][0]["collector"]=="scc 9.9.9" and "collect failed" in d["run"][0]["reason"] and "boom" in d["run"][0]["reason"]' rm -rf "$broken" +# 8b. A collector that succeeds while saying something on stderr: the ok row +# carries what it said as its reason (mypy's error count reaches the run +# table this way); a silent success leaves the reason null. +noisy="$(mktemp -d)" +cat >"$noisy/mypy" < list[dict[str, Any]]: if function and (not isinstance(start, int) or isinstance(start, bool)): known = starts.get((file, function), set()) start = next(iter(known)) if len(known) == 1 else None - key = (file, function, start) + # The lane is part of the identity: two lane rows (`file` null) from + # two lanes must never join into one line. + key = (file, function, start, row.get("lane")) candidates = by_key.setdefault(key, []) for candidate in candidates: if _merge_into(candidate, row): @@ -452,10 +454,15 @@ def render(doc: dict[str, Any], document_path: str | None = None) -> str: for row in sorted( measures, key=lambda r: ( + # A lane row (`lane-total`) is the lane's figure: it leads its + # table and never falls under the row cap. + 0 if "lane-total" in (r.get("labels") or []) else 1, -len(r.get("over_reference", [])), -_over_distance(r, references), _primary_rank(primary, r), - r.get("file", ""), + # A lane row (type debt) has `file: null`; `or ""` keeps it + # comparable with the file rows it now sorts among. + r.get("file") or "", r.get("start_line") or 0, ), ): diff --git a/plugins/code-metrics/scripts/test_report.py b/plugins/code-metrics/scripts/test_report.py index 056496f1cf..d52a8c7a7b 100755 --- a/plugins/code-metrics/scripts/test_report.py +++ b/plugins/code-metrics/scripts/test_report.py @@ -676,6 +676,126 @@ def test_a_below_reference_orders_smallest_first(self) -> None: ) self.assertIn("Files: 2. Functions: 2. Over reference: none.", result.stdout) + def test_a_lane_row_sorts_among_file_rows_that_tie_on_the_value(self) -> None: + # audit-type-debt emits file rows and a lane row (`file: null`) that + # can tie on every earlier key; the sort must not compare None with + # a path. + doc = self._size_doc([]) + doc["skill"] = "audit-type-debt" + doc["thresholds"] = [ + { + "measure": "type_coverage", + "value_key": "type_coverage_pct", + "direction": "below", + "reference": None, + "provenance": "p", + "layer": "bundled default", + } + ] + values = { + "any_expressions": 0, + "expressions_total": 13, + "type_coverage_pct": 100.0, + } + doc["measures"] = [ + { + "file": None, + "function": None, + "lane": "python", + "values": values, + "labels": ["lane-total"], + "over_reference": [], + }, + { + "file": "a.py", + "function": None, + "lane": "python", + "values": values, + "labels": [], + "over_reference": [], + }, + ] + doc["summary"] = {"files": 1, "functions": 0, "over_reference": {}} + result = run("render", stdin=json.dumps(doc)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("| a.py |", result.stdout) + self.assertIn("lane-total", result.stdout) + self.assertIn("Files: 1. Over reference: none.", result.stdout) + + def _type_debt_doc(self, measures: list[dict]) -> dict: + doc = self._size_doc([]) + doc["skill"] = "audit-type-debt" + doc["thresholds"] = [ + { + "measure": "type_coverage", + "value_key": "type_coverage_pct", + "direction": "below", + "reference": None, + "provenance": "p", + "layer": "bundled default", + } + ] + doc["measures"] = measures + doc["summary"] = {"files": 0, "functions": 0, "over_reference": {}} + return doc + + def test_two_lane_rows_with_equal_values_stay_two_lines(self) -> None: + # Both lanes fully typed: the rows tie on file, function and start + # line and must still render one line per lane. + doc = self._type_debt_doc( + [ + { + "file": None, + "function": None, + "lane": lane, + "collector": collector, + "values": {"type_coverage_pct": 100.0}, + "labels": ["lane-total"], + "over_reference": [], + } + for lane, collector in ( + ("python", "mypy-report"), + ("typescript", "type-coverage"), + ) + ] + ) + result = run("render", stdin=json.dumps(doc)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.count("| lane-total |"), 2) + self.assertNotIn("mypy-report, type-coverage", result.stdout) + + def test_the_lane_row_leads_and_survives_the_row_cap(self) -> None: + rows = [ + { + "file": f"m{i:03d}.py", + "function": None, + "lane": "python", + "values": {"type_coverage_pct": float(i % 100)}, + "labels": [], + "over_reference": [], + } + for i in range(201) + ] + rows.append( + { + "file": None, + "function": None, + "lane": "python", + "values": {"type_coverage_pct": 50.0}, + "labels": ["lane-total"], + "over_reference": [], + } + ) + doc = self._type_debt_doc(rows) + result = run("render", stdin=json.dumps(doc)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("| lane-total |", result.stdout) + self.assertIn("more rows", result.stdout) + table_lines = [ + line for line in result.stdout.splitlines() if "| python |" in line + ] + self.assertIn("lane-total", table_lines[0]) + def test_the_row_cap_names_the_key_it_kept_the_top_rows_by(self) -> None: rows = [ { diff --git a/plugins/code-metrics/skills/audit-type-debt/SKILL.md b/plugins/code-metrics/skills/audit-type-debt/SKILL.md index 66c460a182..96fc884adf 100644 --- a/plugins/code-metrics/skills/audit-type-debt/SKILL.md +++ b/plugins/code-metrics/skills/audit-type-debt/SKILL.md @@ -1,5 +1,5 @@ --- -description: "Measure how much of the code is typed for the changed files, a path, or the whole tree, as a percentage per lane: `type-coverage`'s ratio of identifiers whose type is not `any` for TypeScript, and mypy's `--any-exprs-report` coverage over expressions for Python. Because no standard or CWE anchors this measure, the reference is `null` by design and the number is a trend to watch rather than a bar; the report emits no finding, severity, or exit-code gate. Bash and Go have no comparable collector, and C# is not applicable, because an occurrence count is not comparable to a ratio. A lane whose tool is absent says so with an install hint and the run continues. Use when: 'how much of this is typed', 'type coverage', 'how many anys are in this', 'any usage in the change', 'type debt', 'measure our typing', 'mypy any expressions report'; for cyclomatic or cognitive complexity use /code-metrics:audit-complexity, and for what a measure can and cannot tell you use /code-metrics:principles." +description: "Measure how much of the code is typed for the changed files, a path, or the whole tree, per file and as a percentage per lane: `type-coverage`'s ratio of identifiers whose type is not `any` for TypeScript, and mypy's `--any-exprs-report` coverage over expressions for Python. Because no standard or CWE anchors this measure, the reference is `null` by design and the number is a trend to watch rather than a bar; the report emits no finding, severity, or exit-code gate. Bash and Go have no comparable collector, and C# is not applicable, because an occurrence count is not comparable to a ratio. A lane whose tool is absent says so with an install hint and the run continues. Use when: 'how much of this is typed', 'type coverage', 'how many anys are in this', 'any usage in the change', 'type debt', 'measure our typing', 'mypy any expressions report'; for cyclomatic or cognitive complexity use /code-metrics:audit-complexity, and for what a measure can and cannot tell you use /code-metrics:principles." argument-hint: "[--json] [--all] [--base ] [...]" user-invocable: true disable-model-invocation: false @@ -7,7 +7,7 @@ allowed-tools: ["Bash(${CLAUDE_SKILL_DIR}/scripts/audit-type-debt.sh:*)", "Bash( shell: bash metadata: workflow-stage: anytime - summary: Typed-code percentage per lane, with no standard behind it + summary: Typed-code percentage per file and per lane, with no standard behind it --- ## Pre-computed context @@ -18,13 +18,13 @@ Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` "Get `any` and `unknown` to zero" is a goal without a yardstick: no standard and no CWE anchors a typed-code percentage. This skill reports what the two tools that produce a real percentage -measure, per lane, and says plainly where the number comes from and what it does not mean. There -is no pass or fail here, and no bar to argue with. +measure, per file and per lane, and says plainly where the number comes from and what it does not +mean. There is no pass or fail here, and no bar to argue with. -| Lane | Collector | Values per lane | The number it reports | +| Lane | Collector | Values | The number it reports | |---|---|---|---| -| TypeScript/JavaScript | `type-coverage` | `type_coverage_pct`, `typed_identifiers`, `total_identifiers`, `any_count` | identifiers whose type is not `any`, over all identifiers | -| Python | mypy `--any-exprs-report` | `type_coverage_pct`, `any_expressions`, `expressions_total` | mypy's own Coverage column: expressions not typed `Any`, over all expressions | +| TypeScript/JavaScript | `type-coverage` | lane row: `type_coverage_pct`, `typed_identifiers`, `total_identifiers`, `any_count`; file row: `any_count` alone, the other three `null` | identifiers whose type is not `any`, over all identifiers | +| Python | mypy `--any-exprs-report` | `type_coverage_pct`, `any_expressions`, `expressions_total` on every file row and on the lane row | mypy's own Coverage column: expressions not typed `Any`, over all expressions | | Bash | not applicable | | no collector reports a typed-code ratio for shell | | Go | not applicable | | the compiler admits no untyped identifier to count | | C# | not applicable | | no tool produces a comparable percentage for C#; a `dynamic`/`object` occurrence count is not comparable to a ratio | @@ -40,7 +40,8 @@ is no pass or fail here, and no bar to argue with. Present the markdown report as printed. It opens with the scope and a "Coverage of this run" table (lane, collector, status, reason), then the reference with its provenance and layer, then one row -per lane with its values. Keep the `--json` document when the numbers feed a comparison: +per file, least typed first, and one row per lane labelled `lane-total`. Keep the `--json` +document when the numbers feed a comparison: `/verification:measure metrics` consumes it when the `verification` plugin is installed (treat a report whose `status` is `empty` on either side as INCONCLUSIVE); otherwise keep the JSON beside your notes and compare by hand. @@ -53,14 +54,27 @@ your notes and compare by hand. - The reference is `null` by design, because no standard or CWE sets one. A consumer who sets one gets a `below` comparison (a lane under the reference is counted), which is still a count and never a finding, a severity, or an exit code. +- The `lane-total` row is the lane's figure and the summary's `Files:` count is the file rows. + For Python the lane row is the sum of the file rows, so a change-scoped run reports the scope's + own coverage rather than everything mypy followed. mypy names modules, not files; the file is + recovered by re-deriving mypy's `--explicit-package-bases` naming from the path, and when none + of its names matches a scope file (a `mypy_path` in the consumer's mypy config can do that) the + lane row is mypy's own Total, no file row is emitted, and the run row's reason says so. - A value the tool did not produce is `null`, never `0`: `any_count` is `null` when `type-coverage` listed no locations, and `type_coverage_pct` is `null` when nothing was counted - at all (a TypeScript project with no `tsconfig.json` reaches this). -- mypy exits 1 on any type error and still writes its report; the row is kept and labelled - `mypy-reported-errors`, because a type error is not a missing measurement. mypy exits 2 when a - blocking error (a duplicate module name, a usage or config error) stops it before analysis; the - Python lane then reads `unavailable` with mypy's own message, never a percentage, and the run - continues. + at all (a TypeScript project with no `tsconfig.json` reaches this). A TypeScript file row + carries `any_count` alone, the occurrences `type-coverage --detail` listed for that file, + because the CLI gives no per-file denominator. A scope file in the project's `tsconfig.json` + program with no listed occurrence reads `0`, a measured zero; a scope file outside that program + gets no row and is counted in the run row's reason; when the lane counted nothing, no file row + is emitted. +- mypy exits 1 on any type error and still writes its report; the rows are kept, the lane row is + labelled `mypy-reported-errors`, and the run row's reason counts the errors and, among them, + the missing stubs (`import-untyped`, `import-not-found`), which says how much of the `Any` + count is unstubbed imports rather than local typing. A type error is not a missing measurement. + mypy exits 2 when a blocking error (a duplicate module name, a usage or config error) stops it + before analysis; the Python lane then reads `unavailable` with mypy's own message, never a + percentage, and the run continues. - Exit 0 whenever a report was produced, including a run that measured nothing; exit 2 for a usage error such as an explicitly named path that does not exist; exit 3 when a collector resolved but produced nothing parseable, with its stderr in the run table. @@ -97,8 +111,19 @@ the collectors. only the binary is present. Install both as project dev dependencies. - `type-coverage` reads the project's `tsconfig.json`. Without one it counts nothing and reports `null` rather than a percentage. -- mypy type-checks the whole import graph it can see, so the expression count for a scoped run - covers what mypy followed, not only the files in scope. Compare like-scoped runs. +- mypy lists only the files it was given, so the file rows and the lane row cover the scope files + alone; it still follows their imports to type them, so a scope file's count can move when a + module it imports changes with no edit to the file itself. Compare like-scoped runs. +- mypy names modules, not files. The collector re-derives the name mypy gives each scope path + with the working directory as the base; a base the consumer's mypy config adds (`mypy_path = + src`, the src layout) shortens mypy's names, which are then matched to the one scope file whose + derived name ends in that name. A listed name that matches nothing is left out of the lane row + and counted in the run row's reason, as is a scope file mypy listed nothing for. +- `type-coverage` counts only the files its `tsconfig.json` program holds and does not say which + those are, so the collector reads the program through the project's own `typescript` (one + `node` call) and gives a row only to scope files in it; a scope file outside the program is + counted in the run row's reason, never reported as 0. When that call fails, only scope files + with a listed occurrence get a row and the reason says why. - The Python percentage moves when a dependency ships or drops type stubs, because an unfollowed import turns into `Any`. A drop with no local edit is usually that. - mypy runs with `--explicit-package-bases`, so a file vendored into several plugins (sanctioned diff --git a/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.sh b/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.sh index 15201dcb49..e16dd54beb 100755 --- a/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.sh +++ b/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # /code-metrics:audit-type-debt entry point: how much of the code is typed, -# per lane, as a percentage from `type-coverage` (TypeScript) and from mypy's -# `--any-exprs-report` (Python). +# per file and per lane, as a percentage from `type-coverage` (TypeScript) and +# from mypy's `--any-exprs-report` (Python). # # audit-type-debt.sh [--json] [--all] [--base ] [--config ] [...] # diff --git a/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh b/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh index 4fb66e1ab0..f617c5d4cd 100755 --- a/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh +++ b/plugins/code-metrics/skills/audit-type-debt/scripts/audit-type-debt.test.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Regression tests for the audit-type-debt entry point (audit-type-debt.sh): -# the per-lane rows both collectors produce, the lanes that are +# the file rows and the lane row both collectors produce, the lanes that are # not-applicable, the null reference, and what an absent tool looks like. # # Both tools are stubbed at runtime (design T13): a temporary bin/ prepended to @@ -9,7 +9,10 @@ # fixtures/tool-output/mypy-any-exprs.txt into the report directory it is # given. Every case runs from a scratch working directory, because the # type-coverage probe reads ./node_modules/typescript from the current -# directory; the fixture sources are passed as an absolute path. +# directory. The fixture sources are copied into that directory under their +# repository-relative path and passed relative, as the dispatcher passes a +# scope in a real run, because the type-coverage capture names its file by +# that relative path and a file row matches a scope file on the path. set -uo pipefail unset GIT_DIR GIT_WORK_TREE GIT_CONFIG @@ -96,7 +99,19 @@ done [[ -n "\$dir" ]] && mkdir -p "\$dir" cp "$MYPY_CAPTURE" "\$dir/any-exprs.txt" EOF -chmod +x "$STUBS/type-coverage" "$STUBS/mypy" +# The type-coverage adapter reads the tsconfig program through `node` and the +# project's typescript; this stub stands in for that listing, and fails the +# way node does when the working directory has no typescript, so the probe's +# `require.resolve('typescript')` check still fails in the bare directory. +cat >"$STUBS/node" <<'EOF' +#!/usr/bin/env bash +if [[ ! -f node_modules/typescript/package.json ]]; then + printf '%s\n' "Error: Cannot find module 'typescript'" >&2 + exit 1 +fi +printf '%s\n' '["plugins/code-metrics/scripts/fixtures/sources/cm-sample.ts"]' +EOF +chmod +x "$STUBS/type-coverage" "$STUBS/mypy" "$STUBS/node" # A scratch working directory whose node_modules/typescript makes the # type-coverage probe pass, and a bare one that makes it fail. @@ -104,6 +119,9 @@ PROJECT="$WORK/project" BARE="$WORK/bare" mkdir -p "$PROJECT/node_modules/typescript" "$BARE" printf '{"name": "typescript", "version": "5.9.3"}\n' >"$PROJECT/node_modules/typescript/package.json" +SCOPE="plugins/code-metrics/scripts/fixtures/sources" +mkdir -p "$PROJECT/${SCOPE%/*}" +cp -R "$SOURCES" "$PROJECT/$SCOPE" # The five lane fixtures this suite measures, named so the affected-tests # runner maps them here: cm-sample.ts, cm_sample.py, cm-sample.sh, @@ -115,19 +133,24 @@ done pass "the five lane fixtures are present" # 1. Both tools present: a percentage per typed lane, not-applicable elsewhere. -out="$(cd "$PROJECT" && PATH="$STUBS:$EMPTY_PATH" CODE_METRICS_HOME="$HOME_DIR" bash "$SCRIPT" --json --all "$SOURCES")" +out="$(cd "$PROJECT" && PATH="$STUBS:$EMPTY_PATH" CODE_METRICS_HOME="$HOME_DIR" bash "$SCRIPT" --json --all "$SCOPE")" rc=$? assert_eq "--json exits 0 with both collectors stubbed" 0 "$rc" assert_doc "the document is code-metrics/v1 for audit-type-debt" "$out" \ 'd["schema"]=="code-metrics/v1" and d["skill"]=="audit-type-debt"' -assert_doc "the typescript row carries type_coverage_pct from type-coverage.json" "$out" \ - 'next(r for r in d["measures"] if r["lane"]=="typescript")["values"]["type_coverage_pct"]==55.55' -assert_doc "the typescript row is per lane, not per file" "$out" \ - 'all(r["file"] is None and r["function"] is None for r in d["measures"])' -assert_doc "the python row carries any_expressions from mypy-any-exprs.txt" "$out" \ - 'next(r for r in d["measures"] if r["lane"]=="python")["values"]["any_expressions"]==0' -assert_doc "the python row also carries mypy's own coverage percentage" "$out" \ - 'next(r for r in d["measures"] if r["lane"]=="python")["values"]["type_coverage_pct"]==100.0' +assert_doc "the typescript lane row carries type_coverage_pct from type-coverage.json" "$out" \ + 'next(r for r in d["measures"] if r["lane"]=="typescript" and r["file"] is None)["values"]["type_coverage_pct"]==55.55' +assert_doc "each lane has one file row per scope file plus one lane-total row" "$out" \ + 'sorted(r["file"].rsplit("/",1)[-1] for r in d["measures"] if r["file"])==["cm-sample.ts","cm_sample.py"] and sorted(r["lane"] for r in d["measures"] if r["file"] is None and "lane-total" in r["labels"])==["python","typescript"] and all(r["function"] is None for r in d["measures"])' +assert_doc "the typescript file row carries the occurrences listed for that file and nothing else" "$out" \ + 'next(r for r in d["measures"] if r["lane"]=="typescript" and r["file"])["values"]=={"type_coverage_pct":None,"typed_identifiers":None,"total_identifiers":None,"any_count":4}' +assert_doc "the summary counts the file rows" "$out" 'd["summary"]["files"]==2' +assert_doc "the python file row carries any_expressions from mypy-any-exprs.txt" "$out" \ + 'next(r for r in d["measures"] if r["lane"]=="python" and r["file"])["values"]["any_expressions"]==0' +assert_doc "the python lane row also carries mypy's own coverage percentage" "$out" \ + 'next(r for r in d["measures"] if r["lane"]=="python" and r["file"] is None)["values"]["type_coverage_pct"]==100.0' +assert_doc "a silent success leaves the ok row's reason null" "$out" \ + 'all(r["reason"] is None for r in d["run"] if r["status"]=="ok")' assert_doc "the dotnet row is not-applicable with the C# sentence" "$out" \ 'next(r for r in d["run"] if r["lane"]=="dotnet" and r["measure"]=="type_coverage")["status"]=="not-applicable"' assert_doc "the dotnet reason says a count is not comparable to a ratio" "$out" \ @@ -140,12 +163,15 @@ assert_doc "no row is counted over a reference" "$out" \ 'all(r["over_reference"]==[] for r in d["measures"])' # 2. The markdown rendering carries the coverage table. -out="$(cd "$PROJECT" && PATH="$STUBS:$EMPTY_PATH" CODE_METRICS_HOME="$HOME_DIR" bash "$SCRIPT" --all "$SOURCES")" +out="$(cd "$PROJECT" && PATH="$STUBS:$EMPTY_PATH" CODE_METRICS_HOME="$HOME_DIR" bash "$SCRIPT" --all "$SCOPE")" rc=$? assert_eq "markdown exits 0" 0 "$rc" assert_contains "markdown carries the run table" "$out" "## Coverage of this run" assert_contains "markdown names the type-coverage collector" "$out" "type-coverage 2.30.1" assert_contains "markdown prints the null reference" "$out" "| type_coverage | null |" +assert_contains "markdown lists the python file row" "$out" "cm_sample.py |" +assert_contains "markdown marks the lane row" "$out" "| lane-total |" +assert_contains "markdown counts the files measured" "$out" "Files: 2." # 3. type-coverage resolves but typescript does not: the probe's requirement # reaches the run row's reason (the dispatcher relays the adapter's install @@ -166,7 +192,7 @@ assert_doc "the python lane still reports while typescript cannot" "$out" \ # than a 100% measurement, and the run is not a failure. ABORT_STUBS="$WORK/abort-stubs" mkdir -p "$ABORT_STUBS" -cp "$STUBS/type-coverage" "$ABORT_STUBS/type-coverage" +cp "$STUBS/type-coverage" "$STUBS/node" "$ABORT_STUBS/" cat >"$ABORT_STUBS/mypy" <"$ERROR_STUBS/mypy" < Date: Fri, 11 Sep 2026 16:24:43 +0000 Subject: [PATCH 4/5] docs(code-metrics): regenerate the skill cheat sheet for the audit-type-debt summary The summary now reads per file and per lane; validate-plugins.sh compares the generated block and failed every test-linux shard on 72dfff25. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kn6LHhMJfke8gjy8kNdkHk --- docs/SKILL-CHEAT-SHEET.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/SKILL-CHEAT-SHEET.md b/docs/SKILL-CHEAT-SHEET.md index b836abda27..86f6deaa28 100644 --- a/docs/SKILL-CHEAT-SHEET.md +++ b/docs/SKILL-CHEAT-SHEET.md @@ -168,7 +168,7 @@ owned by [docs/CATALOG-TAXONOMY.md](CATALOG-TAXONOMY.md). | [`/code-metrics:audit-coverage`](../plugins/code-metrics/skills/audit-coverage/SKILL.md) | `code-metrics` | Coverage and CRAP read from build artifacts, no verdict | | [`/code-metrics:audit-duplication`](../plugins/code-metrics/skills/audit-duplication/SKILL.md) | `code-metrics` | Clone groups minus the replication the repo declares, no verdict | | [`/code-metrics:audit-size`](../plugins/code-metrics/skills/audit-size/SKILL.md) | `code-metrics` | Lines per file beside a cited reference, no verdict | -| [`/code-metrics:audit-type-debt`](../plugins/code-metrics/skills/audit-type-debt/SKILL.md) | `code-metrics` | Typed-code percentage per lane, with no standard behind it | +| [`/code-metrics:audit-type-debt`](../plugins/code-metrics/skills/audit-type-debt/SKILL.md) | `code-metrics` | Typed-code percentage per file and per lane, with no standard behind it | | [`/code-metrics:principles`](../plugins/code-metrics/skills/principles/SKILL.md) | `code-metrics` | What each code measure can and cannot tell you | | [`/code-tidying:audit-dead-code`](../plugins/code-tidying/skills/audit-dead-code/SKILL.md) | `code-tidying` | Whole-repo dead-code hunt across four labelled lanes with adjudicated candidates | | [`/code-tidying:tidy`](../plugins/code-tidying/skills/tidy/SKILL.md) | `code-tidying` | Proactively hunt one lane for safe structural tidyings and ship a structure-only PR | From 5fad35dff49d12f8acaddc77cfe844112ea13f54 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 16:43:09 +0000 Subject: [PATCH 5/5] fix(code-metrics): rerun mypy without explicit bases when namespace packages are off, match node_modules by segment mypy accepts --explicit-package-bases only with namespace packages on; a consumer config that turns them off made the first run a usage error and the whole Python lane `unavailable`. The collector now repeats that run without the flag, in mypy's own __init__.py naming, and says so in the run row's reason; any other usage error still reaches the exit-4 row. The tsconfig program filter dropped any path containing the string node_modules, so a source file such as src/node_modules_helper.ts lost its row. The exclusion now matches a whole path segment and runs in the collector, where the node stub can exercise it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kn6LHhMJfke8gjy8kNdkHk --- plugins/code-metrics/CHANGELOG.md | 4 +- .../scripts/collectors/mypy-report.py | 68 +++++++++++---- .../scripts/collectors/test_mypy_report.py | 87 ++++++++++++++++++- .../scripts/collectors/test_type_coverage.py | 57 ++++++++++++ .../scripts/collectors/type-coverage.py | 20 +++-- .../skills/audit-type-debt/SKILL.md | 5 +- 6 files changed, 213 insertions(+), 28 deletions(-) diff --git a/plugins/code-metrics/CHANGELOG.md b/plugins/code-metrics/CHANGELOG.md index 5ef1c09eec..15a5ab649f 100644 --- a/plugins/code-metrics/CHANGELOG.md +++ b/plugins/code-metrics/CHANGELOG.md @@ -49,7 +49,9 @@ All notable changes to the `code-metrics` plugin are documented here. Format fol `--explicit-package-bases`, so mypy names each module by its path (`plugins.a.lib.x`) and two same-named files under identifier-named directories no longer collide. Same-named files under two hyphenated directories still collide, because mypy's module walk stops at a directory whose - name is not a Python identifier; that case reaches the `unavailable` row above. + name is not a Python identifier; that case reaches the `unavailable` row above. mypy accepts the + flag only with namespace packages on, so when the consumer's config turns them off the run + repeats without it, in mypy's own `__init__.py` naming, and the run row's reason says so. - **`audit-type-debt`: no `.mypy_cache/` in the consumer's tree.** The collector passes `--cache-dir` with the platform's null device, mypy's documented value for disabling the cache; a one-shot report gained nothing from it (6.8s without a cache against 8.2s with a warm one over diff --git a/plugins/code-metrics/scripts/collectors/mypy-report.py b/plugins/code-metrics/scripts/collectors/mypy-report.py index 889d7442de..3effcb872e 100755 --- a/plugins/code-metrics/scripts/collectors/mypy-report.py +++ b/plugins/code-metrics/scripts/collectors/mypy-report.py @@ -57,7 +57,13 @@ identifier-named directories (`a/foo.py`, `b/foo.py`) no longer collide. The walk stops at a directory whose name is not a Python identifier, so same-named files under two hyphenated directories still collide and reach the - exit-4 path with mypy's message; + exit-4 path with mypy's message. mypy accepts the flag only while namespace + packages are on (its default), so when the consumer's config turns them off + mypy refuses the pairing with a usage error (exit 2); the run then repeats + without the flag, in mypy's own naming mode (packages from `__init__.py` + files), and a stderr note says so. The shorter names that mode gives are + matched by the same suffix pass a config base uses; same-named files collide + again in it, the consumer's own configuration; - `--cache-dir os.devnull` is mypy's documented "disable caching" value (mypy compares the option to os.devnull by string equality, `/dev/null` on POSIX and `nul` on Windows), so no `.mypy_cache` is written into the consumer's @@ -235,24 +241,20 @@ def collect(lane: str, measure: str, files: list[str]) -> int: print(f"{TOOL} not on PATH", file=sys.stderr) return 3 report_dir = tempfile.mkdtemp(prefix="code-metrics-mypy-") + naming_note = "" try: - result = subprocess.run( - [ - exe, - "--any-exprs-report", - report_dir, - "--no-error-summary", - "--show-error-codes", - "--no-pretty", - "--explicit-package-bases", - "--cache-dir", - os.devnull, - *files, - ], - capture_output=True, - text=True, - check=False, - ) + result = _run_mypy(exe, report_dir, files, explicit_bases=True) + if result.returncode == FATAL_EXIT and _rejects_explicit_bases(result.stderr): + # The consumer's config turns namespace packages off, and mypy + # allows --explicit-package-bases only with them on. Overriding + # that config would measure a project the consumer did not + # configure, so the run repeats in mypy's own naming mode + # (packages from __init__.py files) and says so. + result = _run_mypy(exe, report_dir, files, explicit_bases=False) + naming_note = ( + "namespace packages are off in the mypy config, so modules are " + "named from __init__.py packages rather than their paths" + ) if result.returncode == FATAL_EXIT: # A blocking error stopped mypy before analysis; the report it still # wrote is empty, so there is no measurement to read. The tool @@ -289,7 +291,7 @@ def collect(lane: str, measure: str, files: list[str]) -> int: print(f"{NAME}.py: unparsable report ({exc})", file=sys.stderr) return 3 matched = match_modules(modules, files) - notes: list[str] = [] + notes: list[str] = [naming_note] if naming_note else [] if result.returncode != 0: notes.append(error_note(result.stdout)) labels = ["lane-total"] + (["mypy-reported-errors"] if result.returncode else []) @@ -326,6 +328,34 @@ def collect(lane: str, measure: str, files: list[str]) -> int: return 0 +def _run_mypy( + exe: str, report_dir: str, files: list[str], explicit_bases: bool +) -> subprocess.CompletedProcess: + return subprocess.run( + [ + exe, + "--any-exprs-report", + report_dir, + "--no-error-summary", + "--show-error-codes", + "--no-pretty", + *(["--explicit-package-bases"] if explicit_bases else []), + "--cache-dir", + os.devnull, + *files, + ], + capture_output=True, + text=True, + check=False, + ) + + +def _rejects_explicit_bases(stderr: str) -> bool: + """True when mypy's usage error is the one pairing rule this adapter can + trip: `Can only use --explicit-package-bases with --namespace-packages`.""" + return "--explicit-package-bases" in stderr and "--namespace-packages" in stderr + + def match_modules(modules: dict[str, Counts], files: list[str]) -> dict[str, str]: """Map each listed module to the scope file it names: by the cwd-based derivation first, then, for a listed name no derived name equals, the one diff --git a/plugins/code-metrics/scripts/collectors/test_mypy_report.py b/plugins/code-metrics/scripts/collectors/test_mypy_report.py index 7dc0d24d42..4c3035187d 100755 --- a/plugins/code-metrics/scripts/collectors/test_mypy_report.py +++ b/plugins/code-metrics/scripts/collectors/test_mypy_report.py @@ -53,23 +53,41 @@ def make_stub( stdout_line: str = "", stderr_line: str = "", argv_log: Path | None = None, + reject_explicit_bases: bool = False, ) -> None: """Write a `mypy` stub that replays the capture into the report directory. With `argv_log` the stub also records every argument it received, one per - line, so a test can assert on the flags the adapter passes. + line and one run after another, so a test can assert on the flags the + adapter passes. With `reject_explicit_bases` the stub answers a run + carrying `--explicit-package-bases` the way mypy does when the config + turns namespace packages off: the usage error on stderr and exit 2, + before any report is written. """ copy = ( f'cp "{capture}" "$dir/any-exprs.txt"\n' if capture is not None else "# the report is never written\n" ) - log = f'printf \'%s\\n\' "$@" >"{argv_log}"\n' if argv_log is not None else "" + log = f'printf \'%s\\n\' "$@" >>"{argv_log}"\n' if argv_log is not None else "" + reject = ( + 'for arg in "$@"; do\n' + ' if [[ "$arg" == "--explicit-package-bases" ]]; then\n' + " printf '%s\\n' 'mypy: error: Can only use --explicit-package-bases " + "with --namespace-packages, since otherwise examining __init__.py files " + "is sufficient to determine module names for files' >&2\n" + " exit 2\n" + " fi\n" + "done\n" + if reject_explicit_bases + else "" + ) stub = directory / "mypy" stub.write_text( "#!/usr/bin/env bash\n" 'if [[ "${1:-}" == "--version" ]]; then printf \'mypy 1.19.1 (compiled: yes)\\n\'; exit 0; fi\n' + log + + reject + 'dir=""\nprev=""\n' 'for arg in "$@"; do\n' ' [[ "$prev" == "--any-exprs-report" ]] && dir="$arg"\n' @@ -395,6 +413,71 @@ def test_collect_passes_explicit_package_bases_and_a_devnull_cache_dir( self.assertEqual(argv[argv.index("--cache-dir") + 1], os.devnull) self.assertEqual(argv[-1], f"{SOURCES}/cm_sample.py") + def test_a_config_without_namespace_packages_reruns_in_mypys_own_naming( + self, + ) -> None: + # mypy allows --explicit-package-bases only with namespace packages on; + # a consumer config that turns them off makes the first run a usage + # error (exit 2, nothing measured). That is not the consumer's tree + # failing, so the run repeats without the flag and the note says which + # naming the rows follow. The capture's names (`pkg.a`, `b`, `c`) are + # the ones mypy's __init__.py walk gives too, so the rows still map. + with tempfile.TemporaryDirectory() as tmp: + argv_log = Path(tmp) / "argv" + make_stub( + Path(tmp), + capture=MODULES, + argv_log=argv_log, + reject_explicit_bases=True, + ) + result = run( + "collect", + "python", + "type_coverage", + "pkg/a.py", + "pkg-x/b.py", + "c.py", + path_prefix=Path(tmp), + cwd=Path(tmp), + ) + self.assertEqual(result.returncode, 0, result.stderr) + rows = rows_of(result) + self.assertEqual( + [r["file"] for r in rows], [None, "pkg/a.py", "pkg-x/b.py", "c.py"] + ) + self.assertEqual(rows[0]["labels"], ["lane-total"]) + self.assertEqual( + result.stderr.strip(), + "namespace packages are off in the mypy config, so modules are " + "named from __init__.py packages rather than their paths", + ) + argv = argv_log.read_text(encoding="utf-8").splitlines() + # Two runs: the flag on the first only, the report asked for twice. + self.assertEqual(argv.count("--explicit-package-bases"), 1) + self.assertEqual(argv.count("--any-exprs-report"), 2) + self.assertLess( + argv.index("--explicit-package-bases"), + argv.index("--any-exprs-report", argv.index("--any-exprs-report") + 1), + ) + + def test_a_usage_error_that_is_not_the_pairing_rule_is_still_exit_4(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + make_stub( + Path(tmp), + exit_code=2, + capture=ABORTED, + stderr_line="mypy: error: unrecognized arguments: --frobnicate", + ) + result = run( + "collect", + "python", + "type_coverage", + f"{SOURCES}/cm_sample.py", + path_prefix=Path(tmp), + ) + self.assertEqual(result.returncode, 4, result.stderr) + self.assertIn("--frobnicate", result.stderr) + @unittest.skipUnless(shutil.which("mypy"), "the real mypy is not on PATH") def test_the_real_mypy_leaves_no_cache_in_the_working_directory(self) -> None: with tempfile.TemporaryDirectory() as tmp: diff --git a/plugins/code-metrics/scripts/collectors/test_type_coverage.py b/plugins/code-metrics/scripts/collectors/test_type_coverage.py index c393ef4a6d..7f5f6826ed 100755 --- a/plugins/code-metrics/scripts/collectors/test_type_coverage.py +++ b/plugins/code-metrics/scripts/collectors/test_type_coverage.py @@ -239,6 +239,63 @@ def test_file_rows_group_the_listed_occurrences_by_scope_file(self) -> None: ["--detail", "--json-output", "--show-relative-path", "--", "src/a.ts"], ) + def test_the_program_drops_node_modules_by_segment_not_by_substring(self) -> None: + # The dependency tree is left out of the program the way the tool + # leaves it out; a source file whose name contains the string is not a + # dependency and keeps its row. + with tempfile.TemporaryDirectory() as tmp: + stubs = Path(tmp) / "bin" + helper = Path(tmp) / "src" / "node_modules_helper.ts" + helper.parent.mkdir(parents=True) + helper.write_text("export const x: any = 1;\n", encoding="utf-8") + capture = Path(tmp) / "capture.json" + capture.write_text( + json.dumps( + { + "percent": 50.0, + "correctCount": 1, + "totalCount": 2, + "details": [ + { + "filePath": "src/node_modules_helper.ts", + "line": 0, + "character": 13, + "text": "x", + } + ], + } + ), + encoding="utf-8", + ) + write_stub(stubs / "type-coverage", capture=capture) + write_node_stub( + stubs / "node", + [ + str(helper), + str(Path(tmp) / "node_modules" / "dep" / "index.d.ts"), + ], + ) + result = run( + "collect", + "typescript", + "type_coverage", + "src/node_modules_helper.ts", + "node_modules/dep/index.d.ts", + path_prefix=stubs, + cwd=Path(tmp), + ) + self.assertEqual(result.returncode, 0, result.stderr) + rows = [json.loads(line) for line in result.stdout.splitlines()] + self.assertEqual( + [(r["file"], r["values"]["any_count"]) for r in rows], + [(None, 1), ("src/node_modules_helper.ts", 1)], + ) + self.assertEqual( + result.stderr.strip(), + "1 scope file(s) are outside the tsconfig program and were not " + "measured: node_modules/dep/index.d.ts", + ) + def test_an_unreadable_program_keeps_only_the_listed_files(self) -> None: with tempfile.TemporaryDirectory() as tmp: stubs = Path(tmp) / "bin" diff --git a/plugins/code-metrics/scripts/collectors/type-coverage.py b/plugins/code-metrics/scripts/collectors/type-coverage.py index a996d248f7..211e99b6a7 100755 --- a/plugins/code-metrics/scripts/collectors/type-coverage.py +++ b/plugins/code-metrics/scripts/collectors/type-coverage.py @@ -70,9 +70,10 @@ LANE = "typescript" LOCAL_BIN = os.path.join("node_modules", ".bin", "type-coverage") NO_TYPESCRIPT = "type-coverage needs a resolvable typescript (the probe found none)" -# Prints the tsconfig program's source files (absolute paths, node_modules -# excluded) as a JSON array, or `null` when no tsconfig.json resolves from the -# working directory, through the same `typescript` the tool itself uses. +# Prints the tsconfig program's source files (absolute paths, dependencies +# included) as a JSON array, or `null` when no tsconfig.json resolves from the +# working directory, through the same `typescript` the tool itself uses. The +# `node_modules` exclusion is applied here, by path segment. PROGRAM_SCRIPT = """ const ts = require('typescript'); const path = require('path'); @@ -81,7 +82,7 @@ const read = ts.readConfigFile(config, ts.sys.readFile); const parsed = ts.parseJsonConfigFileContent(read.config || {}, ts.sys, path.dirname(config)); const program = ts.createProgram(parsed.fileNames, parsed.options); -console.log(JSON.stringify(program.getSourceFiles().map(f => f.fileName).filter(f => !f.includes('node_modules')))); +console.log(JSON.stringify(program.getSourceFiles().map(f => f.fileName))); """ @@ -137,6 +138,13 @@ def _key(path: str) -> str: return os.path.normcase(os.path.abspath(path)) +def _under_node_modules(path: str) -> bool: + """True when a whole path segment is `node_modules`, the dependency tree + the tool itself leaves out; a file whose name merely contains the string + (`src/node_modules_helper.ts`) is a source file and stays.""" + return "node_modules" in re.split(r"[\\/]", path) + + def _row(lane: str, file: str | None, values: dict, labels: list[str]) -> dict: return { "file": file, @@ -170,7 +178,9 @@ def program_files() -> tuple[set[str] | None, str]: return None, "the program listing was not JSON" if not isinstance(listed, list): return None, "no tsconfig.json resolves from the working directory" - return {_key(str(name)) for name in listed}, "" + return { + _key(str(name)) for name in listed if not _under_node_modules(str(name)) + }, "" def translate( diff --git a/plugins/code-metrics/skills/audit-type-debt/SKILL.md b/plugins/code-metrics/skills/audit-type-debt/SKILL.md index 96fc884adf..6a973dc5f8 100644 --- a/plugins/code-metrics/skills/audit-type-debt/SKILL.md +++ b/plugins/code-metrics/skills/audit-type-debt/SKILL.md @@ -131,6 +131,9 @@ the collectors. aborting the lane. mypy's module walk stops at a directory whose name is not a Python identifier, so two same-named files under two hyphenated directories (`my-pkg/mod.py`, `other-pkg/mod.py`) still collide; that run reads `unavailable` with the duplicate-module - message. + message. mypy accepts the flag only while namespace packages are on (its default), so a + consumer config that turns them off makes the run repeat without it, in mypy's own naming + (packages from `__init__.py` files), and the run row's reason says so; same-named files collide + again in that mode. - mypy runs with its cache disabled (`--cache-dir` set to the platform's null device), so no `.mypy_cache/` is written into the working tree. A one-shot report gains nothing from the cache.