Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions code_review_graph/flows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
71 changes: 71 additions & 0 deletions tests/test_flows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
69 changes: 69 additions & 0 deletions tests/test_refactor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Loading