RemoveDuplicateDependencies: preserve Maven's effective dependency model

[martinfrancois]

Suggested review order: 21 of 52 (Score: 6)
Review first: openrewrite/rewrite-testing-frameworks#1095

What's changed?

RemoveDuplicateDependencies now keeps the declaration Maven resolves to, rather than always the first one.

In <dependencies> the last declaration wins as a whole - with its <version>, its <optional> flag and its <exclusions>. ResolvedPom.doResolveDependencies says so in a comment (// For direct dependencies that are duplicated, last declaration wins), matching Maven's own DefaultModelNormalizer#mergeDuplicates. For duplicate declarations, losing the later declaration's <exclusions> is the damaging case, since those are the exclusions Maven was honouring, so the excluded transitive dependency comes back into the build.

Content and position are now decided separately: the surviving tag holds the later declaration's children, at the earlier declaration's list position and leading whitespace. That is the position the resolved model already gives the dependency, since rootDependencies is a LinkedHashMap where a second put replaces the value but keeps the key's place. The two are compared by the values Maven reads, with properties resolved, so a duplicate that only spells a version out where the other reaches it through a property leaves the earlier tag as written.

<dependencyManagement> merges by a different rule: each field takes the value of the first declaration that sets it, and exclusions accumulate across all of them. A later managed duplicate is removed only when it sets no field an earlier one leaves unset and carries no exclusion an earlier one lacks; otherwise all declarations stay. Declarations are never merged into one tag. A repeated BOM import counts as a duplicate only at the same version, since the first import wins for the entries both manage while the second may manage entries the first does not.

What's your motivation?

Recipe: org.openrewrite.maven.RemoveDuplicateDependencies.

Before

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.13.1</version>
</dependency>
<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.13.2</version>
</dependency>

Actual after the recipe

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.13.1</version>
</dependency>

Expected after the recipe

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.13.2</version>
</dependency>

Anything in particular you'd like reviewers to focus on?

One existing expectation changed. removeDependencyWithDifferentVersion declares com.google.inject:guice as 4.2.1 then 4.2.2 and expected 4.2.1 to survive; it now expects 4.2.2, which is what Maven resolves.

Three limits:

• The recipe can now delete an <exclusions> block belonging to the earlier of two duplicates in <dependencies>. That is intended: Maven was already ignoring it.
• A comment above the earlier declaration stays put, so it can end up describing the later declaration's content.
• Two declarations differing only in <scope> are still left alone, as on main, because scope is part of DependencyKey. Recorded as a limit, not changed here.

removeDuplicates builds the replacement child list by hand rather than with ListUtils, because a later duplicate has to replace an element at an earlier index. It copies into a new list and mutates no existing LST node.

The recipe description gained a sentence, since the recipe no longer only removes tags, and recipes.csv was regenerated with ./gradlew :rewrite-maven:recipeCsvGenerate.

Have you considered any alternatives or workarounds?

For the survivor's position: delete the earlier declaration and leave the later one where it was written. That keeps the later declaration's own indentation and any comment above it, but moves the dependency down the list. The change is contained in removeDuplicates - say the word and I will make it.

Any additional context

Pre-existing tests changed: RemoveDuplicateDependenciesTest.java.removeDependencyWithDifferentVersion (updated).

Brings RemoveDuplicateDependenciesTest from 13 to 25 tests. Ten fail without the code change, covering a later declaration carrying the <version>, <optional> or <exclusions> Maven honours; the survivor's position, indentation and comments; a managed duplicate that contributes a field or exclusion to the effective entry; and a repeated BOM import at a different version, or declared through a property.

This change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.

Why this still warrants a recipe fix

A duplicate dependency is already a POM defect and SHOULD be reviewed by the project. That does not make it safe for a cleanup recipe to keep Maven's losing declaration. A user who trusts the recipe to remove the duplicate receives a valid-looking POM whose effective dependency, exclusions, or optional flag differs from the model Maven resolved before the rewrite. The recipe MUST preserve Maven's deterministic last-direct-duplicate semantics while removing the ambiguity.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've run ./gradlew build locally, and committed any resulting changes to recipes.csv
• I've formatted the lines I changed, without reformatting code I didn't touch