diff --git a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java index 4f068ce090..3411e16569 100644 --- a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java +++ b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java @@ -16,7 +16,6 @@ package org.openrewrite.java.migrate; import lombok.Getter; -import lombok.RequiredArgsConstructor; import org.intellij.lang.annotations.Language; import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; @@ -27,7 +26,6 @@ import org.openrewrite.internal.StringUtils; import org.openrewrite.maven.AddPlugin; import org.openrewrite.maven.AddPropertyVisitor; -import org.openrewrite.maven.ChangePluginExecutions; import org.openrewrite.maven.MavenIsoVisitor; import org.openrewrite.maven.search.DependencyInsight; import org.openrewrite.maven.search.FindPlugin; @@ -35,7 +33,6 @@ import org.openrewrite.maven.tree.ResolvedDependency; import org.openrewrite.maven.tree.Scope; import org.openrewrite.semver.LatestRelease; -import org.openrewrite.xml.AddOrUpdateChildTag; import org.openrewrite.xml.AddToTagVisitor; import org.openrewrite.xml.XPathMatcher; import org.openrewrite.xml.XmlIsoVisitor; @@ -54,14 +51,11 @@ public class AddMockitoJavaAgentToMavenSurefirePlugin extends Recipe { private static final String MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID = "maven-surefire-plugin"; private static final String MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID = "maven-dependency-plugin"; - @Language("xpath") - private static final String MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER = "/project/build/plugins/plugin[artifactId='maven-dependency-plugin']/executions/execution"; + private static final String PROPERTIES_GOAL = "properties"; @Language("xml") - private static final String MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL = "properties"; - - @Language("xml") - private static final String MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG = ""+ MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL + ""; + private static final String MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG = + "get-mockito-agent-path" + PROPERTIES_GOAL + ""; @Getter final String displayName = "Add Mockito Java Agent to Maven Surefire Plugin"; @@ -105,24 +99,6 @@ private boolean usesRuntimeArgLineProperty() { declaresJacocoPlugin(getCursor().firstEnclosingOrThrow(Xml.Document.class)); } - private void maybeAddMavenDependencyPluginWithPropertiesGoal() { - Optional mavenDependencyPlugin = getResolutionResult().getPom().getPlugins().stream() - .filter(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID)).findFirst(); - - if (!mavenDependencyPlugin.isPresent()) { - doAfterVisit(new AddPlugin(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, null, null, null, - "" + MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG + "", "**/pom.xml").getVisitor()); - } else if (mavenDependencyPlugin.get().getExecutions().isEmpty()) { - doAfterVisit(new ChangePluginExecutions(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG).getVisitor()); - } else if (mavenDependencyPlugin.get().getExecutions().stream().noneMatch(execution -> execution.getGoals() != null)) { - doAfterVisit(new AddOrUpdateChildTag(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER, "" + MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL + "", false).getVisitor()); - } else if (!hasPropertiesGoal(mavenDependencyPlugin.get())) { - doAfterVisit(new AppendChildTagToParentVisitor( - new XPathMatcher(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER + "/goals"), - Xml.Tag.build(MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL))); - } - } - private @Nullable String surefireArgLineWithAgent(List plugins) { String agentArgument = getArgLineJavaAgentArgument(); return plugins.stream() @@ -132,11 +108,6 @@ private void maybeAddMavenDependencyPluginWithPropertiesGoal() { .findFirst().orElse(null); } - private boolean mavenDependencyPluginHasPropertiesGoal() { - return getResolutionResult().getPom().getPlugins().stream() - .anyMatch(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID) && hasPropertiesGoal(plugin)); - } - @Override public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { if (surefireArgLineWithAgent(getResolutionResult().getPom().getPluginManagement()) != null) { @@ -145,23 +116,32 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { // Nothing to add when the surefire agent, the properties goal, and any `@{argLine}` property // are already present, whether declared here or inherited from a parent's `build/plugins` (#1164). String pluginArgLine = surefireArgLineWithAgent(getResolutionResult().getPom().getPlugins()); - if (pluginArgLine != null && mavenDependencyPluginHasPropertiesGoal() && + boolean mavenDependencyPluginRunsPropertiesGoalAtDefaultPhase = getResolutionResult().getPom().getPlugins().stream() + .anyMatch(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID) && + plugin.getExecutions().stream() + .anyMatch(execution -> execution.getPhase() == null && + execution.getGoals() != null && execution.getGoals().contains(PROPERTIES_GOAL))); + if (mavenDependencyPluginRunsPropertiesGoalAtDefaultPhase && pluginArgLine != null && (!pluginArgLine.contains("@{argLine}") || getResolutionResult().getPom().getProperties().containsKey("argLine"))) { return document; } - maybeAddMavenDependencyPluginWithPropertiesGoal(); - if (usesRuntimeArgLineProperty()) { - doAfterVisit(new AddPropertyVisitor("argLine", "", true)); + Xml.Document d = document; + if (!mavenDependencyPluginRunsPropertiesGoalAtDefaultPhase) { + // AddPlugin no-ops when the plugin exists; the visitor below appends only to an already declared plugin. + d = (Xml.Document) new AddPlugin(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, null, null, null, + "" + MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG + "", "**/pom.xml") + .getVisitor().visitNonNull(d, ctx); + d = (Xml.Document) new AddPropertiesGoalExecutionVisitor().visitNonNull(d, ctx); } - - if (FindPlugin.find(document, "org.apache.maven.plugins", "maven-surefire-plugin").isEmpty()) { - doAfterVisit(new AddPlugin("org.apache.maven.plugins", "maven-surefire-plugin", null, - String.format(CONFIGURATION_TAG_TEMPLATE, newArgLineValue(getArgLineJavaAgentArgument())), null, - null, "**/pom.xml").getVisitor()); - return document; + if (usesRuntimeArgLineProperty()) { + d = (Xml.Document) new AddPropertyVisitor("argLine", "", true).visitNonNull(d, ctx); } - return super.visitDocument(document, ctx); + return FindPlugin.find(d, MAVEN_PLUGINS_GROUP_ID, MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID).isEmpty() ? + (Xml.Document) new AddPlugin(MAVEN_PLUGINS_GROUP_ID, MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID, null, + String.format(CONFIGURATION_TAG_TEMPLATE, newArgLineValue(getArgLineJavaAgentArgument())), null, + null, "**/pom.xml").getVisitor().visitNonNull(d, ctx) : + super.visitDocument(d, ctx); } @Override @@ -246,23 +226,36 @@ public Xml.Tag visitTag(Xml.Tag tag, AtomicBoolean found) { }.reduce(document, new AtomicBoolean()).get(); } - private static boolean hasPropertiesGoal(Plugin plugin) { - return plugin.getExecutions().stream() - .anyMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties")); - } - - @RequiredArgsConstructor - private static class AppendChildTagToParentVisitor extends XmlIsoVisitor { - private final XPathMatcher parentXPathMatcher; - private final Xml.Tag newChildTag; + // Matching the plugin, not each execution, keeps this to one new execution however many the plugin already has. + private static class AddPropertiesGoalExecutionVisitor extends XmlIsoVisitor { + private static final XPathMatcher PLUGIN_MATCHER = new XPathMatcher("/project/build/plugins/plugin"); @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { - if (parentXPathMatcher.matches(getCursor()) && - tag.getChildren(newChildTag.getName()).stream().noneMatch(child -> child.getValue().equals(newChildTag.getValue()))) { - return autoFormat(AddToTagVisitor.addToTag(tag, newChildTag, getCursor()), ctx); + Xml.Tag t = super.visitTag(tag, ctx); + // A missing groupId is Maven's own; the matcher excludes pluginManagement. + if (!"plugin".equals(t.getName()) || + !MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID.equals(t.getChildValue("artifactId").orElse(null)) || + !MAVEN_PLUGINS_GROUP_ID.equals(t.getChildValue("groupId").orElse(MAVEN_PLUGINS_GROUP_ID)) || + !PLUGIN_MATCHER.matches(getCursor())) { + return t; } - return super.visitTag(tag, ctx); + + Optional executions = t.getChild("executions"); + if (executions.isPresent() && executions.get().getChildren("execution").stream() + .anyMatch(execution -> !execution.getChild("phase").isPresent() && + execution.getChildren("goals").stream() + .flatMap(goals -> goals.getChildren("goal").stream()) + .anyMatch(goal -> PROPERTIES_GOAL.equals(goal.getValue().orElse(null))))) { + return t; + } + + Xml.Tag scope = executions.orElse(t); + Xml.Tag tagToAdd = executions.isPresent() ? + Xml.Tag.build(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG) : + Xml.Tag.build("" + MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG + ""); + return (Xml.Tag) new AddToTagVisitor(scope, tagToAdd) + .visitNonNull(t, ctx, getCursor().getParentOrThrow()); } } } diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 6c2d593022..1dfc38ed48 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -245,6 +245,7 @@ examples: maven-dependency-plugin + get-mockito-agent-path properties diff --git a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java index cde5b1f83b..5bcd4cfc88 100644 --- a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java +++ b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -99,6 +100,7 @@ void addsMockitoAgentArgAndPropertiesGoalToMavenPlugins() { maven-dependency-plugin + get-mockito-agent-path properties @@ -206,6 +208,9 @@ void addsByteBuddyAgentArgWithSurefirePluginAndNoExistingConfigurationWithOlderM + + + get-mockito-agent-path properties @@ -640,6 +645,7 @@ void onlyAddsConfigurationToPomWithMockitoInMultiModuleProject() { maven-dependency-plugin + get-mockito-agent-path properties @@ -720,6 +726,7 @@ void addsMavenSurefireAndDependencyPluginsWhenAbsent() { maven-dependency-plugin + get-mockito-agent-path properties @@ -825,6 +832,9 @@ void addsGoalsTagWithPropertiesGoalToExistingMavenDependencyPluginWhenMissing() + + + get-mockito-agent-path properties @@ -935,6 +945,362 @@ void addsPropertiesGoalToExistingGoalsSectionInMavenDependencyPlugin() { analyize + + + + get-mockito-agent-path + + properties + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -javaagent:${org.mockito:mockito-core:jar} + + + + + + """ + ) + ) + ); + } + + @Issue("https://github.com/moderneinc/customer-requests/issues/2968") + @Test + void addsSeparateExecutionWhenExistingExecutionRunsAfterTests() { + rewriteRun( + mavenProject("test-project", + pomXml( + """ + + 4.0.0 + org.sample + test + ${revision} + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + org.apache.maven.plugins + maven-dependency-plugin + + + copy-dependencies + package + + copy-dependencies + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -javaagent:${org.mockito:mockito-core:jar} + + + + + + """, + """ + + 4.0.0 + org.sample + test + ${revision} + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + org.apache.maven.plugins + maven-dependency-plugin + + + copy-dependencies + package + + copy-dependencies + + + + get-mockito-agent-path + + properties + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -javaagent:${org.mockito:mockito-core:jar} + + + + + + """ + ) + ) + ); + } + + @Test + void addsSeparateExecutionWhenPropertiesGoalIsBoundToAPhase() { + rewriteRun( + mavenProject("test-project", + pomXml( + """ + + 4.0.0 + org.sample + test + ${revision} + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + org.apache.maven.plugins + maven-dependency-plugin + + + copy-dependencies + package + + copy-dependencies + properties + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -javaagent:${org.mockito:mockito-core:jar} + + + + + + """, + """ + + 4.0.0 + org.sample + test + ${revision} + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + org.apache.maven.plugins + maven-dependency-plugin + + + copy-dependencies + package + + copy-dependencies + properties + + + + get-mockito-agent-path + + properties + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -javaagent:${org.mockito:mockito-core:jar} + + + + + + """ + ) + ) + ); + } + + @Test + void addsSingleExecutionWhenMavenDependencyPluginHasSeveralExecutions() { + rewriteRun( + mavenProject("test-project", + pomXml( + """ + + 4.0.0 + org.sample + test + ${revision} + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + maven-dependency-plugin + + + copy-dependencies + package + + copy-dependencies + + + + unpack + prepare-package + + unpack-dependencies + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -javaagent:${org.mockito:mockito-core:jar} + + + + + + """, + """ + + 4.0.0 + org.sample + test + ${revision} + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + maven-dependency-plugin + + + copy-dependencies + package + + copy-dependencies + + + + unpack + prepare-package + + unpack-dependencies + + + + get-mockito-agent-path + properties @@ -1238,6 +1604,7 @@ void updatesIndividualPomsWhenParentPomManagesSurefirePluginWithoutAgentConfigur maven-dependency-plugin + get-mockito-agent-path properties @@ -1328,6 +1695,7 @@ void updatesIndividualPomsWhenParentPomManagesSurefirePluginWithoutAgentConfigur maven-dependency-plugin + get-mockito-agent-path properties @@ -1684,6 +2052,7 @@ void addsSurefireAgentToModuleWhenParentReactorPomManagesPluginsWithoutDeclaring maven-dependency-plugin + get-mockito-agent-path properties @@ -1752,6 +2121,7 @@ void omitsLateArgLineReplacementWhenNothingSetsArgLineAtRuntime() { maven-dependency-plugin + get-mockito-agent-path properties @@ -1836,6 +2206,7 @@ void usesLateArgLineReplacementWhenJacocoPluginPresent() { maven-dependency-plugin + get-mockito-agent-path properties @@ -1934,6 +2305,7 @@ void usesLateArgLineReplacementWhenJacocoPluginOnlyActiveInProfile() { maven-dependency-plugin + get-mockito-agent-path properties @@ -2007,6 +2379,7 @@ void usesLateArgLineReplacementWhenArgLinePropertyAlreadyDeclared() { maven-dependency-plugin + get-mockito-agent-path properties