diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/AddAnnotationProcessor.java b/rewrite-maven/src/main/java/org/openrewrite/maven/AddAnnotationProcessor.java index 742c309123..bfe91964a0 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/AddAnnotationProcessor.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/AddAnnotationProcessor.java @@ -33,8 +33,13 @@ import org.openrewrite.xml.tree.Xml; import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; @@ -78,47 +83,75 @@ public class AddAnnotationProcessor extends ScanningRecipe aggregatorPaths = new HashSet<>(); + Map> aggregatorSubmodulePaths = new HashMap<>(); + /** - * Source paths of POMs that are referenced as parents by at least one child within the reactor. - * These should get pluginManagement updates. + * Child-to-parent links recorded whenever + * {@code MavenResolutionResult.parentPomIsProjectPom()} is true. The + * check is GAV-based, so the link is upheld in {@link #resolve()} only + * when the child is reachable via some aggregator's <modules> + * chain; otherwise the child is treated as an orphan instead. */ + Map tentativeChildToParent = new HashMap<>(); + + Set noReactorParentPaths = new HashSet<>(); + Set packagingPomPaths = new HashSet<>(); + Set alreadyConfiguredInEffectivePomPaths = new HashSet<>(); + Set parentPomPaths = new HashSet<>(); + Set orphanPomPaths = new HashSet<>(); - /** - * Source paths of POMs that have no parent within the reactor. - * After scanning, aggregator-only POMs will be filtered out. - */ - Set candidateOrphanPaths = new HashSet<>(); + void resolve() { + Set reactorLinked = computeReactorLinkedPaths(); + Set orphans = new HashSet<>(noReactorParentPaths); - /** - * Source paths of POMs that have a <modules> section (aggregators). - * Used to identify aggregator-only POMs that should not be modified. - */ - Set aggregatorPaths = new HashSet<>(); + for (Map.Entry e : tentativeChildToParent.entrySet()) { + Path child = e.getKey(); + Path parent = e.getValue(); + if (reactorLinked.contains(child)) { + parentPomPaths.add(parent); + } else { + orphans.add(child); + } + } - /** - * Paths of POMs where the annotation processor is already present in the effective POM - * (via a parent POM outside the reactor), but not in the POM's own XML. - * These POMs should be skipped to avoid redundant configuration. - */ - Set alreadyConfiguredInEffectivePomPaths = new HashSet<>(); + // Aggregator-only POMs are not modified. + for (Path aggregator : aggregatorPaths) { + if (!parentPomPaths.contains(aggregator)) { + orphans.remove(aggregator); + } + } - /** - * Get the actual orphan paths (candidates minus aggregator-only POMs). - * A true orphan has no parent in reactor and is not an aggregator-only POM. - */ - Set getOrphanPomPaths() { - Set result = new HashSet<>(candidateOrphanPaths); - // Remove aggregator-only POMs (aggregators that are not also parents) - for (Path aggregatorPath : aggregatorPaths) { - if (!parentPomPaths.contains(aggregatorPath)) { - result.remove(aggregatorPath); + // Dangling pom-packaging POMs (not claimed as parents, not aggregators) + // have nothing to compile; leave them alone. + for (Path packagingPom : packagingPomPaths) { + if (!parentPomPaths.contains(packagingPom) && !aggregatorPaths.contains(packagingPom)) { + orphans.remove(packagingPom); } } - return result; + + orphanPomPaths.addAll(orphans); + } + + private Set computeReactorLinkedPaths() { + Set reachable = new HashSet<>(); + Deque queue = new ArrayDeque<>(aggregatorPaths); + while (!queue.isEmpty()) { + Path p = queue.poll(); + if (!reachable.add(p)) { + continue; + } + Set subs = aggregatorSubmodulePaths.get(p); + if (subs != null) { + queue.addAll(subs); + } + } + return reachable; } } @@ -148,28 +181,54 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { acc.alreadyConfiguredInEffectivePomPaths.add(sourcePath); } - if (mrr.parentPomIsProjectPom()) { - // This module has a parent within the reactor - // Mark the parent for pluginManagement update - MavenResolutionResult parent = mrr.getParent(); - if (parent != null) { - Path parentPath = parent.getPom().getRequested().getSourcePath(); + if (sourcePath != null) { + if (mrr.parentPomIsProjectPom()) { + // Parent's GAV matches a project pom, but this is a tentative + // signal only — verified at resolution time by checking that + // the child is reactor-linked via an aggregator's + // chain. Two POMs co-ingested in the LST without a real + // aggregator linking them must not be treated as a reactor. + MavenResolutionResult parent = mrr.getParent(); + Path parentPath = parent == null ? null : + parent.getPom().getRequested().getSourcePath(); if (parentPath != null) { - acc.parentPomPaths.add(parentPath); + acc.tentativeChildToParent.put(sourcePath, parentPath); + } else { + acc.noReactorParentPaths.add(sourcePath); } + } else { + // No project-pom parent — true single-module root or + // standalone pom-packaging shell. + acc.noReactorParentPaths.add(sourcePath); } - } else { - // This module has no parent within the reactor - // Mark as candidate orphan (will be filtered later if it's aggregator-only) - if (sourcePath != null) { - acc.candidateOrphanPaths.add(sourcePath); + + if ("pom".equals(resolvedPom.getPackaging())) { + acc.packagingPomPaths.add(sourcePath); } } - // Track aggregator POMs (those with section) - List subprojects = mrr.getPom().getSubprojects(); - if (sourcePath != null && subprojects != null && !subprojects.isEmpty()) { + // Treat this POM as a reactor aggregator only when its raw XML + // declared /. We read that off the + // requested (unresolved) Pom — `mrr.getModules()` is unsuitable + // because it lists POMs that declare *this* one as their + // (which can happen without any + // declaration on this side). + List requestedSubs = mrr.getPom().getRequested().getSubprojects(); + if (sourcePath != null && requestedSubs != null && !requestedSubs.isEmpty()) { acc.aggregatorPaths.add(sourcePath); + // Resolve each string relative to the aggregator's + // directory. baseDir is null when the aggregator is at the + // root (e.g. "pom.xml"); fall back to the empty path so + // root-level reactors still reach their children. + Path baseDir = sourcePath.getParent(); + Set resolvedSubmodules = new HashSet<>(); + for (String sub : requestedSubs) { + Path resolved = baseDir == null ? + Paths.get(sub, "pom.xml").normalize() : + baseDir.resolve(sub).resolve("pom.xml").normalize(); + resolvedSubmodules.add(resolved); + } + acc.aggregatorSubmodulePaths.put(sourcePath, resolvedSubmodules); } return document; @@ -179,6 +238,7 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { @Override public TreeVisitor getVisitor(Scanned acc) { + acc.resolve(); return new TreeVisitor() { @Override public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { @@ -197,21 +257,23 @@ public TreeVisitor getVisitor(Scanned acc) { } boolean isParent = acc.parentPomPaths.contains(sourcePath); - // Skip POMs that are neither parents nor orphans (children with parents in reactor) - if (!isParent && !acc.getOrphanPomPaths().contains(sourcePath)) { + if (!isParent && !acc.orphanPomPaths.contains(sourcePath)) { return tree; } - - // Skip POMs where the annotation processor is already configured via a parent POM - // outside the reactor (present in effective POM but not in own XML) if (acc.alreadyConfiguredInEffectivePomPaths.contains(sourcePath)) { return tree; } - // First, ensure the plugin exists - use the source path as file pattern + // GAV-coincident orphan: AddPluginVisitor.isAcceptable would + // otherwise short-circuit via its parentPomIsProjectPom() + // check and refuse to add the plugin. Targeting the visitor + // at this exact source path bypasses that guard. + boolean isGavCoincidentOrphan = !isParent && mrr.parentPomIsProjectPom(); + String pluginFilePattern = isGavCoincidentOrphan ? sourcePath.toString() : null; tree = new AddPluginVisitor(isParent, MAVEN_COMPILER_PLUGIN_GROUP_ID, MAVEN_COMPILER_PLUGIN_ARTIFACT_ID, null, - "", null, null, null + "", null, null, + pluginFilePattern ).visit(tree, ctx); // Then, configure the annotation processor path @@ -223,6 +285,14 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { MavenResolutionResult currentMrr = getResolutionResult(); AtomicReference> maybePropertyUpdate = new AtomicReference<>(); + // Ensure + // exists on the plugin so the path-adding visitor below has + // something to attach to. AddPluginVisitor only seeds this + // template when it adds a *new* plugin; when the plugin is + // pre-declared (e.g. just to set /), the + // structure must be filled in here. + Xml.Tag pluginTree = ensureAnnotationProcessorPathsTag(plugin.getTree()); + Xml.Tag modifiedPlugin = new XmlIsoVisitor() { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { @@ -283,7 +353,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { groupId, artifactId, version); return tg.withContent(ListUtils.concat(tg.getChildren(), Xml.Tag.build(pathXml))); } - }.visitTag(plugin.getTree(), ctx); + }.visitTag(pluginTree, ctx); if (maybePropertyUpdate.get() != null) { doAfterVisit(maybePropertyUpdate.get()); @@ -302,6 +372,27 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { }; } + /** + * If the matched maven-compiler-plugin lacks {@code } or its + * {@code } lacks {@code }, add the + * missing structure so the path-adding visitor has somewhere to attach. + * No-op when the structure is already present. + */ + private static Xml.Tag ensureAnnotationProcessorPathsTag(Xml.Tag plugin) { + Xml.Tag config = plugin.getChild("configuration").orElse(null); + if (config == null) { + Xml.Tag newConfig = Xml.Tag.build("\n\n"); + return plugin.withContent(ListUtils.concat(plugin.getChildren(), newConfig)); + } + if (config.getChild("annotationProcessorPaths").isPresent()) { + return plugin; + } + Xml.Tag updatedConfig = config.withContent( + ListUtils.concat(config.getChildren(), Xml.Tag.build(""))); + return plugin.withContent(ListUtils.map(plugin.getChildren(), + child -> child == config ? updatedConfig : child)); + } + /** * True when the effective maven-compiler-plugin version is 3.12.0 or newer. * From 3.12.0 onward the plugin resolves annotation processor path versions diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/AddAnnotationProcessorTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/AddAnnotationProcessorTest.java index 931e441363..86e22314b2 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/AddAnnotationProcessorTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/AddAnnotationProcessorTest.java @@ -1012,6 +1012,118 @@ void createBuildSectionWhenMissing() { ); } + @Test + void addConfigurationToExistingPluginWithNoConfig() { + // Single-module: plugin exists in build/plugins with NO + // block. Common when a module declares maven-compiler-plugin only to + // set / elsewhere or for default lifecycle binding, + // leaving annotationProcessorPaths unconfigured. Recipe must fill in + // the entire configuration tree. + rewriteRun( + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + + """, + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + org.projectlombok + lombok-mapstruct-binding + 0.2.0 + + + + + + + + """ + ) + ); + } + + @Test + void addAnnotationProcessorPathsToExistingConfiguration() { + // Single-module: plugin exists with a block but no + // . Recipe must add the inner element + // without disturbing other configuration entries. + rewriteRun( + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + + org.apache.maven.plugins + maven-compiler-plugin + + 17 + 17 + + + + + + """, + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + + org.apache.maven.plugins + maven-compiler-plugin + + 17 + 17 + + + org.projectlombok + lombok-mapstruct-binding + 0.2.0 + + + + + + + + """ + ) + ); + } + @Test void addToBuildPluginsWhenInBothLocations() { // Single-module: plugin exists in both build/plugins AND pluginManagement @@ -2457,5 +2569,141 @@ void multipleParentsWithSharedChildren() { ) ); } + + @DocumentExample + @Test + void orphanModuleSharedLstWithExternalParent() { + // An "orphan" Maven module (its corporate parent lives outside any + // aggregator chain) where the parent POM happens to share the same + // LST set — e.g. ingested separately as another artifact. There is + // NO aggregator POM linking them, so they are not a real + // reactor; the actual build sees only the child. + // parentPomIsProjectPom() is GAV-based and returns true because + // the parent's GAV is among the project POMs, so + // AddAnnotationProcessor routes the child to the in-reactor-parent + // branch and never adds it to candidateOrphanPaths. In the visitor + // it is neither parent nor orphan and is skipped, so its own + // build/plugins is never updated. + rewriteRun( + mavenProject("corp-parent", + pomXml( + // Corporate parent POM resolved externally in real builds. + // No section — this parent is not aggregating anything. + // Has BOM imports (BOM-driven dependency layout) and + // maven-compiler-plugin in pluginManagement with no annotationProcessorPaths. + """ + + 4.0.0 + com.mycompany.platform + corp-parent + 1.0.0 + pom + + + + org.springframework.boot + spring-boot-dependencies + 3.2.0 + pom + import + + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.13.0 + + + + + + """ + ) + ), + mavenProject("service-module", + pomXml( + // Service module: declares the corporate parent by GAV (no relativePath + // — externally resolved), has Lombok at provided scope, no + // declared (relies on Maven default lifecycle / inherited plugin config). + """ + + 4.0.0 + + com.mycompany.platform + corp-parent + 1.0.0 + + service-module + 0.0.1-SNAPSHOT + + + org.projectlombok + lombok + 1.18.44 + provided + + + org.projectlombok + lombok-mapstruct-binding + 0.2.0 + provided + + + + """, + // Expected: the orphan child gets the annotation processor in its own + // build/plugins, exactly as orphanModuleInSeparatedStructure() asserts + // for the genuine-orphan case. + """ + + 4.0.0 + + com.mycompany.platform + corp-parent + 1.0.0 + + service-module + 0.0.1-SNAPSHOT + + + org.projectlombok + lombok + 1.18.44 + provided + + + org.projectlombok + lombok-mapstruct-binding + 0.2.0 + provided + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + org.projectlombok + lombok-mapstruct-binding + 0.2.0 + + + + + + + + """ + ) + ) + ); + } } }