From b7c2467c7a71d78257b0c1d63af44b5ddfcfd20b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 26 Jun 2026 18:03:54 +0200 Subject: [PATCH 1/4] Avoid duplicate dependencies in ChangeDependencyGroupIdAndArtifactId When the new coordinates were already declared directly and the existing declaration was at a lower version than the requested one, the recipe renamed the old dependency in place, producing two declarations of the same dependency. Now the old declaration is removed whenever an existing direct dependency already matches the new coordinates on groupId, artifactId, classifier, type, and scope, and the surviving declaration is upgraded in place to the requested version. Declarations that differ on classifier, type, or scope are left untouched as distinct dependencies. --- .../ChangeDependencyGroupIdAndArtifactId.java | 61 +++++++--- ...ngeDependencyGroupIdAndArtifactIdTest.java | 107 +++++++++++++++++- 2 files changed, 154 insertions(+), 14 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index 0d036878a7e..d73bb36abb8 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -259,14 +259,19 @@ public TreeVisitor getVisitor(Accumulator acc) { final VersionComparator versionComparator = newVersion != null ? Semver.validate(newVersion, versionPattern).getValue() : null; @Nullable private Collection availableVersions; - private boolean isNewDependencyPresent; private Set safeVersionPlaceholdersToChange = new HashSet<>(); + private final boolean dedupeEnabled = newGroupId != null && newArtifactId != null; + private List existingNewDirectDependencies = new ArrayList<>(); + private List existingOldDirectDependencies = new ArrayList<>(); private final boolean configuredToOverrideManagedVersion = overrideManagedVersion != null && overrideManagedVersion; // False by default private final boolean configuredToChangeManagedDependency = changeManagedDependency == null || changeManagedDependency; // True by default @Override public Xml visitDocument(Xml.Document document, ExecutionContext ctx) { - isNewDependencyPresent = checkIfNewDependencyPresent(newGroupId, newArtifactId, newVersion); + if (dedupeEnabled) { + existingNewDirectDependencies = directDependencies(newGroupId, newArtifactId); + existingOldDirectDependencies = directDependencies(oldGroupId, oldArtifactId); + } safeVersionPlaceholdersToChange = getSafeVersionPlaceholdersToChange(oldGroupId, oldArtifactId, ctx); if (configuredToChangeManagedDependency) { doAfterVisit(new ChangeManagedDependencyGroupIdAndArtifactId( @@ -333,15 +338,22 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { } boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId); - if (isOldDependencyTag && isNewDependencyPresent) { + boolean isNewDependencyTag = dedupeEnabled && isDependencyTag(newGroupId, newArtifactId); + // The new coordinates are already declared directly with the same classifier, type, and scope; + // drop the old declaration rather than renaming it into a duplicate of the existing one. + if (isOldDependencyTag && !isNewDependencyTag && matchesAnyDirectDependency(t, existingNewDirectDependencies)) { doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); return t; } + // When the old declaration is being dropped as a duplicate, upgrade the surviving new + // declaration in place to the requested version instead of leaving it untouched. + boolean isSurvivingNewDependencyTag = isNewDependencyTag && newVersion != null && + matchesAnyDirectDependency(t, existingOldDirectDependencies); boolean isPluginDependency = isPluginDependencyTag(oldGroupId, oldArtifactId); boolean isAnnotationProcessorPath = isAnnotationProcessorPathTag(oldGroupId, oldArtifactId); boolean deferUpdate = false; - if (isOldDependencyTag || isPluginDependency || isAnnotationProcessorPath) { + if (isOldDependencyTag || isPluginDependency || isAnnotationProcessorPath || isSurvivingNewDependencyTag) { if (newVersion != null) { String currentVersionValue = t.getChildValue("version").orElse(null); if (isImplicitlyDefinedVersionProperty(currentVersionValue)) { @@ -373,7 +385,7 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { boolean versionTagPresent = versionTag.isPresent(); // dependencyManagement does not apply to plugin dependencies or annotation processor paths boolean oldDependencyDefinedManaged = isOldDependencyTag && canAffectManagedDependency(getResolutionResult(), scope, oldGroupId, oldArtifactId); - boolean newDependencyManaged = isOldDependencyTag && isDependencyManaged(scope, groupId, artifactId); + boolean newDependencyManaged = (isOldDependencyTag || isSurvivingNewDependencyTag) && isDependencyManaged(scope, groupId, artifactId); if (versionTagPresent) { // If the previous dependency had a version but the new artifact is managed, removed the version tag. if (!configuredToOverrideManagedVersion && newDependencyManaged || (oldDependencyDefinedManaged && configuredToChangeManagedDependency)) { @@ -412,15 +424,38 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { return t; } - private boolean checkIfNewDependencyPresent(@Nullable String groupId, @Nullable String artifactId, @Nullable String version) { - if ((groupId == null) || (artifactId == null)) { - return false; + private List directDependencies(String groupId, String artifactId) { + List direct = new ArrayList<>(); + for (ResolvedDependency rd : findDependencies(groupId, artifactId)) { + if (rd.isDirect()) { + direct.add(rd); + } + } + return direct; + } + + // Matches a declared dependency tag against resolved dependencies on the full coordinates that + // determine identity: groupId/artifactId (already filtered) plus classifier, type, and scope. + private boolean matchesAnyDirectDependency(Xml.Tag tag, List dependencies) { + String classifier = emptyToNull(tag.getChildValue("classifier").orElse(null)); + String type = defaultType(tag.getChildValue("type").orElse(null)); + Scope scope = Scope.fromName(tag.getChildValue("scope").orElse("compile")); + for (ResolvedDependency rd : dependencies) { + if (Objects.equals(classifier, emptyToNull(rd.getClassifier())) && + type.equals(defaultType(rd.getType())) && + scope == Scope.fromName(rd.getRequested().getScope() == null ? "compile" : rd.getRequested().getScope())) { + return true; + } } - List dependencies = findDependencies(groupId, artifactId); - return dependencies.stream() - .filter(ResolvedDependency::isDirect) - .anyMatch(rd -> version == null || - versionComparator != null && versionComparator.compare(null, version, rd.getVersion()) <= 0); + return false; + } + + private @Nullable String emptyToNull(@Nullable String value) { + return isBlank(value) ? null : value; + } + + private String defaultType(@Nullable String type) { + return isBlank(type) ? "jar" : type; } private boolean isDependencyManaged(Scope scope, String groupId, String artifactId) { diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java index f6860e59396..c58ae407142 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java @@ -278,7 +278,7 @@ void shouldNotAddNewIfDependencyAlreadyExists2() { @Issue("https://github.com/openrewrite/rewrite/issues/4514") @Test - void shouldAddNewIfDependencyAlreadyExistsInOlderVersion() { + void shouldUpgradeExistingInPlaceIfDependencyAlreadyExistsInOlderVersion() { rewriteRun( spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( "javax.activation", @@ -309,6 +309,110 @@ void shouldAddNewIfDependencyAlreadyExistsInOlderVersion() { """, + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + jakarta.activation + jakarta.activation-api + 1.2.2 + + + + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/pull/1153") + @Test + void shouldNotLeaveDuplicateWhenExistingNewDependencyIsAtLowerVersion() { + rewriteRun( + spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( + "jakarta.jws", + "jakarta.jws-api", + "jakarta.xml.ws", + "jakarta.xml.ws-api", + "4.0.0", + null + )), + pomXml( + """ + + 4.0.0 + com.example + demo + 0.0.1-SNAPSHOT + + + jakarta.jws + jakarta.jws-api + 3.0.0 + + + jakarta.xml.ws + jakarta.xml.ws-api + 3.0.1 + + + + """, + """ + + 4.0.0 + com.example + demo + 0.0.1-SNAPSHOT + + + jakarta.xml.ws + jakarta.xml.ws-api + 4.0.0 + + + + """ + ) + ); + } + + @Test + void shouldNotDeduplicateWhenClassifierDiffers() { + rewriteRun( + spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( + "javax.activation", + "javax.activation-api", + "jakarta.activation", + "jakarta.activation-api", + "1.2.2", + null + )), + pomXml( + """ + + 4.0.0 + com.mycompany.app + my-app + 1 + + + javax.activation + javax.activation-api + 1.2.0 + + + jakarta.activation + jakarta.activation-api + 1.2.1 + sources + + + + """, """ 4.0.0 @@ -325,6 +429,7 @@ void shouldAddNewIfDependencyAlreadyExistsInOlderVersion() { jakarta.activation jakarta.activation-api 1.2.1 + sources From 004c78521e6f58eb85724127c2ba26382fb88f9d Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 9 Jul 2026 19:28:41 +0200 Subject: [PATCH 2/4] Detect duplicates on single-coordinate changes and clarify dedupe matching Address review feedback: - Dedupe now applies when only newGroupId or only newArtifactId changes, not just when both are provided. - Match duplicates on full renamed coordinates (groupId/artifactId plus classifier/type/scope) rather than assuming exact pre-filtering, and rename the helpers to make the intent explicit. --- .../ChangeDependencyGroupIdAndArtifactId.java | 72 ++++++++++++++----- ...ngeDependencyGroupIdAndArtifactIdTest.java | 51 +++++++++++++ 2 files changed, 105 insertions(+), 18 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index d73bb36abb8..9892264bf21 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -260,7 +260,12 @@ public TreeVisitor getVisitor(Accumulator acc) { @Nullable private Collection availableVersions; private Set safeVersionPlaceholdersToChange = new HashSet<>(); - private final boolean dedupeEnabled = newGroupId != null && newArtifactId != null; + // Coordinates the matched dependencies are renamed to; a null option keeps the existing value. + private final String effectiveNewGroupId = newGroupId != null ? newGroupId : oldGroupId; + private final String effectiveNewArtifactId = newArtifactId != null ? newArtifactId : oldArtifactId; + // Dedupe applies whenever the coordinates actually change (a new group id and/or artifact id). + private final boolean dedupeEnabled = (newGroupId != null || newArtifactId != null) && + !(effectiveNewGroupId.equals(oldGroupId) && effectiveNewArtifactId.equals(oldArtifactId)); private List existingNewDirectDependencies = new ArrayList<>(); private List existingOldDirectDependencies = new ArrayList<>(); private final boolean configuredToOverrideManagedVersion = overrideManagedVersion != null && overrideManagedVersion; // False by default @@ -269,7 +274,7 @@ public TreeVisitor getVisitor(Accumulator acc) { @Override public Xml visitDocument(Xml.Document document, ExecutionContext ctx) { if (dedupeEnabled) { - existingNewDirectDependencies = directDependencies(newGroupId, newArtifactId); + existingNewDirectDependencies = directDependencies(effectiveNewGroupId, effectiveNewArtifactId); existingOldDirectDependencies = directDependencies(oldGroupId, oldArtifactId); } safeVersionPlaceholdersToChange = getSafeVersionPlaceholdersToChange(oldGroupId, oldArtifactId, ctx); @@ -338,18 +343,20 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { } boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId); - boolean isNewDependencyTag = dedupeEnabled && isDependencyTag(newGroupId, newArtifactId); - // The new coordinates are already declared directly with the same classifier, type, and scope; - // drop the old declaration rather than renaming it into a duplicate of the existing one. - if (isOldDependencyTag && !isNewDependencyTag && matchesAnyDirectDependency(t, existingNewDirectDependencies)) { + boolean isNewDependencyTag = dedupeEnabled && isDependencyTag(effectiveNewGroupId, effectiveNewArtifactId); + // Renaming this old declaration would land on coordinates (including classifier, type, and scope) + // that are already declared directly; drop it rather than creating a duplicate. The + // !isNewDependencyTag guard skips tags whose old and new coordinates overlap (e.g. via globs), + // so we never remove a declaration that is itself the intended new dependency. + if (isOldDependencyTag && !isNewDependencyTag && renamedOldTagWouldDuplicate(t)) { doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); return t; } - // When the old declaration is being dropped as a duplicate, upgrade the surviving new + // When an old declaration is being dropped as a duplicate, upgrade the surviving new // declaration in place to the requested version instead of leaving it untouched. boolean isSurvivingNewDependencyTag = isNewDependencyTag && newVersion != null && - matchesAnyDirectDependency(t, existingOldDirectDependencies); + isSurvivorOfRemovedOldDependency(t); boolean isPluginDependency = isPluginDependencyTag(oldGroupId, oldArtifactId); boolean isAnnotationProcessorPath = isAnnotationProcessorPathTag(oldGroupId, oldArtifactId); boolean deferUpdate = false; @@ -434,22 +441,51 @@ private List directDependencies(String groupId, String artif return direct; } - // Matches a declared dependency tag against resolved dependencies on the full coordinates that - // determine identity: groupId/artifactId (already filtered) plus classifier, type, and scope. - private boolean matchesAnyDirectDependency(Xml.Tag tag, List dependencies) { - String classifier = emptyToNull(tag.getChildValue("classifier").orElse(null)); - String type = defaultType(tag.getChildValue("type").orElse(null)); - Scope scope = Scope.fromName(tag.getChildValue("scope").orElse("compile")); - for (ResolvedDependency rd : dependencies) { - if (Objects.equals(classifier, emptyToNull(rd.getClassifier())) && - type.equals(defaultType(rd.getType())) && - scope == Scope.fromName(rd.getRequested().getScope() == null ? "compile" : rd.getRequested().getScope())) { + // The old dependency declaration, once renamed to the new coordinates, would duplicate a + // dependency that is already declared directly under those same coordinates (matching on + // groupId, artifactId, classifier, type, and scope). + private boolean renamedOldTagWouldDuplicate(Xml.Tag oldTag) { + String renamedGroupId = newGroupId != null ? newGroupId : oldTag.getChildValue("groupId").orElse(null); + String renamedArtifactId = newArtifactId != null ? newArtifactId : oldTag.getChildValue("artifactId").orElse(null); + for (ResolvedDependency rd : existingNewDirectDependencies) { + if (Objects.equals(renamedGroupId, rd.getGroupId()) && + Objects.equals(renamedArtifactId, rd.getArtifactId()) && + sameClassifierTypeScope(oldTag, rd)) { + return true; + } + } + return false; + } + + // This declaration already carries the new coordinates and lines up with an old dependency that + // is being removed as a duplicate (its renamed coordinates, classifier, type, and scope match), + // making it the surviving declaration that should take the requested new version. + private boolean isSurvivorOfRemovedOldDependency(Xml.Tag newTag) { + String newTagGroupId = newTag.getChildValue("groupId").orElse(null); + String newTagArtifactId = newTag.getChildValue("artifactId").orElse(null); + for (ResolvedDependency rd : existingOldDirectDependencies) { + String renamedGroupId = newGroupId != null ? newGroupId : rd.getGroupId(); + String renamedArtifactId = newArtifactId != null ? newArtifactId : rd.getArtifactId(); + if (Objects.equals(newTagGroupId, renamedGroupId) && + Objects.equals(newTagArtifactId, renamedArtifactId) && + sameClassifierTypeScope(newTag, rd)) { return true; } } return false; } + // Compares the classifier, type, and scope of a declared dependency tag against a resolved + // dependency, defaulting absent classifier/type/scope to their Maven defaults. + private boolean sameClassifierTypeScope(Xml.Tag tag, ResolvedDependency rd) { + String classifier = emptyToNull(tag.getChildValue("classifier").orElse(null)); + String type = defaultType(tag.getChildValue("type").orElse(null)); + Scope scope = Scope.fromName(tag.getChildValue("scope").orElse("compile")); + return Objects.equals(classifier, emptyToNull(rd.getClassifier())) && + type.equals(defaultType(rd.getType())) && + scope == Scope.fromName(rd.getRequested().getScope() == null ? "compile" : rd.getRequested().getScope()); + } + private @Nullable String emptyToNull(@Nullable String value) { return isBlank(value) ? null : value; } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java index c58ae407142..d42adffbb7b 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java @@ -438,6 +438,57 @@ void shouldNotDeduplicateWhenClassifierDiffers() { ); } + @Test + void shouldDeduplicateWhenOnlyArtifactIdChanges() { + rewriteRun( + spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( + "org.junit.jupiter", + "junit-jupiter-api", + null, + "junit-jupiter-engine", + "5.9.0", + null + )), + pomXml( + """ + + 4.0.0 + com.example + demo + 0.0.1-SNAPSHOT + + + org.junit.jupiter + junit-jupiter-api + 5.8.2 + + + org.junit.jupiter + junit-jupiter-engine + 5.8.2 + + + + """, + """ + + 4.0.0 + com.example + demo + 0.0.1-SNAPSHOT + + + org.junit.jupiter + junit-jupiter-engine + 5.9.0 + + + + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/4514") @Test void shouldNotAddNewIfDependencyAlreadyExistsWithVersion() { From 470ef1a3677f1ad6e9567c5a580ed60548e321df Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 10 Jul 2026 18:10:01 +0200 Subject: [PATCH 3/4] Simplify dedupe logic and drop redundant defaulting - Remove explanatory comments now covered by method/variable names - Collapse the dead first conjunct in dedupeEnabled - Delete shadowing effectiveNew* locals in the exclusion pass - Centralize the rename rule in renamedGroupId/renamedArtifactId helpers - Rely on ResolvedDependency.getType() and the null-safe Scope.fromName instead of hand-rolled defaulting --- .../ChangeDependencyGroupIdAndArtifactId.java | 56 ++++++------------- 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index 9892264bf21..8cc4b8bab0f 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -260,12 +260,9 @@ public TreeVisitor getVisitor(Accumulator acc) { @Nullable private Collection availableVersions; private Set safeVersionPlaceholdersToChange = new HashSet<>(); - // Coordinates the matched dependencies are renamed to; a null option keeps the existing value. private final String effectiveNewGroupId = newGroupId != null ? newGroupId : oldGroupId; private final String effectiveNewArtifactId = newArtifactId != null ? newArtifactId : oldArtifactId; - // Dedupe applies whenever the coordinates actually change (a new group id and/or artifact id). - private final boolean dedupeEnabled = (newGroupId != null || newArtifactId != null) && - !(effectiveNewGroupId.equals(oldGroupId) && effectiveNewArtifactId.equals(oldArtifactId)); + private final boolean dedupeEnabled = !effectiveNewGroupId.equals(oldGroupId) || !effectiveNewArtifactId.equals(oldArtifactId); private List existingNewDirectDependencies = new ArrayList<>(); private List existingOldDirectDependencies = new ArrayList<>(); private final boolean configuredToOverrideManagedVersion = overrideManagedVersion != null && overrideManagedVersion; // False by default @@ -287,8 +284,6 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) { } // Add sibling exclusions for the new coordinates alongside existing old exclusions if (newGroupId != null || newArtifactId != null) { - String effectiveNewGroupId = newGroupId != null ? newGroupId : oldGroupId; - String effectiveNewArtifactId = newArtifactId != null ? newArtifactId : oldArtifactId; doAfterVisit(new MavenVisitor() { @Override public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { @@ -344,17 +339,11 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId); boolean isNewDependencyTag = dedupeEnabled && isDependencyTag(effectiveNewGroupId, effectiveNewArtifactId); - // Renaming this old declaration would land on coordinates (including classifier, type, and scope) - // that are already declared directly; drop it rather than creating a duplicate. The - // !isNewDependencyTag guard skips tags whose old and new coordinates overlap (e.g. via globs), - // so we never remove a declaration that is itself the intended new dependency. if (isOldDependencyTag && !isNewDependencyTag && renamedOldTagWouldDuplicate(t)) { doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); return t; } - // When an old declaration is being dropped as a duplicate, upgrade the surviving new - // declaration in place to the requested version instead of leaving it untouched. boolean isSurvivingNewDependencyTag = isNewDependencyTag && newVersion != null && isSurvivorOfRemovedOldDependency(t); boolean isPluginDependency = isPluginDependencyTag(oldGroupId, oldArtifactId); @@ -441,15 +430,12 @@ private List directDependencies(String groupId, String artif return direct; } - // The old dependency declaration, once renamed to the new coordinates, would duplicate a - // dependency that is already declared directly under those same coordinates (matching on - // groupId, artifactId, classifier, type, and scope). private boolean renamedOldTagWouldDuplicate(Xml.Tag oldTag) { - String renamedGroupId = newGroupId != null ? newGroupId : oldTag.getChildValue("groupId").orElse(null); - String renamedArtifactId = newArtifactId != null ? newArtifactId : oldTag.getChildValue("artifactId").orElse(null); + String groupId = renamedGroupId(oldTag.getChildValue("groupId").orElse(null)); + String artifactId = renamedArtifactId(oldTag.getChildValue("artifactId").orElse(null)); for (ResolvedDependency rd : existingNewDirectDependencies) { - if (Objects.equals(renamedGroupId, rd.getGroupId()) && - Objects.equals(renamedArtifactId, rd.getArtifactId()) && + if (Objects.equals(groupId, rd.getGroupId()) && + Objects.equals(artifactId, rd.getArtifactId()) && sameClassifierTypeScope(oldTag, rd)) { return true; } @@ -457,17 +443,12 @@ private boolean renamedOldTagWouldDuplicate(Xml.Tag oldTag) { return false; } - // This declaration already carries the new coordinates and lines up with an old dependency that - // is being removed as a duplicate (its renamed coordinates, classifier, type, and scope match), - // making it the surviving declaration that should take the requested new version. private boolean isSurvivorOfRemovedOldDependency(Xml.Tag newTag) { String newTagGroupId = newTag.getChildValue("groupId").orElse(null); String newTagArtifactId = newTag.getChildValue("artifactId").orElse(null); for (ResolvedDependency rd : existingOldDirectDependencies) { - String renamedGroupId = newGroupId != null ? newGroupId : rd.getGroupId(); - String renamedArtifactId = newArtifactId != null ? newArtifactId : rd.getArtifactId(); - if (Objects.equals(newTagGroupId, renamedGroupId) && - Objects.equals(newTagArtifactId, renamedArtifactId) && + if (Objects.equals(newTagGroupId, renamedGroupId(rd.getGroupId())) && + Objects.equals(newTagArtifactId, renamedArtifactId(rd.getArtifactId())) && sameClassifierTypeScope(newTag, rd)) { return true; } @@ -475,23 +456,22 @@ private boolean isSurvivorOfRemovedOldDependency(Xml.Tag newTag) { return false; } - // Compares the classifier, type, and scope of a declared dependency tag against a resolved - // dependency, defaulting absent classifier/type/scope to their Maven defaults. private boolean sameClassifierTypeScope(Xml.Tag tag, ResolvedDependency rd) { - String classifier = emptyToNull(tag.getChildValue("classifier").orElse(null)); - String type = defaultType(tag.getChildValue("type").orElse(null)); - Scope scope = Scope.fromName(tag.getChildValue("scope").orElse("compile")); - return Objects.equals(classifier, emptyToNull(rd.getClassifier())) && - type.equals(defaultType(rd.getType())) && - scope == Scope.fromName(rd.getRequested().getScope() == null ? "compile" : rd.getRequested().getScope()); + return Objects.equals(emptyToNull(tag.getChildValue("classifier").orElse(null)), emptyToNull(rd.getClassifier())) && + tag.getChildValue("type").orElse("jar").equals(rd.getType()) && + Scope.fromName(tag.getChildValue("scope").orElse(null)) == Scope.fromName(rd.getRequested().getScope()); } - private @Nullable String emptyToNull(@Nullable String value) { - return isBlank(value) ? null : value; + private @Nullable String renamedGroupId(@Nullable String fallback) { + return newGroupId != null ? newGroupId : fallback; + } + + private @Nullable String renamedArtifactId(@Nullable String fallback) { + return newArtifactId != null ? newArtifactId : fallback; } - private String defaultType(@Nullable String type) { - return isBlank(type) ? "jar" : type; + private @Nullable String emptyToNull(@Nullable String value) { + return isBlank(value) ? null : value; } private boolean isDependencyManaged(Scope scope, String groupId, String artifactId) { From bd68e8f9b4a1fd3dd116537fcce8ec3e69ad25ef Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 10 Jul 2026 19:40:34 +0200 Subject: [PATCH 4/4] Use rename helpers for effective new coordinate fields --- .../maven/ChangeDependencyGroupIdAndArtifactId.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index 9542cdf55a3..2e9348dd463 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -260,8 +260,8 @@ public TreeVisitor getVisitor(Accumulator acc) { @Nullable private Collection availableVersions; private Set safeVersionPlaceholdersToChange = new HashSet<>(); - private final String effectiveNewGroupId = newGroupId != null ? newGroupId : oldGroupId; - private final String effectiveNewArtifactId = newArtifactId != null ? newArtifactId : oldArtifactId; + private final String effectiveNewGroupId = renamedGroupId(oldGroupId); + private final String effectiveNewArtifactId = renamedArtifactId(oldArtifactId); private final boolean dedupeEnabled = !effectiveNewGroupId.equals(oldGroupId) || !effectiveNewArtifactId.equals(oldArtifactId); private List existingNewDirectDependencies = new ArrayList<>(); private List existingOldDirectDependencies = new ArrayList<>();