Treat @Override and JUnit annotations as entry points in dead-code - #1035
aala-conga wants to merge 2 commits into
Conversation
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 tirth8205#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 <noreply@anthropic.com>
|
A dead public class MultiHandler extends com.external.BaseHandler {
@Override public void handleLive() { doWork(); }
@Override public void handleStale() { doWork(); }
}
// App.main constructs MultiHandler and calls handleLive() only
Half of the JUnit alternation cannot fire.
The flows-side cost is real and the body does not size it. Every
The fix is Java-annotation-only while the body frames the problem generically. Kotlin The core claim reproduces well outside your own tests. On google/gson To get this mergeable: either drop |
code-review-graph reviewOverall risk: 0.15 (LOW) — 13 changed function(s)/class(es), 3 affected flow(s), 3 test gap(s) Risk-scored changes
Affected execution flows
Test gaps
Token savings: this graph-backed report used ~16,476 fewer tokens (~63%) than reading every changed file in full (estimated, chars/4 approximation). Powered by code-review-graph — local-first analysis; no code leaves the CI runner. |
This branch has not been deployed
2 participants
Fixes #1034.
dead-codereports two families of method whose caller is a contract or a test runner, not a callsite in the repository:
@Overridein production code. An override implements a supertype contract, so its caller isthat contract — a supertype-typed reference, a framework, or the runtime itself for
Object.hashCode/equals/toString. No hierarchy walk can rescue these, because the base isoften the JDK and therefore outside the graph.
mainso several test modules can depend on it, so Treat every symbol in a test file as test code #1023's test-file exclusion does not cover it.Both annotations are already on the node — the parser stores them in
extra['decorators']— and_is_entry_pointalready consults them through_has_framework_decorator. The patterns list coveredSpring, Django, Click, Celery, Angular, Express and
pytest.fixture, but had no entry forOverrideand none for JUnit or TestNG. This adds two patterns, both anchored so they cannot match unrelated
names such as
override_settings, which is handled by its own pattern further down the list.Relationship to #1023
Against stock 2.3.8 this looked like a four-item problem; on current
stagingit is a two-item one,because #1023 already removed everything in the family that lives in a test-named file. What is left
is
@Overridein production code, and JUnit annotations on methods outside such files.Measured
A 125-file multi-module Java library, built on
stagingand again with this patch:staging@OverrideThe removal set is exactly the 54
@Overridemethods, nothing else moved. Spot-checking them:getHeaderandgetStatusCodeimplement the Azure FunctionsHttpResponseMessageinterface,getSupportedAnnotationTypesoverridesjavax.annotation.processing.AbstractProcessor— all calledby a framework, none from the repository.
On this library the JUnit patterns change nothing, because its tests all live in test-named files.
The second reproducer in the issue is the case they cover.
Tests
Seven cases, in the two modules that share the predicate —
detect_entry_pointsinflows.pyandfind_dead_codeinrefactor.py, which both reach it through_has_framework_decorator.Five of them fail on
stagingand pass with the patch:The other two pass in both directions on purpose — they are guards rather than proofs:
test_override_pattern_is_anchored(a decorator namedOverridablemust not match^Override$) andtest_a_genuinely_unreferenced_method_is_still_reported(the patch must narrow the false positives,not silence the query).
Each annotated method in the
flowstests is given an incomingCALLSedge, so that rule 1 ofdetect_entry_points— no callers — cannot carry the assertion on its own and the annotation is theonly thing that can make the method an entry point. Note that
detect_entry_pointsskips test filesunless
include_tests=True; the JUnit cases are therefore written in ordinary files, which is alsothe case the patch is for.
Full suite, fresh clone of
stagingatf0e4eb7, Python 3.13,pip install -e ".[dev]":staging, untouched — baseline3963 passed, 773 skipped, 2 xfailed, 2 xpassed(349 s)(339 s)Not addressed here
Two further false positives the reproducers surface, both out of scope for this change: the harness
class is still reported, because the class-level exclusion only checks
_has_framework_decoratorand not whether every method inside is an entry point; and constructors are reported, because a
new Widget(...)edge targets the bare nameWidgetrather than the constructor node, whichtherefore has no incoming edge at all.
🤖 Generated with Claude Code