Fix AddAnnotationProcessor when parent matches by GAV alone#7527
Open
Fix AddAnnotationProcessor when parent matches by GAV alone#7527
Conversation
…ares the LST set When a Maven module has no in-reactor parent in the actual build but its corporate parent POM happens to also be present as a separately ingested LST artifact, AddAnnotationProcessor produces zero changes on the child. parentPomIsProjectPom() is GAV-based, so it returns true and the child is routed to the in-reactor-parent branch and never added to candidateOrphanPaths. In the visitor it is neither parent nor orphan and is skipped, so its own build/plugins is never updated. The new test orphanModuleSharedLstWithExternalParent reproduces this with two independently-rooted mavenProject() blocks (no aggregator between them).
…ents as in-reactor MavenResolutionResult.parentPomIsProjectPom() answers a GAV-only question: "is the parent's GAV among the project poms?" Two POMs co-ingested into the same LST (e.g. an external corp-parent and one of its independently-built modules) match by GAV without sharing any actual Maven reactor, but the scanner used the GAV match alone to route the child away from the orphan branch — resulting in zero changes for the affected module. Defer the routing decision until aggregator data has been collected, then verify the parent-child link by walking aggregator <modules> chains. A link counts only if the child is reachable via some aggregator's reactor BFS; otherwise the child falls into the orphan branch and gets its own build/plugins. Read aggregator membership from the requested (raw) Pom's subprojects, since ResolvedPom.subprojects is unreliable in some LST fixtures and mrr.getModules() lists POMs that declare *this* one as their <parent> rather than ones aggregated by it. Also: - Skip dangling pom-packaging POMs (not claimed as parents, not aggregators) so coincidentally-ingested external parents don't get spurious build/plugins. - Pass the source path as AddPluginVisitor's filePattern only for the reactor-broken-orphan case, so its own parentPomIsProjectPom() guard doesn't refuse to add the plugin. - Simplify the reproducer test: drop the pre-declared maven-compiler-plugin on the child, since AddPluginVisitor does not currently fill in missing configuration on an existing plugin (a separate latent bug; not addressed in this commit).
…notationProcessorPaths When the target POM pre-declares maven-compiler-plugin (e.g. just to set <source>/<target>) but does not configure annotationProcessorPaths, AddAnnotationProcessor produces zero changes today: AddPluginVisitor short-circuits because the plugin already exists, and the inner visitor only operates on existing <annotationProcessorPaths> tags. Two new tests in SingleModuleProject reproduce the gap: - addConfigurationToExistingPluginWithNoConfig — plugin declared with no <configuration> at all. - addAnnotationProcessorPathsToExistingConfiguration — plugin has <configuration> with <source>/<target> but no annotationProcessorPaths. Both expect the recipe to fill in the missing structure. Both fail on the current main with "Recipe was expected to make a change but made no changes." Fix follows in a separate commit.
…g maven-compiler-plugin When the recipe target POM already declared maven-compiler-plugin (e.g. solely to set <source>/<target>), AddPluginVisitor's "plugin already exists" branch left the plugin untouched, and the path-adding inner visitor only operated on existing <annotationProcessorPaths> tags — so nothing got added. This is the second of the two bugs surfacing on top of the GAV-coincidence orphan-detection issue. Before invoking the path-adding visitor on the matched plugin, ensure <configuration><annotationProcessorPaths/></configuration> exists. The helper is conservative: it adds nothing when the structure is already present, only fills in whichever level is missing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MavenResolutionResult.parentPomIsProjectPom()is GAV-only).<modules>chains; treat the child as an orphan when no aggregator actually links them.<configuration>/<annotationProcessorPaths>when the matched plugin is pre-declared without it (previouslyAddPluginVisitorleft such plugins untouched).Problem
AddAnnotationProcessorproduced zero changes on Maven modules that:maven-compiler-pluginwithoutannotationProcessorPaths(e.g. only to set<source>/<target>or for inherited config).In case 1,
parentPomIsProjectPom()returns true purely because GAVs match, so the child is routed to the in-reactor-parent branch and never added tocandidateOrphanPaths. In the visitor it is neither parent nor orphan and is skipped, so its ownbuild/pluginsis never updated.In case 2,
AddPluginVisitor's "plugin already exists" branch leaves the plugin untouched, and the path-adding inner visitor only operates on existing<annotationProcessorPaths>tags — so nothing is ever added.Solution
Reactor-aware parent classification. During scan, record
tentativeChildToParentlinks and aggregator<modules>membership (read off the requested Pom, sinceResolvedPom.subprojectsis unreliable in some LST fixtures andmrr.getModules()lists POMs claiming this one as<parent>rather than ones aggregated by it). Before the visitor runs, walk aggregator<modules>chains via BFS; uphold a tentative link only when the child is reactor-reachable. Otherwise reclassify as orphan and add the processor to its ownbuild/plugins. Skip danglingpackaging=pomPOMs that are neither claimed parents nor aggregators.Bypass for GAV-coincident orphans. Pass the source path as
AddPluginVisitor'sfilePatternonly for the GAV-coincident orphan case so its ownparentPomIsProjectPom()short-circuit doesn't refuse to add the plugin. Genuine in-reactor parents and true single-module orphans hit the existing acceptance path withfilePattern=null.Fill in missing
<configuration>structure. Before the path-adding visitor runs, ensure the matched plugin has<configuration><annotationProcessorPaths/></configuration>. The helper is conservative: only fills in whichever level is missing.Test plan
orphanModuleSharedLstWithExternalParentreproducer fails onmainand passes after the fix.addConfigurationToExistingPluginWithNoConfigandaddAnnotationProcessorPathsToExistingConfigurationreproducers fail onmainand pass after the fix.AddAnnotationProcessorTestcases continue to pass.:rewrite-maven:testpasses.