Add AddPlatformDependency recipe.

[BrendanHart]

What's changed?

Add recipe to support adding platform/enforcedPlatform dependencies to Gradle build files.

What's your motivation?

• closes Support adding platform dependencies to Gradle project #5357

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

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

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 used the IntelliJ IDEA auto-formatter on affected files

[BrendanHart]

Currently decided to not include "onlyIfUsing" in this recipe, mostly as unsure if it should be explicit if it would be best for users to be explicit in where they want to add a platform dependency? Would appreciate any thoughts, @timtebeek / @shanman190!

[shanman190]

So having only read your comment here about onlyIfUsing, in a multi module project, what would be the pattern for only adding the platform dependency to only some of the modules?

Maybe if we go ahead and include something like a ProjectUsesType recipe that was project aware and could be utilized as a project-level precondition? That would mean that the onlyIfUsing pattern could safely be soft deprecated like we did with filePatterns on other recipes.

[BrendanHart]

Maybe there's the option of using the dependencies also to work out whether to add a platform dependency? E.g. if a module is using any dependencies matching a group org.springframework* then add the spring-boot-dependencies platform dependency.

Sorry, could you help me understand more on your second point, not sure I followed. Would that be for refactoring out the onlyIfUsing logic into something common that can be used across multiple recipes?

[shanman190]

The short answer is that preconditions are a much more powerful and expressive way to capture these types of extra conditioning (types in use, acceptable file patterns, etc).

With preconditions we can build all kinds of acceptance rules and combine them together arbitrarily in declarative recipes, including using DependencyInsight in the manner that you described for other Spring Boot dependencies being present. There are a ton of possible reasons that you may want to use a condition around when to add a dependency and by having the flexibility of preconditions it means that each of those different "reasons" can be utilized arbitrarily as a recipe author is building out their recipe(s).

[BrendanHart]

Thanks for elaborating! Is the suggestion then that these recipes don't require such restrictions built in and we can assume them to be handled in the precondition recipes, and embedding logic into the recipe to restrict to specific modules isn't required?

[shanman190]

That's kinda what I was thinking.

I'd probably go one step further to go ahead and create the precondition recipe (not 100% on the name of it though) while we're here thinking about it, but that's not strictly required. WDYT, @timtebeek ?

[timtebeek]

Do you mean to create a recipe to check if a GradleProject is using any dependency managed by a BOM? And then to add that as a precondition check on the AddPlatformDependency recipe? would be ideal, I think, to make something like this just work out of the box:

@Test
void twoProjects() {
    rewriteRun(
      spec -> spec.recipe(new AddPlatformDependency(
        "org.springframework.boot", "spring-boot-dependencies", "3.2.4", null, null, null)),
      mavenProject("projectA",
        buildGradle(
          """
            plugins {
                id "java-library"
            }
            repositories {
                mavenCentral()
            }
            dependencies {
                testImplementation "org.springframework:spring-core:6.1.5"
            }
            """,
          """
            plugins {
                id "java-library"
            }
            repositories {
                mavenCentral()
            }
            dependencies {
                testImplementation platform("org.springframework.boot:spring-boot-dependencies:3.2.4")
                testImplementation "org.springframework:spring-core"
            }
            """
        )
      ),
      mavenProject("projectB",
        buildGradle(
          """
            plugins {
                id "java-library"
            }
            repositories {
                mavenCentral()
            }
            """
          // No change, as we're not using any dependencies managed in the platform bom added
        )
      )
    );
}

[shanman190]

So I was more thinking of:

• AddPlatformDependency adds unconditionally
• Create secondary recipes for the various conditioning requirements (ie: only if using, has managed dependency, etc) to be used as preconditions on a declarative recipe (or composite programmatic recipe).

I do like the idea of scheduling a RemoveRedundantDependencyVersions recipe execute as an after visit though.

[shanman190]

Just a FYI, I have an idea that I'd like to play with locally for a day or two that would change the need for the GradleConfigurationNames over to a Trait implementation for the specific feature that it's trying to cover. I've got the JvmTestSuite trait working locally, but am presently trying to adapt it to be able to have an addDependency helper function to completely centralize the configuration name editing in addition to the trait's overall simplification and accuracy around Gradle JvmTestSuite usage and identification.

[BrendanHart]

Added more test cases and made the changes mentioned. Marked PR as ready but of course pending any changes you wish to make regarding the configuration names as already mentioned 😄

One thing I'm not sure about is how useful the "inferred" configuration would be, rather than using an explicit configuration from the recipe parameters. Let me know if it should be mandatory to specify the configuration for platform dependencies instead of having such logic as well!

[jevanlingen]

Just did a review on your code, nice contribution @BrendanHart! I like the introduction of the GradleConfigurationFilter class.

I was wondering though, does it make sense to introduce an abstract class above the AddPlatformDependency and AddDependency recipes? There is a lot of duplicated code there going on in the visitor part. Another option would be to create a separate Dependency visitor class with an "isPlatform" flag or something. What do you think? Also notice Tim's comment in the original issue:

Sharing code with the visitor as described above is of course possible and preferred here. Thanks both! - Tim

[shanman190]

I've still got this @jevanlingen . I've got a JvmTestSuite trait that I just need to fix 2 more test cases for, then it should land here.

[BrendanHart]

Thanks for the review @jevanlingen ! Will await @shanman190 's changes and any further comments I can address.

[shanman190]

@BrendanHart, @jevanlingen, and @timtebeek, apologies for the delay on this, but I've pushed the new JvmTestSuite trait. I was able to run tests successfully pre merge from main, but then began to have issues with recent Gradle marker/model changes. I'm hopeful that CI doesn't produce any new errors, but let me know what you all think.

As part of this, you'll also notice that we can also now add a dependencies block where one was missing within a JvmTestSuite as well. So that'll be handy also.