`org.openrewrite.maven.AddDependency` may add a duplicate when scopes are different

[ghusta]

Adding a dependency with Maven may add a duplicate declaration in some cases.
For example, if for whatever reason a dependency is already present with a scope runtime, and a recipe adds the same dependency without scope (or with scope compile), it will be declared twice, which should be avoided.

What version of OpenRewrite are you using?

Using rewrite-maven-plugin 6.25.0.

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.example</groupId>
    <artifactId>maven-dependency-duplicate</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    </properties>

    <dependencies>
        <!-- in case of duplicates, the last wins ! -->
        <dependency>
            <groupId>org.apache.commons</groupId>
            <artifactId>commons-lang3</artifactId>
            <version>3.20.0</version>
            <scope>runtime</scope>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>6.25.0</version>
                <configuration>
                    <activeRecipes>
                        <recipe>com.yourorg.AddDependencyExample</recipe>
                    </activeRecipes>
                </configuration>
            </plugin>
        </plugins>
    </build>
</project>

My recipe is in rewrite.yml:

type: specs.openrewrite.org/v1beta/recipe
name: com.yourorg.AddDependencyExample
displayName: Add dep commons-lang3
description: Add dep commons-lang3.
recipeList:
  - org.openrewrite.maven.AddDependency:
      groupId: org.apache.commons
      artifactId: commons-lang3
      version: 3.20.0

What is the smallest, simplest way to reproduce the problem?

Run the recipe:

mvn rewrite:run

What did you expect to see?

No changes, to be discussed.

What did you see instead?

Dependency duplicated in pom.xml, with two different scopes.

Maven warning

Maven warns when duplicate declaration of a dependency are found.

[INFO] Scanning for projects...
[WARNING] 
[WARNING] Some problems were encountered while building the effective model for org.example:maven-dependency-duplicate:jar:1.0-SNAPSHOT
[WARNING] 'dependencies.dependency.(groupId:artifactId:type:classifier)' must be unique: org.apache.commons:commons-lang3:jar -> duplicate declaration of version 3.20.0 @ line 28, column 21
[WARNING] 
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING] 
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING] 

Maven dependency resolution

In case of duplicate dependency (at the same depth of the tree in this case), the last dependency declaration wins.

Example :

    <dependencies>
        <dependency>
            <groupId>org.apache.commons</groupId>
            <artifactId>commons-lang3</artifactId>
            <version>3.20.0</version>
            <scope>compile</scope>
        </dependency>
        <dependency>
            <groupId>org.apache.commons</groupId>
            <artifactId>commons-lang3</artifactId>
            <version>3.20.0</version>
            <scope>test</scope>
        </dependency>
    </dependencies>

Dependency is resolved with scope test in that case.

Check with:

mvn dependency:resolve

[INFO] --- dependency:3.9.0:resolve (default-cli) @ maven-dependency-duplicate ---
[INFO] 
[INFO] The following files have been resolved:
[INFO]    org.apache.commons:commons-lang3:jar:3.20.0:test -- module org.apache.commons.lang3
[INFO] 

Changing the declaration order will change the resolution.

Adding a test in AddDependencyTest

Here is the test (failing) I did:

    @Test
    void addDependencyDoesntAddWhenExistingDependencyWithScopeRuntime() {
        rewriteRun(
          spec -> spec.recipe(addDependency("com.google.guava:guava:29.0-jre")),
          mavenProject(
            "project",
            srcMainJava(
              java(usingGuavaIntMath)
            ),
            pomXml(
              """
                    <project>
                        <groupId>com.mycompany.app</groupId>
                        <artifactId>my-app</artifactId>
                        <version>1</version>
                        <dependencies>
                            <dependency>
                                <groupId>com.google.guava</groupId>
                                <artifactId>guava</artifactId>
                                <version>28.0-jre</version>
                                <scope>runtime</scope>
                            </dependency>
                        </dependencies>
                    </project>
                """
            )
          )
        );
    }

NOTE: There are other combinations which generate duplicates.
Like:

EXISTING	ADD REQUEST
runtime	compile
test	compile
test	provided
test	runtime

Are you interested in [contributing a fix to OpenRewrite]

Not sure I can fix it.

[timtebeek]

Thanks for the report @ghusta ! I think @steve-aom-elliott has most recently been looking into the related classes here.

[steve-aom-elliott]

• Fix has been merged now Changing AddDependencyVisitor so it is able to broaden the scope of a direct dependency if needed #6434