Allowing AddDependency to broaden the scope of existing dependency, provided you're requesting an equal or higher version number. If requesting same scope but higher version number, the version number will be upgraded.

Author: steve-aom-elliott

rewrite-maven/src/main/java/org/openrewrite/maven/AddDependency.java

@@ -224,7 +224,10 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
                 if (dependencies.get(Scope.Compile) != null) {
                     for (ResolvedDependency d : dependencies.get(Scope.Compile)) {
                         if (hasAcceptableTransitivity(d, acc) &&
-                            groupId.equals(d.getGroupId()) && artifactId.equals(d.getArtifactId())) {
+                            groupId.equals(d.getGroupId()) &&
+                            artifactId.equals(d.getArtifactId()) &&
+                            version.equals(d.getVersion())
+                        ) {
                             return maven;
                         }
                     }

rewrite-maven/src/main/java/org/openrewrite/maven/AddDependencyVisitor.java

@@ -19,12 +19,10 @@
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Validated;
+import org.openrewrite.marker.Markup;
 import org.openrewrite.maven.internal.InsertDependencyComparator;
 import org.openrewrite.maven.table.MavenMetadataFailures;
-import org.openrewrite.maven.tree.MavenMetadata;
-import org.openrewrite.maven.tree.ResolvedDependency;
-import org.openrewrite.maven.tree.Scope;
-import org.openrewrite.maven.tree.Version;
+import org.openrewrite.maven.tree.*;
 import org.openrewrite.semver.ExactVersion;
 import org.openrewrite.semver.LatestRelease;
 import org.openrewrite.semver.Semver;
@@ -39,6 +37,7 @@
 import java.util.regex.Pattern;
 
 import static java.util.Collections.emptyList;
+import static java.util.Objects.requireNonNull;
 
 @RequiredArgsConstructor
 public class AddDependencyVisitor extends MavenIsoVisitor<ExecutionContext> {
@@ -89,38 +88,86 @@ public AddDependencyVisitor(String groupId, String artifactId, String version,
 
     @Override
     public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext executionContext) {
-        if (isDependencyTag() &&
-                groupId.equals(tag.getChildValue("groupId").orElse(null)) &&
-                artifactId.equals(tag.getChildValue("artifactId").orElse(null)) &&
-                Scope.fromName(scope) == Scope.fromName(tag.getChildValue("scope").orElse(null))) {
-            getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "alreadyHasDependency", true);
-            return tag;
+        if (isDependencyTag()) {
+            ResolvedPom resolvedPom = getResolutionResult().getPom();
+            String existingGroupId = resolvedPom.getValue(tag.getChildValue("groupId").orElse(null));
+            String existingArtifactId = resolvedPom.getValue(tag.getChildValue("artifactId").orElse(null));
+            if (groupId.equals(existingGroupId) && artifactId.equals(existingArtifactId)) {
+                Scope requestedScope = Scope.fromName(scope);
+                Scope existingScope = Scope.fromName(resolvedPom.getValue(tag.getChildValue("scope").orElse(null)));
+                if (tag.getMarkers().getMarkers().stream()
+                        .anyMatch(m -> m instanceof Markup.Warn &&
+                                ((Markup.Warn) m).getDetail().startsWith("org.openrewrite.maven.MavenDownloadingException"))
+                ) {
+                    getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "existingDependencyFailure", true);
+                } else if (requestedScope != existingScope &&
+                        // Scope reduction
+                        ((existingScope.isInClasspathOf(requestedScope) &&
+                                Scope.maxPrecedence(existingScope, requestedScope) == existingScope) ||
+                                // System / Import / Invalid which we don't support changing to
+                                (!existingScope.isInClasspathOf(requestedScope) &&
+                                        !requestedScope.isInClasspathOf(existingScope) &&
+                                        Scope.Test != requestedScope &&
+                                        Scope.maxPrecedence(Scope.Test, requestedScope) == Scope.Test))
+                ) {
+                    getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "doNotAlterDependency", true);
+                } else {
+                    String versionToUse = null;
+                    String managedVersion = getResolutionResult().getPom().getManagedVersion(groupId, artifactId, type, classifier);
+                    if (managedVersion == null || (versionComparator != null && !versionComparator.isValid(version, managedVersion))) {
+                        versionToUse = tryGetFamilyVersion();
+                        if (versionToUse == null) {
+                            try {
+                                versionToUse = findVersionToUse(executionContext);
+                            } catch (MavenDownloadingException e) {
+                                return e.warn(tag);
+                            }
+                        }
+                    }
+                    if (versionToUse != null) {
+                        getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "requestedVersionChange", true);
+                        getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "newResolvedVersion", versionToUse);
+                    }
+                    if (requestedScope != existingScope) {
+                        getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "requestedScopeChange", true);
+                        getCursor().putMessageOnFirstEnclosing(Xml.Document.class, "oldScope", existingScope);
+                    }
+                }
+                return tag;
+            }
         }
         return super.visitTag(tag, executionContext);
     }
 
 
     @Override
     public Xml.Document visitDocument(Xml.Document document, ExecutionContext executionContext) {
+        Validated<VersionComparator> versionValidation = Semver.validate(version, versionPattern);
+        if (versionValidation.isValid()) {
+            versionComparator = versionValidation.getValue();
+        }
+
         Xml.Document maven = super.visitDocument(document, executionContext);
 
-        if (getCursor().getMessage("alreadyHasDependency", false)) {
+        if (getCursor().getMessage("doNotAlterDependency", false) ||
+                getCursor().getMessage("existingDependencyFailure", false)
+        ) {
             return document;
         }
 
-        Scope resolvedScope = scope == null ? Scope.Compile : Scope.fromName(scope);
+        boolean requestedVersionChange = getCursor().getMessage("requestedVersionChange", false);
+        Scope resolvedScope = Scope.fromName(scope);
         Map<Scope, List<ResolvedDependency>> dependencies = getResolutionResult().getDependencies();
         if (dependencies.containsKey(resolvedScope)) {
             for (ResolvedDependency d : dependencies.get(resolvedScope)) {
                 if (d.isDirect() && groupId.equals(d.getGroupId()) && artifactId.equals(d.getArtifactId())) {
+                    if (requestedVersionChange) {
+                        checkVersionUpdate(d.getVersion());
+                    }
                     return maven;
                 }
             }
-        }
 
-        Validated<VersionComparator> versionValidation = Semver.validate(version, versionPattern);
-        if (versionValidation.isValid()) {
-            versionComparator = versionValidation.getValue();
         }
 
         Xml.Tag root = maven.getRoot();
@@ -129,7 +176,43 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext execut
                     new MavenTagInsertionComparator(root.getContent() == null ? emptyList() : root.getContent())));
         }
 
-        doAfterVisit(new InsertDependencyInOrder(scope));
+        boolean isUpdating = false;
+        if (getCursor().getMessage("requestedScopeChange", false)) {
+            isUpdating = true;
+            Scope oldScope = getCursor().getMessage("oldScope");
+            if (dependencies.containsKey(oldScope)) {
+                for (ResolvedDependency d : dependencies.get(oldScope)) {
+                    if (d.isDirect() && groupId.equals(d.getGroupId()) && artifactId.equals(d.getArtifactId())) {
+                        if (requestedVersionChange) {
+                            checkVersionUpdate(d.getVersion());
+                        }
+                        doAfterVisit(new ChangeDependencyScope(groupId, artifactId, scope).getVisitor());
+                        maybeUpdateModel();
+                        break;
+                    }
+                }
+            } else { // Going from System / Import / Invalid to something else
+                ResolvedPom resolvedPom = getResolutionResult().getPom();
+                for (Dependency d : resolvedPom.getRequestedDependencies()) {
+                    if (groupId.equals(resolvedPom.getValue(d.getGroupId())) && artifactId.equals(resolvedPom.getValue(d.getArtifactId()))) {
+                        // This is run in this order because `ChangeDependencyScope` can end up moving a dependency from just requested to an actual dependency
+                        // whereas updating the version relies on something being in the dependencies
+                        doAfterVisit(new ChangeDependencyScope(groupId, artifactId, scope).getVisitor());
+                        maybeUpdateModel();
+                        if (requestedVersionChange && d.getVersion() != null) {
+                            checkVersionUpdate(requireNonNull(resolvedPom.getValue(d.getVersion())));
+                            maybeUpdateModel();
+                        }
+                        break;
+                    }
+                }
+
+            }
+        }
+
+        if (!isUpdating) {
+            doAfterVisit(new InsertDependencyInOrder(scope));
+        }
 
         return maven;
     }
@@ -144,21 +227,19 @@ private class InsertDependencyInOrder extends MavenVisitor<ExecutionContext> {
         public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
             if (DEPENDENCIES_MATCHER.matches(getCursor())) {
                 String versionToUse = null;
-
-                if (getResolutionResult().getPom().getManagedVersion(groupId, artifactId, type, classifier) == null) {
-                    if (familyRegex != null) {
-                        versionToUse = findDependencies(d -> familyRegex.matcher(d.getGroupId()).matches()).stream()
-                                .max(Comparator.comparing(d -> new Version(d.getVersion())))
-                                .map(d -> d.getRequested().getVersion())
-                                .orElse(null);
-                    }
+                String managedVersion = getResolutionResult().getPom().getManagedVersion(groupId, artifactId, type, classifier);
+                boolean scheduleVersionUpgrade = false;
+                if (managedVersion == null) {
+                    versionToUse = tryGetFamilyVersion();
                     if (versionToUse == null) {
                         try {
                             versionToUse = findVersionToUse(ctx);
                         } catch (MavenDownloadingException e) {
                             return e.warn(tag);
                         }
                     }
+                } else if (versionComparator != null && !versionComparator.isValid(version, managedVersion)) {
+                    scheduleVersionUpgrade = true;
                 }
 
                 Xml.Tag dependencyTag = Xml.Tag.build(
@@ -179,36 +260,56 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
 
                 doAfterVisit(new AddToTagVisitor<>(tag, dependencyTag, new InsertDependencyComparator(tag.getContent() == null ? emptyList() : tag.getContent(), dependencyTag)));
                 maybeUpdateModel();
+                if (scheduleVersionUpgrade) {
+                    doAfterVisit(new UpgradeDependencyVersion(groupId, artifactId, version, versionPattern, true, null).getVisitor());
+                }
 
                 return tag;
             }
 
             return super.visitTag(tag, ctx);
         }
+    }
 
-        private String findVersionToUse(ExecutionContext ctx) throws MavenDownloadingException {
-            if (resolvedVersion == null) {
-                if (versionComparator == null || versionComparator instanceof ExactVersion) {
-                    resolvedVersion = version;
-                } else {
-                    MavenMetadata mavenMetadata = metadataFailures == null ?
-                            downloadMetadata(groupId, artifactId, ctx) :
-                            metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx));
-                    // TODO This is hacky, but the class structure of LatestRelease is suboptimal, see https://github.com/openrewrite/rewrite/pull/5029
-                    // Fix it when we have a chance to refactor the code.
-                    if ("LatestRelease".equals(versionComparator.getClass().getSimpleName()) && mavenMetadata.getVersioning().getRelease() != null) {
-                        return mavenMetadata.getVersioning().getRelease();
-                    }
-                    LatestRelease latest = new LatestRelease(versionPattern);
-                    resolvedVersion = mavenMetadata.getVersioning().getVersions().stream()
-                            .filter(v -> versionComparator.isValid(null, v))
-                            .filter(v -> !Boolean.TRUE.equals(releasesOnly) || latest.isValid(null, v))
-                            .max((v1, v2) -> versionComparator.compare(null, v1, v2))
-                            .orElse(version);
+    private @Nullable String tryGetFamilyVersion() {
+        if (familyRegex != null) {
+            return findDependencies(d -> familyRegex.matcher(d.getGroupId()).matches()).stream()
+                    .max(Comparator.comparing(d -> new Version(d.getVersion())))
+                    .map(d -> d.getRequested().getVersion())
+                    .orElse(null);
+        }
+        return null;
+    }
+
+    private void checkVersionUpdate( String existingVersion) {
+        String newResolvedVersion = requireNonNull(getCursor().getMessage("newResolvedVersion"));
+        if (!existingVersion.equals(getResolutionResult().getPom().getValue(newResolvedVersion))) {
+            doAfterVisit(new UpgradeDependencyVersion(groupId, artifactId, newResolvedVersion, versionPattern, true, null).getVisitor());
+        }
+    }
+
+    private String findVersionToUse(ExecutionContext ctx) throws MavenDownloadingException {
+        if (resolvedVersion == null) {
+            if (versionComparator == null || versionComparator instanceof ExactVersion) {
+                resolvedVersion = version;
+            } else {
+                MavenMetadata mavenMetadata = metadataFailures == null ?
+                        downloadMetadata(groupId, artifactId, ctx) :
+                        metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx));
+                // TODO This is hacky, but the class structure of LatestRelease is suboptimal, see https://github.com/openrewrite/rewrite/pull/5029
+                // Fix it when we have a chance to refactor the code.
+                if ("LatestRelease".equals(versionComparator.getClass().getSimpleName()) && mavenMetadata.getVersioning().getRelease() != null) {
+                    return mavenMetadata.getVersioning().getRelease();
                 }
+                LatestRelease latest = new LatestRelease(versionPattern);
+                resolvedVersion = mavenMetadata.getVersioning().getVersions().stream()
+                        .filter(v -> versionComparator.isValid(null, v))
+                        .filter(v -> !Boolean.TRUE.equals(releasesOnly) || latest.isValid(null, v))
+                        .max((v1, v2) -> versionComparator.compare(null, v1, v2))
+                        .orElse(version);
             }
-
-            return resolvedVersion;
         }
+
+        return resolvedVersion;
     }
 }