Skip to content

Fix AddAnnotationProcessor when parent matches by GAV alone #7527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MBoegers wants to merge 4 commits into
base: main
Choose a base branch
from
+392 −53
Open

Fix AddAnnotationProcessor when parent matches by GAV alone #7527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89e4fe8
Add failing reproducer: orphan module skipped when external parent sh…
MBoegers Apr 29, 2026
4c4ac61
Fix AddAnnotationProcessor incorrectly classifying GAV-coincident par…
MBoegers Apr 29, 2026
96333f8
Add failing reproducers for existing maven-compiler-plugin without an…
MBoegers Apr 29, 2026
82eb3fd
Fill in missing <configuration>/<annotationProcessorPaths> on existin…
MBoegers Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 144 additions & 53 deletions rewrite-maven/src/main/java/org/openrewrite/maven/AddAnnotationProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -78,47 +83,75 @@ public class AddAnnotationProcessor extends ScanningRecipe<AddAnnotationProcesso
"Updates the annotation processor version if a newer version is specified.";

/**
* Accumulator to track which POMs need modifications and how.
* Accumulator populated during the scan phase and resolved into final
* parent/orphan path sets via {@link #resolve()} before the visitor runs.
*/
public static class Scanned {
Set<Path> aggregatorPaths = new HashSet<>();
Map<Path, Set<Path>> 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 &lt;modules&gt;
* chain; otherwise the child is treated as an orphan instead.
*/
Map<Path, Path> tentativeChildToParent = new HashMap<>();

Set<Path> noReactorParentPaths = new HashSet<>();
Set<Path> packagingPomPaths = new HashSet<>();
Set<Path> alreadyConfiguredInEffectivePomPaths = new HashSet<>();

Set<Path> parentPomPaths = new HashSet<>();
Set<Path> orphanPomPaths = new HashSet<>();

/**
* Source paths of POMs that have no parent within the reactor.
* After scanning, aggregator-only POMs will be filtered out.
*/
Set<Path> candidateOrphanPaths = new HashSet<>();
void resolve() {
Set<Path> reactorLinked = computeReactorLinkedPaths();
Set<Path> orphans = new HashSet<>(noReactorParentPaths);

/**
* Source paths of POMs that have a &lt;modules&gt; section (aggregators).
* Used to identify aggregator-only POMs that should not be modified.
*/
Set<Path> aggregatorPaths = new HashSet<>();
for (Map.Entry<Path, Path> 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<Path> 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<Path> getOrphanPomPaths() {
Set<Path> 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<Path> computeReactorLinkedPaths() {
Set<Path> reachable = new HashSet<>();
Deque<Path> queue = new ArrayDeque<>(aggregatorPaths);
while (!queue.isEmpty()) {
Path p = queue.poll();
if (!reachable.add(p)) {
continue;
}
Set<Path> subs = aggregatorSubmodulePaths.get(p);
if (subs != null) {
queue.addAll(subs);
}
}
return reachable;
}
}

Expand Down Expand Up @@ -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 <modules>
// 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 <modules> section)
List<String> subprojects = mrr.getPom().getSubprojects();
if (sourcePath != null && subprojects != null && !subprojects.isEmpty()) {
// Treat this POM as a reactor aggregator only when its raw XML
// declared <modules>/<subprojects>. We read that off the
// requested (unresolved) Pom — `mrr.getModules()` is unsuitable
// because it lists POMs that declare *this* one as their
// <parent> (which can happen without any <modules>
// declaration on this side).
List<String> requestedSubs = mrr.getPom().getRequested().getSubprojects();
if (sourcePath != null && requestedSubs != null && !requestedSubs.isEmpty()) {
acc.aggregatorPaths.add(sourcePath);
// Resolve each <module> 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<Path> 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;
Expand All @@ -179,6 +238,7 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {

@Override
public TreeVisitor<?, ExecutionContext> getVisitor(Scanned acc) {
acc.resolve();
return new TreeVisitor<Tree, ExecutionContext>() {
@Override
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
Expand All @@ -197,21 +257,23 @@ public TreeVisitor<?, ExecutionContext> 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,
"<configuration><annotationProcessorPaths/></configuration>", null, null, null
"<configuration><annotationProcessorPaths/></configuration>", null, null,
pluginFilePattern
).visit(tree, ctx);

// Then, configure the annotation processor path
Expand All @@ -223,6 +285,14 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
MavenResolutionResult currentMrr = getResolutionResult();
AtomicReference<TreeVisitor<?, ExecutionContext>> maybePropertyUpdate = new AtomicReference<>();

// Ensure <configuration><annotationProcessorPaths/></configuration>
// 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 <source>/<target>), the
// structure must be filled in here.
Xml.Tag pluginTree = ensureAnnotationProcessorPathsTag(plugin.getTree());

Xml.Tag modifiedPlugin = new XmlIsoVisitor<ExecutionContext>() {
@Override
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
Expand Down Expand Up @@ -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());
Expand All @@ -302,6 +372,27 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
};
}

/**
* If the matched maven-compiler-plugin lacks {@code <configuration>} or its
* {@code <configuration>} lacks {@code <annotationProcessorPaths>}, 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("<configuration>\n<annotationProcessorPaths/>\n</configuration>");
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("<annotationProcessorPaths/>")));
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
Expand Down
Loading
⚡