Skip to content

Comments

Skip AddAnnotationProcessor when processor already in effective POM via ancestor#6759

Open
mcebanupgrade wants to merge 4 commits intoopenrewrite:mainfrom
mcebanupgrade:fix/add-annotation-processor-effective-pom-check
Open

Skip AddAnnotationProcessor when processor already in effective POM via ancestor#6759
mcebanupgrade wants to merge 4 commits intoopenrewrite:mainfrom
mcebanupgrade:fix/add-annotation-processor-effective-pom-check

Conversation

@mcebanupgrade
Copy link
Contributor

Summary

AddAnnotationProcessor would add a redundant annotation processor entry when an in-reactor parent POM inherits the configuration from a parent outside the reactor. The processor was present in the effective POM (via merged ancestor configs) but not in the in-reactor parent's own XML, causing a duplicate to be written.

Root cause: The scanner only tracked reactor topology — it had no visibility into whether the annotation processor was already inherited from an external ancestor via the effective POM.

Fix:

  • In the scanner phase, compare ResolvedPom.getPlugins()/getPluginManagement() (merged effective lists from all ancestors) against Pom.getPlugins()/getPluginManagement() (current POM's own XML only)
  • If processor is in effective POM but not in own XML → mark path as alreadyConfiguredInEffectivePomPaths
  • In the visitor phase, skip any path in alreadyConfiguredInEffectivePomPaths

Added hasAnnotationProcessor(List<Plugin>) and isMatchingProcessor(JsonNode) helpers for the effective POM check, using Jackson JsonNode traversal of Plugin.getConfiguration() (consistent with how ResolvedPom exposes plugin config).

Test plan

  • New EffectivePomCheck.doesNotAddWhenAlreadyInEffectivePomViaAncestor test in AddAnnotationProcessorTest: 3-level hierarchy where grandparent has the processor in pluginManagement, intermediate parent inherits it via effective POM but has no own XML config — intermediate parent must NOT be modified

🤖 Generated with Claude Code

…ia ancestor

When an in-reactor parent POM inherits the annotation processor configuration from
a parent outside the reactor (present in the effective POM but not in own XML),
the recipe would previously add a redundant entry, duplicating what the ancestor
already provides.

Fix: in the scanner phase, compare `ResolvedPom.getPlugins()/getPluginManagement()`
(the merged effective lists including ancestor POMs) against
`Pom.getPlugins()/getPluginManagement()` (the current POM's own XML only).
If the processor is in the effective POM but not in the own XML, mark the path as
already-configured and skip it in the visitor phase.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mcebanupgrade
Copy link
Contributor Author

The CI failure in rewrite-gradle:test > RemoveRedundantDependencyVersionsTest.unmanagedDependency() is unrelated to this PR.

Evidence:

  1. This PR only modifies two files, both in rewrite-maven:
    • rewrite-maven/src/main/java/org/openrewrite/maven/AddAnnotationProcessor.java
    • rewrite-maven/src/test/java/org/openrewrite/maven/AddAnnotationProcessorTest.java
  2. The failing test is in rewrite-gradle:test — a separate module with no dependency on rewrite-maven
  3. The failure is a Kotlin stdlib version mismatch (2.2.0 vs 1.9.0 expected) in a Gradle tooling API test that resolves Spring Boot platform dependencies — nothing to do with Maven annotation processor configuration
  4. All recent CI runs on main were green

This appears to be a pre-existing flaky test. Could a maintainer re-run the CI?

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mcebanupgrade
Copy link
Contributor Author

Local test results: All 23 AddAnnotationProcessorTest tests pass locally (installed Java 8 toolchain to run rewrite-maven:test):

AddAnnotationProcessorTest$EffectivePomCheck:          1 test,  0 failures
AddAnnotationProcessorTest$SeparatedAggregatorAndParent: 5 tests, 0 failures
AddAnnotationProcessorTest$CombinedAggregatorAndParent:  8 tests, 0 failures
AddAnnotationProcessorTest$SingleModuleProject:          5 tests, 0 failures
AddAnnotationProcessorTest (top-level):                  4 tests, 0 failures

CI flakiness confirmation: The second CI run failed with a completely different error — GradleConnectionException: Could not fetch model of type 'OpenRewriteModelProxy' using connection to Gradle distribution 'https://services.gradle.org/distributions/gradle-8.6-bin.zip' (a network failure downloading Gradle 8.6 in rewrite-gradle-tooling-model:model:test). Different test, different module, different root cause — confirming these are pre-existing flaky infrastructure tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@timtebeek
Copy link
Member

Thanks for the fix & validation @mcebanupgrade ! Any reason your PR is still marked as draft? Can it be reviewed and merged?

@mcebanupgrade mcebanupgrade marked this pull request as ready for review February 18, 2026 22:28
@mcebanupgrade
Copy link
Contributor Author

Hi @timtebeek !
I was having some issues with CI last night and had to put it on pause. The tests seem to pass after a couple of retries 🤷
I was able to confirm locally that this fix indeed fixes the issue I was having, so the PR is ready for review now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants