From d414c0b563e96e1a37026391ad6c6da32e5eff36 Mon Sep 17 00:00:00 2001 From: Anthony Ala Date: Thu, 17 Sep 2026 18:30:28 +0200 Subject: [PATCH] Treat @Override and JUnit annotations as entry points in dead-code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dead-code reports two families of method whose caller is a contract or a test runner rather than a call site in the repository: - @Override in production code. An override implements a supertype contract, so its caller is that contract. No hierarchy walk can rescue these, because the base is often the JDK and outside the graph. - JUnit/TestNG annotations outside a test-named file. A shared harness deliberately lives in main so several test modules can depend on it, so the test-file exclusion from #1023 does not cover it. Both annotations are already stored on the node in extra['decorators'], and _is_entry_point already consults them through _has_framework_decorator; the patterns list simply had no entry for Override, nor any for JUnit or TestNG. Both new patterns are anchored so they cannot match unrelated names such as override_settings, which has its own pattern further down the list. Measured on a 125-file multi-module Java library: symbols reported dead 174 -> 120, the removal set being exactly the 54 @Override methods, with nothing added. Seven tests across the two modules that share the predicate. Five fail on staging; the remaining two are guards that hold in both directions — the anchoring of ^Override$, and the requirement that a genuinely unreferenced method still be reported. Co-Authored-By: Claude Opus 5 --- code_review_graph/flows.py | 12 +++++++ tests/test_flows.py | 71 ++++++++++++++++++++++++++++++++++++++ tests/test_refactor.py | 69 ++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+) diff --git a/code_review_graph/flows.py b/code_review_graph/flows.py index add84faac..12d425e37 100644 --- a/code_review_graph/flows.py +++ b/code_review_graph/flows.py @@ -43,8 +43,20 @@ re.compile(r"receiver", re.IGNORECASE), re.compile(r"api_view", re.IGNORECASE), re.compile(r"\baction\b", re.IGNORECASE), + # An override implements a supertype contract, so its caller is that + # contract: a supertype-typed reference, a framework, or the runtime itself + # for Object.hashCode/equals/toString. It is never dead by virtue of having + # no explicit call site, and the base is often outside the graph (the JDK), + # so no hierarchy walk can rescue it either. Anchored so it cannot match + # unrelated names such as override_settings, handled below. + re.compile(r"^Override$"), # Testing re.compile(r"pytest\.(fixture|mark)"), + # JUnit / TestNG lifecycle and test methods are invoked by the runner. + re.compile( + r"^(Test|Before|After|BeforeEach|AfterEach|BeforeAll|AfterAll" + r"|BeforeClass|AfterClass|ParameterizedTest|RepeatedTest|TestFactory)$" + ), re.compile(r"(override_settings|modify_settings)", re.IGNORECASE), # SQLAlchemy / event systems re.compile(r"(event\.)?listens_for", re.IGNORECASE), diff --git a/tests/test_flows.py b/tests/test_flows.py index a549f2fe6..e79bdf67e 100644 --- a/tests/test_flows.py +++ b/tests/test_flows.py @@ -172,6 +172,77 @@ def test_detect_entry_points_agent_tool(self): ep_names = {ep.name for ep in eps} assert "query_health" in ep_names + def test_detect_entry_points_java_override(self): + """@Override implements a supertype contract, so the contract is the caller. + + The base is frequently outside the graph — ``Object.hashCode``/``equals``/ + ``toString`` live in the JDK — so no hierarchy walk can rescue the method + either. ``hashCode`` is given a caller here so that rule 1 (no incoming + CALLS) cannot carry the assertion on its own: the annotation is the only + thing that can make it an entry point. + """ + self._add_func("entry_func") + self._add_func( + "hashCode", + parent="Widget", + language="java", + extra={"decorators": ["Override"]}, + ) + self._add_call("app.py::entry_func", "app.py::Widget.hashCode") + + eps = detect_entry_points(self.store) + assert "hashCode" in {ep.name for ep in eps} + + def test_detect_entry_points_junit_lifecycle(self): + """JUnit lifecycle methods are invoked by the runner, never explicitly. + + Each one is given a caller, for the same reason as above. + """ + self._add_func("entry_func") + for name, annotation in ( + ("setUp", "BeforeEach"), + ("tearDown", "AfterEach"), + ("initAll", "BeforeAll"), + ("cleanAll", "AfterAll"), + ): + self._add_func( + name, + parent="Harness", + language="java", + extra={"decorators": [annotation]}, + ) + self._add_call("app.py::entry_func", f"app.py::Harness.{name}") + + eps = {ep.name for ep in detect_entry_points(self.store)} + assert {"setUp", "tearDown", "initAll", "cleanAll"} <= eps + + def test_detect_entry_points_junit_test_annotation(self): + """@Test on a Java method, whose camelCase name carries no ``test`` prefix. + + ``_matches_entry_name`` keys on a ``test``-prefixed name, which Java's + convention does not produce. + """ + self._add_func("entry_func") + self._add_func( + "keepsItsId", + parent="WidgetCases", + language="java", + extra={"decorators": ["Test"]}, + ) + self._add_call("app.py::entry_func", "app.py::WidgetCases.keepsItsId") + + eps = detect_entry_points(self.store) + assert "keepsItsId" in {ep.name for ep in eps} + + def test_override_pattern_is_anchored(self): + """``^Override$`` must not match a decorator that merely contains it.""" + self._add_func("entry_func") + self._add_func("helper", extra={"decorators": ["Overridable"]}) + self._add_call("app.py::entry_func", "app.py::helper") + + eps = detect_entry_points(self.store) + assert "helper" not in {ep.name for ep in eps} + def test_detect_entry_points_alembic(self): """upgrade/downgrade functions are entry points.""" self._add_func("upgrade") diff --git a/tests/test_refactor.py b/tests/test_refactor.py index c3e314e86..b92f48467 100644 --- a/tests/test_refactor.py +++ b/tests/test_refactor.py @@ -1099,3 +1099,72 @@ def test_if_main_block_caller_prevents_dead_code_flag(self, tmp_path): dead = find_dead_code(self.store) dead_names = {d["name"] for d in dead} assert "launch" not in dead_names + + +class TestFindDeadCodeJavaAnnotations: + """``dead-code`` must not report methods whose caller is a contract or a runner. + + Both annotations are already stored on the node — the parser captures them in + ``extra['decorators']`` — and ``_is_entry_point`` already consults them through + ``_has_framework_decorator``. The patterns list simply had no entry for + ``Override``, nor any for JUnit/TestNG. + + Neither case is rescued by the test-file exclusion added in #1023: + ``Widget.java`` is production code, and a shared JUnit harness deliberately + lives in main so that several test modules can depend on it. See: #1034 + """ + + def setup_method(self): + self.tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) + self.tmp.close() # release the handle before GraphStore reopens it on Windows + self.store = GraphStore(self.tmp.name) + self._seed() + + def teardown_method(self): + self.store.close() + Path(self.tmp.name).unlink(missing_ok=True) + + def _seed(self): + for path in ("/repo/Widget.java", "/repo/AbstractWidgetHarness.java"): + self.store.upsert_node(NodeInfo( + kind="File", name=path, file_path=path, + line_start=1, line_end=100, language="java", + )) + # Overrides of Object's contract: called by HashMap, Objects.hash, the + # runtime — never from this repository. + for name, line in (("hashCode", 16), ("equals", 19)): + self.store.upsert_node(NodeInfo( + kind="Function", name=name, file_path="/repo/Widget.java", + line_start=line, line_end=line + 2, language="java", + parent_name="Widget", extra={"decorators": ["Override"]}, + )) + # Invoked by the JUnit runner, in a file the test-file exclusion does + # not cover. + self.store.upsert_node(NodeInfo( + kind="Function", name="prepareFixture", + file_path="/repo/AbstractWidgetHarness.java", + line_start=9, line_end=10, language="java", + parent_name="AbstractWidgetHarness", + extra={"decorators": ["BeforeEach"]}, + )) + # A genuinely unreferenced method, to prove the query still reports. + self.store.upsert_node(NodeInfo( + kind="Function", name="unusedHelper", file_path="/repo/Widget.java", + line_start=30, line_end=32, language="java", parent_name="Widget", + )) + self.store.commit() + + def _dead(self) -> set[str]: + return {d["name"] for d in find_dead_code(self.store, root="/repo")} + + def test_override_is_not_dead_code(self): + dead = self._dead() + assert "hashCode" not in dead + assert "equals" not in dead + + def test_junit_lifecycle_outside_a_test_file_is_not_dead_code(self): + assert "prepareFixture" not in self._dead() + + def test_a_genuinely_unreferenced_method_is_still_reported(self): + """The fix must narrow the false positives, not silence the query.""" + assert "unusedHelper" in self._dead()