AddManagedDependency adds dependencies too low in the Maven POM's dependencyManagement section

[gjd6640]

What problem are you trying to solve?

I frequently encounter situations where I use the “AddManagedDependency” recipe and the new entry gets placed near the bottom of the dependencyManagement section. This doesn't have any effect on Maven in cases where some other BOM appearing above the new entry specifies a dependencyManagement entry for the same library or libraries. The topmost entry in the dependencyManagement section “wins”. Often, I have to move the new entry upward in the managed dependencies section so that it’ll have the desired effect.

A specific common example is projects that use the spring-boot-dependencies BOM v2.7.18. To upgrade the various Jackson libraries (due to CVEs) one has to add the Jackson BOM above that spring BOM. Note that while the BOM does internally set and use a “jackson-bom.version” maven property Maven doesn’t let you simply set this property in your project and have it be honored by the BOM.

Describe the solution you'd like

I'm open to ideas for the specific new behavior.

Based on this conversation, a "minimal impact to users" approach might be to change the default behavior of this recipe such that it additionally looks for preexisting entries in the dependencyManagement section that have scope=import and place the new managed dependency entry above the first entry whose scope is "import". If there are none with scope=import, the new entry can be placed according to whatever strategy the recipe uses today.

Alternatively, if we want to prioritize the "Principle of Least Surprise" and make the recipe behave in a way that most users would expect, it might be better to give this recipe the simple behavior of always adding managed dependencies to the top. That behavior would be trivial to explain and to understand.

@timtebeek FYI

[timtebeek]

Thanks again for logging the issue both in Slack and again here. Indeed perhaps easiest to start with always adding managed dependencies at the top if there's any dependency BOM present. I think that's still fairly straightforward with little impact. Is this something you'd want to kick off with a draft PR?

[timtebeek]

@gjd6640 I've had a quick go to try to replicate the issue you've reported, arriving at this unit test added to

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

Line 27 in 345a8f2

class AddManagedDependencyTest implements RewriteTest {

@Test
@Issue("https://github.com/openrewrite/rewrite/issues/4337")
void overrideShouldPrecedeBom() {
    rewriteRun(
      spec -> spec.recipe(new AddManagedDependency("redis.clients", "jedis", "5.1.3",
        null, null, null, null, null, null, true)),
      pomXml(
        """
          <project>
              <groupId>com.mycompany.app</groupId>
              <artifactId>my-app</artifactId>
              <version>1</version>
              <dependencyManagement>
                  <!-- Manages redis.clients:jedis:5.0.2; which should be added above the BOM to be effective -->
                  <!-- https://docs.spring.io/spring-boot/3.3.2/appendix/dependency-versions/coordinates.html -->
                  <dependencies>
                      <dependency>
                          <groupId>org.springframework.boot</groupId>
                          <artifactId>spring-boot-dependencies</artifactId>
                          <version>3.3.2</version>
                          <type>pom</type>
                          <scope>import</scope>
                      </dependency>
                  </dependencies>
              </dependencyManagement>
          </project>
          """,
        """
          <project>
              <groupId>com.mycompany.app</groupId>
              <artifactId>my-app</artifactId>
              <version>1</version>
              <dependencyManagement>
                  <!-- Manages redis.clients:jedis:5.0.2; which should be added above the BOM to be effective -->
                  <!-- https://docs.spring.io/spring-boot/3.3.2/appendix/dependency-versions/coordinates.html -->
                  <dependencies>
                      <dependency>
                          <groupId>redis.clients</groupId>
                          <artifactId>jedis</artifactId>
                          <version>5.1.3</version>
                      </dependency>
                      <dependency>
                          <groupId>org.springframework.boot</groupId>
                          <artifactId>spring-boot-dependencies</artifactId>
                          <version>3.3.2</version>
                          <type>pom</type>
                          <scope>import</scope>
                      </dependency>
                  </dependencies>
              </dependencyManagement>
          </project>
          """
      )
    );
}

This then correctly inserts jedis at the top, to override the version specified in the BOM. Would you mind clarifying, perhaps with a runnable test, in what cases you still saw issues? Was it when adding a second BOM perhaps?