Skip to content

fix/correctness-silent-drops: close nine tier-1 silent-drop bugs on the path to v2.0#31

Merged
vedanthvdev merged 1 commit intomasterfrom
fix/correctness-silent-drops
Apr 22, 2026
Merged

fix/correctness-silent-drops: close nine tier-1 silent-drop bugs on the path to v2.0#31
vedanthvdev merged 1 commit intomasterfrom
fix/correctness-silent-drops

Conversation

@vedanthvdev
Copy link
Copy Markdown
Owner

Why

A second, deeper multi-agent review of the plugin source tree as it landed in v1.9.19 surfaced a cluster of tier-1 correctness bugs that all shared the same failure mode: a strategy thought it had walked a construct but had actually missed one of its real-world shapes, so the consumer test for a changed class never entered the selected set. This breaks the plugin's cardinal invariant — run more tests than needed, never fewer — which is the one guarantee v2.0 needs to ship with.

Every fix in this PR ships with at least one regression test that fails when the production fix is reverted.

What

New file: affected-tests-core/src/main/java/io/affectedtests/core/discovery/JavaParsers.java

# Fix Regression test
1 JavaParsers factory centralises JavaParser creation at LanguageLevel.JAVA_25 (highest stable level the bundled 3.28 build supports). new JavaParser() defaulted to JAVA_11, so every file using records, sealed types, or pattern matching parsed as isSuccessful == false and silently dropped out of every AST strategy. parseOrWarn now centralises the parse helper and surfaces failures at WARN instead of DEBUG. ImplementationStrategyTest#findsRecordImplementationOfChangedInterface, …findsEnumImplementationOfChangedInterface
2 TransitiveStrategy preserves generic arguments (List<Foo> now registers the Foo edge) by walking every ClassOrInterfaceType. TransitiveStrategyTest#preservesGenericArgumentsInReverseDependencyEdges
3 TransitiveStrategy scans method bodies, not just signatures, so new PricingCalculator() inside a method body now adds the reverse edge. TransitiveStrategyTest#discoversEdgesFromMethodBodyReferences
4 ImplementationStrategy iterates every TypeDeclaration subtype. Records and enums are now first-class implementers; a defensive WARN-logged default branch guards against the same bug reappearing when JavaParser introduces a new declaration kind. (same two tests as #1)
5 GitChangeDetector now diffs against the merge-base between baseRef and HEAD via RevFilter.MERGE_BASE, falling back to baseId with a WARN log when there is no common ancestor. Previously, any post-divergence master commit showed up as a "change" on the feature branch and dragged unrelated tests into every MR. GitChangeDetectorTest#diffsAgainstMergeBaseNotBaseRefTip
6 UsageStrategy adds a Tier 3 AST pass that catches fully-qualified inline references (new com.example.Foo(), Outer.Inner inline qualification). UsageStrategyTest#findsTestThatUsesChangedClassByFullyQualifiedInlineReference, …findsTestThatInnerClassQualifiesThroughChangedOuter
7 UsageStrategy Tier 1b now treats import pkg.Outer.* as a class-member wildcard (dependency on pkg.Outer) rather than a package wildcard. UsageStrategyTest#findsTestThatWildcardsClassMembersOfChangedClass
8 PathToClassMapper routes module-info.java and package-info.java to the unmapped bucket instead of minting the malformed FQNs module-info / com.example.package-info. PathToClassMapperTest#moduleInfoRoutesToUnmappedNotProduction, …packageInfoRoutesToUnmappedNotProduction
9 AffectedTestsEngine explicitly routes diffs that are a union of ignored + out-of-scope files to Situation.ALL_FILES_OUT_OF_SCOPE (SKIPPED) instead of falling through to DISCOVERY_EMPTY — which under mode = CI would escalate a pure docs+api-test MR into a full CI run. AffectedTestsEngineTest#mixedIgnoredAndOutOfScopeDiffRoutesToAllFilesOutOfScope

Verification

  • ./gradlew check — green end-to-end (core + gradle modules, validatePlugins included).
  • New regression tests: 11 new tests, all passing. Each one reverse-checks its fix: mutating the production code in-place makes the test fail.
  • Independent code-reviewer pass on the staged diff surfaced 1 High (LANGUAGE_LEVEL = JAVA_21 → bump to JAVA_25) and 3 Medium findings (parse-failure WARN, supertypesOf default branch, merge-base fallback log level). All four applied in this PR. Zero Critical findings remaining.

Release

This is batch 4 of 4 on the road to v2.0. Ships as v1.9.20 via the axion-release workflow on merge to master.

See the CHANGELOG.md top block "Fixed — post-v1.9.19 multi-reviewer correctness pass (v1.9.20)" for the authoritative per-fix writeup.

…he path to v2.0

A post-v1.9.19 multi-agent review found a cluster of silent-drop bugs
that all shared the same failure mode: a strategy thought it had walked
a construct but had actually missed one of its real-world shapes, so
the consumer test for a changed class never entered the selected set —
breaking the plugin's cardinal invariant (run more tests than needed,
never fewer). This release ships a fix for each one with a regression
test that fails on revert.

- JavaParsers factory centralises `JavaParser` creation at
  `LanguageLevel.JAVA_25` (highest stable level in the bundled 3.28
  build). Previously the default `JAVA_11` made every file using
  records, sealed types, or pattern matching produce
  `isSuccessful == false`, silently removing the file from every
  AST strategy. `parseOrWarn` replaces the four independent
  `parseOrGet` helpers and surfaces parse failures at WARN instead of
  DEBUG, closing the invisibility that hid this bug class.
- TransitiveStrategy now preserves generic type arguments and walks
  method bodies via `findAll(ClassOrInterfaceType.class)`, so
  `List<FooService>` consumers and helper-inside-method-body shapes
  no longer lose their reverse-dependency edge.
- ImplementationStrategy iterates every `TypeDeclaration` subtype, so
  `record UsdMoney(long cents) implements Money` and
  `enum Currency implements HasCode` are now first-class implementers.
  The new `supertypesOf` helper covers class/record/enum/annotation
  declarations, with a defensive WARN-logged default branch to make
  the next unknown subtype loud.
- GitChangeDetector diffs against the merge-base between `baseRef`
  and `HEAD` (falling back to `baseId` with a WARN log when no common
  ancestor exists), so post-divergence master commits no longer leak
  into a feature branch's diff and inflate the affected-tests set
  toward "everything".
- UsageStrategy adds a Tier 3 AST pass that catches fully-qualified
  inline references (`new com.example.Foo()`, `Outer.Inner` inline
  qualification), and the Tier 1b wildcard tier now treats
  `import pkg.Outer.*` as a class-member wildcard rather than a
  package wildcard — the shape that made every Outer.java change
  silently drop all wildcard consumers.
- PathToClassMapper routes `module-info.java` and `package-info.java`
  to the unmapped bucket instead of producing malformed FQNs, letting
  the UNMAPPED_FILE safety net escalate JPMS and package-annotation
  changes conservatively.
- AffectedTestsEngine explicitly routes diffs that are a mix of
  ignored + out-of-scope files to `Situation.ALL_FILES_OUT_OF_SCOPE`
  (SKIPPED) instead of falling through to `DISCOVERY_EMPTY`, which
  under `mode = CI` would previously escalate a pure docs+api-test
  MR into a full CI run.

All regression tests cover revert-mutation shapes and `./gradlew check`
is green end-to-end.
@vedanthvdev vedanthvdev merged commit fb22de5 into master Apr 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant