First Maven dependency declaration wins for equal distance (#6002)

* First Maven dependency declaration wins for equal distance

* Apply suggestions from code review

---------

Co-authored-by: Tim te Beek <tim@moderne.io>

Author: bmuschko

rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java

@@ -994,13 +994,13 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
                                                         MavenPomDownloader downloader, ExecutionContext ctx) throws MavenDownloadingExceptions {
         List<ResolvedDependency> dependencies = new ArrayList<>();
 
-        Map<GroupArtifact, DependencyAndDependent> rootDependencies = new HashMap<>();
+        Map<GroupArtifact, DependencyAndDependent> rootDependencies = new LinkedHashMap<>();
         for (Dependency requestedDependency : getRequestedDependencies()) {
             Dependency d = getValues(requestedDependency, 0);
             Scope dScope = Scope.fromName(d.getScope());
             if (dScope == scope || dScope.transitiveOf(scope) == scope) {
-                // TODO can we always use the Map.put approach? Using the latest one is Maven specific, but this resolving is also used for gradle which does use highest version.
-                //  We could introduce a ResolutionStrategy to handle this and use Map.merge where we take later occurring one for LAST_WINS/MAVEN and higher version one for LATEST_WINS/GRADLE
+                // For direct dependencies that are duplicated, last declaration wins
+                // TODO: We could introduce a ResolutionStrategy to handle this differently for Gradle which uses highest version
                 rootDependencies.put(d.getGav().asGroupArtifact(), new DependencyAndDependent(requestedDependency, Scope.Compile, null, requestedDependency, this));
             }
         }
@@ -1009,7 +1009,7 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
         int depth = 0;
         Collection<DependencyAndDependent> dependenciesAtDepth = rootDependencies.values();
         while (!dependenciesAtDepth.isEmpty()) {
-            List<DependencyAndDependent> dependenciesAtNextDepth = new ArrayList<>();
+            Map<GroupArtifact, DependencyAndDependent> dependenciesAtNextDepthMap = new LinkedHashMap<>();
 
             for (DependencyAndDependent dd : dependenciesAtDepth) {
                 // First get the dependency (relative to the pom it was defined in)
@@ -1139,15 +1139,17 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
 
                         Scope d2Scope = getDependencyScope(d2, resolvedPom);
                         if (d2Scope.isInClasspathOf(dd.getScope())) {
-                            dependenciesAtNextDepth.add(new DependencyAndDependent(d2, d2Scope, resolved, dd.getRootDependent(), resolvedPom));
+                            // For transitive dependencies at same depth, first parent declaration wins
+                            GroupArtifact d2Ga = new GroupArtifact(d2.getGroupId() == null ? "" : d2.getGroupId(), d2.getArtifactId());
+                            dependenciesAtNextDepthMap.putIfAbsent(d2Ga, new DependencyAndDependent(d2, d2Scope, resolved, dd.getRootDependent(), resolvedPom));
                         }
                     }
                 } catch (MavenDownloadingException e) {
                     exceptions = MavenDownloadingExceptions.append(exceptions, e.setRoot(dd.getRootDependent().getGav()));
                 }
             }
 
-            dependenciesAtDepth = dependenciesAtNextDepth;
+            dependenciesAtDepth = dependenciesAtNextDepthMap.values();
             depth++;
         }
 

rewrite-maven/src/test/java/org/openrewrite/maven/AddManagedDependencyTest.java

@@ -397,7 +397,7 @@ void doesNotDowngradeVersionIfTransitiveDependencyAppearsOnceInTree() {
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"2.14.0", "2.15.3"})
+    @ValueSource(strings = {"2.14.0", "2.15.3", "2.18.0"})
     void doesNotDowngradeVersionIfTransitiveDependencyAppearsInTreeWithDifferentVersions(String requestedVersion) {
         rewriteRun(
           spec -> spec.recipe(new AddManagedDependency("com.fasterxml.jackson.core", "jackson-databind", requestedVersion, null,

rewrite-maven/src/test/java/org/openrewrite/maven/MavenParserTest.java

@@ -29,7 +29,9 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.openrewrite.*;
 import org.openrewrite.maven.http.OkHttpSender;
@@ -45,6 +47,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.stream.Stream;
 
 import static java.util.stream.Collectors.groupingBy;
 import static org.assertj.core.api.Assertions.*;
@@ -4097,6 +4100,67 @@ void lastListedDependencyIsUsedForTransitiveScope(String firstVersion, String se
         );
     }
 
+    @MethodSource("jacksonDependencies")
+    @ParameterizedTest
+    void firstDeclarationWinsForEqualDistanceOfTransitiveDependencies(String dependencies, String resolvedTransitiveVersion) {
+        rewriteRun(
+          pomXml(
+            //language=xml
+            """
+              <project>
+                <groupId>com.mycompany.app</groupId>
+                <artifactId>my-app</artifactId>
+                <version>1</version>
+                <dependencies>
+                  %s
+                </dependencies>
+              </project>
+              """.formatted(dependencies),
+            spec -> spec.afterRecipe(pom -> {
+                MavenResolutionResult resolution = pom.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow();
+                assertThat(resolution.findDependencies("com.fasterxml.jackson.core", "jackson-databind", Scope.Compile))
+                  .hasSize(1)
+                  .extracting(ResolvedDependency::getGav)
+                  .extracting(ResolvedGroupArtifactVersion::getVersion)
+                  .containsExactly(resolvedTransitiveVersion);
+            })
+          )
+        );
+    }
+
+    private static Stream<Arguments> jacksonDependencies() {
+        return Stream.of(
+          Arguments.of(
+            """
+            <dependency>
+                <groupId>com.fasterxml.jackson.dataformat</groupId>
+                <artifactId>jackson-dataformat-xml</artifactId>
+                <version>2.18.0</version>
+            </dependency>
+            <dependency>
+                <groupId>com.fasterxml.jackson.datatype</groupId>
+                <artifactId>jackson-datatype-jsr310</artifactId>
+                <version>2.15.3</version>
+            </dependency>
+            """,
+            "2.18.0"),
+          Arguments.of(
+            """
+            <dependency>
+                <groupId>com.fasterxml.jackson.datatype</groupId>
+                <artifactId>jackson-datatype-jsr310</artifactId>
+                <version>2.15.3</version>
+            </dependency>
+            <dependency>
+                <groupId>com.fasterxml.jackson.dataformat</groupId>
+                <artifactId>jackson-dataformat-xml</artifactId>
+                <version>2.18.0</version>
+            </dependency>
+            """,
+            "2.15.3")
+        );
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {"latest", "release"})
     void latestOrReleaseVersionInDependencyManagement(String version) {