[Bug]: dead-code reports @Override methods as dead, and JUnit lifecycle methods outside test-named files

code-review-graph version

Measured on staging at f0e4eb7 (post-2.3.8). The numbers below differ from what 2.3.8 shows, because #1023 has since removed a large part of the same family — see What #1023 already fixed.

Operating system

macOS

Python version

3.13

AI platform

claude-code

Steps to reproduce

Reproducer 1 — @Override in production code.

src/main/java/demo/Widget.java

package demo;

import java.util.Objects;

public class Widget {
  private final String id;

  public Widget(String id) { this.id = id; }

  public String getId() { return id; }

  @Override
  public int hashCode() { return Objects.hash(id); }

  @Override
  public boolean equals(Object other) {
    return other instanceof Widget && Objects.equals(id, ((Widget) other).id);
  }
}

src/test/java/demo/WidgetTest.java

package demo;

import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class WidgetTest {
  private Widget widget;

  @BeforeEach
  void setUp() { widget = new Widget("a"); }

  @Test
  void keepsItsId() { assertEquals("a", widget.getId()); }
}

git init && git add -A && git commit -m repro
code-review-graph build --repo . --data-dir /tmp/dc-data
code-review-graph dead-code --repo . --data-dir /tmp/dc-data

Reproducer 2 — a JUnit lifecycle method in a file the test-file exclusion does not cover. A shared harness deliberately lives in main so that several test modules can depend on it:

src/main/java/demo/AbstractWidgetHarness.java

package demo;

import org.junit.jupiter.api.BeforeEach;

/** Shared harness, lives in main so several test modules can depend on it. */
public abstract class AbstractWidgetHarness {
  protected String fixture;

  @BeforeEach
  void prepareFixture() { fixture = "ready"; }
}

Expected vs actual behavior

Reproducer 1, staging at f0e4eb7:

Dead code: 2 item(s); showing 2
  [Function] Widget    (src/main/java/demo/Widget.java:8)
  [Function] hashCode  (src/main/java/demo/Widget.java:16)

hashCode carries @Override. Its caller is Object's contract — HashMap, Objects.hash, anything that hashes a Widget. It is not dead, and no hierarchy walk can rescue it, because the base is the JDK and therefore outside the graph. (Widget at line 8 is the constructor; that is a separate defect, noted at the end.)

Reproducer 2, same build:

Dead code: 2 item(s); showing 2
  [Class] AbstractWidgetHarness   (src/main/java/demo/AbstractWidgetHarness.java:6)
  [Function] prepareFixture       (src/main/java/demo/AbstractWidgetHarness.java:9)

prepareFixture carries @BeforeEach. Its caller is the JUnit runner.

The annotation is already stored on the node in both cases:

sqlite> SELECT name, extra FROM nodes WHERE kind IN ('Function','Test') AND extra != '{}';
hashCode       | {"decorators": ["Override"]}
equals         | {"decorators": ["Override"]}
prepareFixture | {"decorators": ["BeforeEach"]}

_is_entry_point already consults extra.decorators through _has_framework_decorator. The patterns list in flows.py covers Spring, Django, Click, Celery, Angular, Express and pytest.fixture — but has no entry for Override, and none for JUnit or TestNG.

What #1023 already fixed

Against stock 2.3.8 this reproducer reports four items, adding [Class] WidgetTest and [Function] setUp. On current staging both are gone: find_dead_code now reads test-ness from the path relative to the repository root, and src/test/java/demo/WidgetTest.java is recognised. So the part of this family that lives in test-named files is already handled.

What remains is the part that does not: @Override in production code, and JUnit annotations on methods that sit outside a test-named file — reproducer 2, which is a real pattern in multi-module Java builds.

Root cause and fix

Two patterns added to _FRAMEWORK_DECORATOR_PATTERNS in flows.py:

# 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. Anchored so it cannot match unrelated names
# such as override_settings, handled below.
re.compile(r"^Override$"),
# 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)$"
),

Both reproducers drop from 2 findings to 1.

Measured on a real 125-file multi-module Java library, on staging:

The removal set is exactly the 54 @Override methods, nothing else. Spot-checking them: getHeader and getStatusCode implement the Azure Functions HttpResponseMessage interface; getSupportedAnnotationTypes overrides javax.annotation.processing.AbstractProcessor. All are called by a framework, none from the repository.

On this particular library the JUnit patterns change nothing — its tests all live in test-named files, which #1023 already covers. Reproducer 2 is the case they do cover.

Additional context

What the patch deliberately does not fix. Of the 120 that remain, most are public with no @Override. In a library those are unknowable, not dead: the consumers are in other repositories and a single-repository graph cannot see them. I verified one — a public test-helper class reported dead, used by six other repositories in the same workspace. The fix there is not detection but labelling, and there is a precedent in this codebase: the confidence key added by #884 explains why a query result is empty instead of presenting the zero as proof. dead-code on a library could say the same thing about its public surface.

Two further false-positive families the reproducers surface, which this patch does not fix:

1. The harness class itself is still reported. AbstractWidgetHarness has no class-level annotation, and the class-level exclusion only checks _has_framework_decorator. A class whose methods are all entry points is not dead.
2. Constructors are reported. Widget at line 8 is the constructor, and new Widget("a") is called in the test. The edge exists, but its target is the bare name Widget, never the constructor node Widget.java::Widget.Widget, which therefore has no incoming edge at all.

A cheap detector for a related invention, found while looking into this and worth its own check: SELECT COUNT(*) FROM edges WHERE kind='CALLS' AND source_qualified = target_qualified — a method calling itself. On the same library that is 39 edges, and of eight sampled at random seven are not recursion: they are super.toString() and casts used as receivers (((SomeType) x).getY()), where the parser records no receiver at all (extra is {}) and a target is still built on the enclosing class. That is #984's invented target with zero evidence attached, and a self-loop is a one-line way to find them.

Related

• #1023 — removed the test-file half of this family. This issue is the remainder.
• #984 — call targets are invented when they cannot be worked out. The self-loop family above is an instance of it.
• #884 — the confidence key on empty query results; the same idea applied to dead-code would cover the unknowable public surface.

Patch

Against staging at f0e4eb7.

--- a/code_review_graph/flows.py
+++ b/code_review_graph/flows.py
@@ -42,8 +42,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),