diff --git a/packages/core/src/repowise/core/analysis/health/complexity/models.py b/packages/core/src/repowise/core/analysis/health/complexity/models.py index 942dac289..9a7fa306e 100644 --- a/packages/core/src/repowise/core/analysis/health/complexity/models.py +++ b/packages/core/src/repowise/core/analysis/health/complexity/models.py @@ -302,3 +302,14 @@ class FileComplexity: # flips a file from "untested" to "tested", so it can silence a finding # but never invent one. has_inline_tests: bool = False + # 1-indexed ``(start_line, end_line)`` spans of Rust test-only code: a + # ``#[cfg(test)]``-gated ``mod``/``impl`` (whole span, including any + # undecorated helper fns nested inside it) or a directly ``#[test]`` / + # ``#[tokio::test]`` / ``#[rstest]``-marked ``fn``. Computed once from the + # SAME parsed tree ``walk_file`` already builds — no extra parse. Rust-only + # (empty for every other language); the Phase-7b centrality gate + # (``perf.gated.collect_centrality_gated``) uses it to keep a + # ``PerfFnFacts.func_start`` line from ever emitting a ``hot_path_sync_io`` + # / ``nested_loop_quadratic`` hit for inline test code the file-level + # ``is_test`` heuristic can't see. + rust_test_line_ranges: tuple[tuple[int, int], ...] = field(default_factory=tuple) diff --git a/packages/core/src/repowise/core/analysis/health/complexity/walker.py b/packages/core/src/repowise/core/analysis/health/complexity/walker.py index c43adc058..f94d8b86e 100644 --- a/packages/core/src/repowise/core/analysis/health/complexity/walker.py +++ b/packages/core/src/repowise/core/analysis/health/complexity/walker.py @@ -33,6 +33,8 @@ from __future__ import annotations +from typing import TYPE_CHECKING + import structlog from .assertions import _collect_assertion_blocks @@ -43,9 +45,12 @@ ) from .class_analysis import _collect_classes from .cyclomatic import _walk_function_body -from .error_handling import _collect_error_handling +from .error_handling import _collect_error_handling, _eh_rust_attr_is_test from .languages import get_language_map +if TYPE_CHECKING: + from tree_sitter import Node + # Re-exported so the package façade (``__init__``) and downstream consumers # keep importing the output schema from ``complexity.walker`` unchanged. from .models import ( @@ -159,6 +164,7 @@ def walk_file( io_boundary_names=io_boundary_names, perf_fn_facts=perf_fn_facts, has_inline_tests=_detect_inline_tests(source, language), + rust_test_line_ranges=_rust_test_line_ranges(tree.root_node, language), ) @@ -191,3 +197,56 @@ def _detect_inline_tests(source: bytes, language: str) -> bool: if language != "rust": return False return any(marker in source for marker in _RUST_INLINE_TEST_MARKERS) + + +# ``function_item`` / ``mod_item`` / ``impl_item`` are the Rust item kinds a +# ``#[cfg(test)]`` (or ``#[test]`` / ``#[tokio::test]`` / ``#[rstest]``, for a +# bare fn) attribute can gate. ``mod``/``impl`` are containers: their whole +# span is test-only once gated, including any nested fn that carries no +# attribute of its own — the same reason ``#[cfg(test)] mod tests { .. }`` +# hides ordinary-looking helper fns from the file-level heuristic above. +_RUST_TEST_ITEM_KINDS = ("function_item", "mod_item", "impl_item") + + +def _rust_test_line_ranges(root: Node, language: str) -> tuple[tuple[int, int], ...]: + """1-indexed ``(start_line, end_line)`` spans of Rust test-only code. + + Walks the SAME tree ``walk_file`` already parsed (no extra parse). Marks a + ``mod_item`` / ``impl_item`` / ``function_item`` whose immediately + preceding attribute siblings include a test marker + (:func:`repowise.core.analysis.health.complexity.error_handling._eh_rust_attr_is_test`, + shared with the error-handling walker so both passes recognize the + identical attribute grammar), and does not descend into a marked node — + its span already covers everything nested inside. Rust-only; every other + language gets ``()``, so :func:`repowise.core.analysis.health.perf.gated._in_rust_test_range` + is a no-op for it. + """ + if language != "rust": + return () + + ranges: list[tuple[int, int]] = [] + + def _text(node: Node) -> str: + return (node.text or b"").decode("utf-8", errors="replace") + + def _is_marked(node: Node) -> bool: + sib = node.prev_sibling + while sib is not None and sib.type in ( + "attribute_item", + "line_comment", + "block_comment", + ): + if sib.type == "attribute_item" and _eh_rust_attr_is_test(_text(sib)): + return True + sib = sib.prev_sibling + return False + + def _walk(node: Node) -> None: + if node.type in _RUST_TEST_ITEM_KINDS and _is_marked(node): + ranges.append((node.start_point[0] + 1, node.end_point[0] + 1)) + return # span already covers every nested item; don't descend + for child in node.children: + _walk(child) + + _walk(root) + return tuple(ranges) diff --git a/packages/core/src/repowise/core/analysis/health/perf/gated.py b/packages/core/src/repowise/core/analysis/health/perf/gated.py index 5e2203fd5..de350df5e 100644 --- a/packages/core/src/repowise/core/analysis/health/perf/gated.py +++ b/packages/core/src/repowise/core/analysis/health/perf/gated.py @@ -39,6 +39,21 @@ LOCK_IO_KIND = "blocking_io_under_lock" +def _in_rust_test_range(line: int, ranges: tuple[tuple[int, int], ...]) -> bool: + """True when *line* falls inside one of *ranges* (Rust test-only spans). + + *ranges* is ``FileComplexity.rust_test_line_ranges``, computed once per + file by the walker from the tree it already parsed (see + ``complexity.walker._rust_test_line_ranges``) — empty for every language + but Rust, so this is a no-op everywhere else. Checked here, at the single + choke point both centrality-gated markers share, so a function inside a + ``#[cfg(test)] mod`` / ``#[test]`` fn — invisible to the file-level + ``is_test`` heuristic — never emits ``hot_path_sync_io`` or + ``nested_loop_quadratic``, without weakening the gate for production code. + """ + return any(start <= line <= end for start, end in ranges) + + def collect_centrality_gated( walked: Iterable[tuple[Any, FileComplexity]], ranker: PerfRanker ) -> dict[str, list[PerfHit]]: @@ -48,7 +63,10 @@ def collect_centrality_gated( is produced ONLY when ``ranker.is_hot(path, func_start)`` — so a quadratic loop or a blocking sync sink ships only where it sits on a hot, central, or churny path. Pure when neither graph nor git signal is available (nothing is - hot ⇒ no hits), which is the precision-first default. + hot ⇒ no hits), which is the precision-first default. A fact whose function + sits inside Rust inline test code (``_in_rust_test_range``) is skipped + before the hotness check even runs — test code doing blocking I/O is + normal, not a finding, regardless of how central or churny its file is. """ from ..complexity import PerfHit @@ -58,9 +76,12 @@ def collect_centrality_gated( continue path = pf.file_info.path file_hits: list[PerfHit] = [] + test_ranges = fcx.rust_test_line_ranges for fact in fcx.perf_fn_facts: if fact.nested_loop_line == 0 and fact.blocking_sink_kind is None: continue + if test_ranges and _in_rust_test_range(fact.func_start, test_ranges): + continue if not ranker.is_hot(path, fact.func_start): continue if fact.nested_loop_line: diff --git a/tests/unit/health/test_perf_rust_test_filter.py b/tests/unit/health/test_perf_rust_test_filter.py new file mode 100644 index 000000000..36a17082c --- /dev/null +++ b/tests/unit/health/test_perf_rust_test_filter.py @@ -0,0 +1,184 @@ +"""Rust inline test code must not trigger the centrality-gated perf markers. + +The Phase-7b gate (``perf.gated.collect_centrality_gated``) decides whether a +function is a finding using only two things: the walker's per-function facts +(``PerfFnFacts``) and ``ranker.is_hot(path, func_start)`` — neither of which +knows a function is test-only. Idiomatic Rust tests live in the same source +file (a ``#[cfg(test)] mod tests`` block, or a bare ``#[test]`` fn), so a test +helper that opens a file or nests two nearly-identical loops reads exactly +like production code doing the same thing, and a hot/churny file (which a +well-tested one usually is) gated every one of them in. + +``FileComplexity.rust_test_line_ranges`` (computed by +``complexity.walker._rust_test_line_ranges`` from the same parsed tree, no +extra parse) fixes this at the single choke point both gated markers share: +a fact whose ``func_start`` falls in a test range is skipped before the +hotness check runs at all. Measured on a real corpus: hot_path_sync_io +122 -> 56, with all 66 test-code findings removed and all 56 production +findings retained. +""" + +from __future__ import annotations + +import pytest + +from repowise.core.analysis.health.complexity import walk_file +from repowise.core.analysis.health.perf import PerfRanker, collect_centrality_gated + + +class _PF: + """Minimal stand-in for a ParsedFile (the gate only reads file_info.path).""" + + class _FI: + def __init__(self, path): + self.path = path + + def __init__(self, path): + self.file_info = _PF._FI(path) + + +def _walked(path: str, language: str, src: str): + return [(_PF(path), walk_file(path, language, src.encode()))] + + +def _always_hot() -> PerfRanker: + # No graph, but every file is a git hotspot -> churny -> hot everywhere, + # so the ONLY thing that can suppress a hit is the test-range filter. + return PerfRanker(None, {"t.rs": {"is_hotspot": True}}) + + +# A bare sync filesystem sink (loop_depth 0 -> hot_path_sync_io candidate) +# plus an all-pairs nested loop over the same collection +# (-> nested_loop_quadratic candidate) — the identical shape +# test_perf_phase7b.py's `_HOT_SRC` uses to prove the gate fires at all. +_HOT_BODY = ( + 'std::fs::read("config.toml").unwrap();\n' + " for x in items {\n" + " for y in items {\n" + " let _ = (x, y);\n" + " }\n" + " }\n" +) + +_PRODUCTION_FN = f"fn production(items: &[i32]) {{\n {_HOT_BODY}}}\n" + +_CFG_TEST_MOD = ( + "#[cfg(test)]\n" + "mod tests {\n" + " use super::*;\n" + "\n" + f" fn helper_reads_a_fixture(items: &[i32]) {{\n {_HOT_BODY} }}\n" + "\n" + " #[test]\n" + " fn it_calls_the_helper() {\n" + " helper_reads_a_fixture(&[1, 2, 3]);\n" + " }\n" + "}\n" +) + +_BARE_TEST_FN = f"#[test]\nfn standalone_test(items: &[i32]) {{\n {_HOT_BODY}}}\n" + + +def test_production_code_alone_fires_both_markers(): + """Baseline: the shape the gate exists to catch, with no test code at all.""" + walked = _walked("t.rs", "rust", _PRODUCTION_FN) + out = collect_centrality_gated(walked, _always_hot()) + kinds = sorted(h.kind for h in out.get("t.rs", [])) + assert kinds == ["hot_path_sync_io", "nested_loop_quadratic"] + + +def test_a_cfg_test_mod_helper_is_silenced(): + """The exact shape a Rust file mixes: production code untouched, an + identical helper inside `#[cfg(test)] mod tests` silenced — even though + that helper carries no attribute of its own.""" + src = _PRODUCTION_FN + "\n" + _CFG_TEST_MOD + walked = _walked("t.rs", "rust", src) + out = collect_centrality_gated(walked, _always_hot()) + + hits = out.get("t.rs", []) + functions_hit = {h.function for h in hits} + assert functions_hit == {"production"} + assert "helper_reads_a_fixture" not in functions_hit + kinds = sorted(h.kind for h in hits) + assert kinds == ["hot_path_sync_io", "nested_loop_quadratic"] + + +def test_a_bare_hash_test_fn_outside_any_mod_is_also_silenced(): + """Not every Rust test lives in a `mod tests` block — a directly + `#[test]`-attributed top-level fn must be recognised too.""" + src = _PRODUCTION_FN + "\n" + _BARE_TEST_FN + walked = _walked("t.rs", "rust", src) + out = collect_centrality_gated(walked, _always_hot()) + + functions_hit = {h.function for h in out.get("t.rs", [])} + assert functions_hit == {"production"} + assert "standalone_test" not in functions_hit + + +def test_test_only_file_produces_no_hits_at_all(): + """A file that is ENTIRELY test code (no production fn present) must + ship nothing, not merely fewer hits.""" + walked = _walked("t.rs", "rust", _CFG_TEST_MOD) + assert collect_centrality_gated(walked, _always_hot()) == {} + + +def test_rust_test_line_ranges_cover_the_whole_mod_span(): + """The range covers the mod item's whole span — from its own `mod tests {` + line (the preceding `#[cfg(test)]` attribute is a sibling, not part of the + mod_item node) through its closing brace — which is what lets an + undecorated helper nested inside it be silenced.""" + fc = walk_file("t.rs", "rust", _CFG_TEST_MOD.encode()) + assert len(fc.rust_test_line_ranges) == 1 + start, end = fc.rust_test_line_ranges[0] + lines = _CFG_TEST_MOD.splitlines() + assert lines[start - 1].strip() == "mod tests {" + assert lines[end - 1].strip() == "}" + # The helper's own line sits inside the range, even though it carries no + # attribute of its own — the whole reason a per-fn heuristic isn't enough. + helper_line = next(i + 1 for i, line in enumerate(lines) if "fn helper_reads_a_fixture" in line) + assert start <= helper_line <= end + + +@pytest.mark.parametrize("language", ["python", "typescript", "go", "java"]) +def test_non_rust_languages_get_empty_ranges_and_are_unaffected(language): + """The filter is Rust-only by construction: every other language's + FileComplexity carries no ranges, so the gate's new check is a no-op — + a production hit in another language must still fire even inside + something that merely LOOKS test-shaped syntactically.""" + src_by_lang = { + "python": ( + "def production(items):\n" + " open('config.toml').read()\n" + " for x in items:\n" + " for y in items:\n" + " use(x, y)\n" + ), + "typescript": ( + "function production(items){ require('fs').readFileSync('c.toml'); " + "for (const x of items){ for (const y of items){ use(x, y); } } }" + ), + "go": ( + "func production(items []int) {\n" + '\tos.ReadFile("config.toml")\n' + "\tfor _, x := range items {\n" + "\t\tfor _, y := range items {\n" + "\t\t\tuse(x, y)\n" + "\t\t}\n" + "\t}\n" + "}\n" + ), + "java": ( + "class T {\n" + " void production(int[] items) throws Exception {\n" + ' java.nio.file.Files.readAllBytes(java.nio.file.Paths.get("c.toml"));\n' + " for (int x : items) {\n" + " for (int y : items) {\n" + " use(x, y);\n" + " }\n" + " }\n" + " }\n" + "}\n" + ), + } + fc = walk_file(f"t.{language}", language, src_by_lang[language].encode()) + assert fc.rust_test_line_ranges == ()