Added gradle recipe to remove enableFeaturePreview method

[ryanwalker]

What's changed?

Added a gradle recipe to remove the enableFeaturePreview method from settings.gradle.
Provide the feature to remove via a property.
A rewrite.yml file would include the recipe like this:

  - og.openrewrite.gradle.RemoveEnableFeaturePreview:
      previewFeatureName: ONE_LOCKFILE_PER_PROJECT

It would remove the following line from settings.gradle

enableFeaturePreview('ONE_LOCKFILE_PER_PROJECT')

What's your motivation?

I wrote this recipe for use at my company and was encouraged to contribute it here by @shanman190 and @timtebeek. Slack thread: https://rewriteoss.slack.com/archives/C01A843MWG5/p1715414946440599?thread_ts=1715381394.713129&cid=C01A843MWG5

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

[shanman190]

Left a few comments, but overall things look good.

nit: You might review your formatter options or we can clean this up once the PR is ready.

[shanman190]

If I recall correctly, the type information should be present such that this can be done with a MethodMatcher.

MethodMatcher enableFeaturePreviewMatcher = new MethodMatcher("RewriteSettings enableFeaturePreview(String)");
if (enableFeaturePreviewMatcher.matches(m)) {
    // do the thing
}

https://github.com/openrewrite/rewrite/blob/main/rewrite-gradle/src/main/groovy/RewriteSettings.groovy#L32-L36

[ryanwalker]

The pattern above didn't work, but this one below did:

new MethodMatcher("org.gradle.api.initialization.Settings enableFeaturePreview(String)");

I debugged and constructed a MethodMatcher by passing in the actual method object, then looked at the toString() and saw this. Let me know if I should do something different.

[shanman190]

That works, you could have also flipped the inheritance included parameter to true as well. I forgot that in my example, but this does the same thing.

[ryanwalker]

Looks like it needs a license. I can add one to the file if that's the process.

[timtebeek]

Ah normally we have a bot to then give you helpful suggestions, but not yet on this project. A quick ./gradlew lF should clear out this issue!

[ryanwalker]

Don't merge this yet, I tested this locally and it didn't work. Not sure if it's my fault or not. I published to maven local, then ran it on a project, and it didn't make any changes. I added some logging statements and it seems the method matcher (or something else) isn't quite right. I'll look into it tomorrow.

I added a failing test to try and keep it from getting merged.

[ryanwalker]

So I tested this on the spring-petclinic-migration project and it worked fine, it removed the method.

I tested it on one of our microservices and it did not do it. I printed out some debug statements and found that the method.getMethodType() is returning null and so the method matcher doesn't work. My project is a multi-module gradle project, not sure if that would have anything to do with it. Any suggestions?

Here's some debug info for the spring-petclinic project that did work, I had a breakpoint in the IsSettingsGradle.java class, method type has a value.

Here is debug info for my local microservice where it did NOT work, methodType is null:

[ryanwalker]

I was using a method matcher, and it passed tests fine, but I manually ran this recipe against a few local gradle projects and it worked on 1 but on another it didn't match at all. The one it didn't work on, I debugged the LST and found that the method had a methodType of null for some reason. I dont' know if it's a bug in the parser or not, but I changed the logic here to just check the method name, check that there's exactly 1 argument, and check that the argument matches the @Option passed in.

[shanman190]

It being flaky is a bit curious to me. My likely suspect though is that in the cases where it is not working, the type is missing from the Groovy AST -- coming out of the Groovy Compiler -- prior to it being mapped to the OpenRewrite Groovy LST.

[timtebeek]

Added a bit of polish to minimize the implementation & better adhere to some project conventions that aren't automated on this particular project yet, but are in others.

Thanks a lot for the work done here! I'd say this is good to go as is, but will give Shannon a little time to respond if he sees anything still.

[timtebeek]

I was wondering if we should include any declarative Gradle upgrade recipes already for major versions that remove experimental features removed per major version.

https://github.com/gradle/gradle/blob/v8.7.0/subprojects/core/src/main/java/org/gradle/api/internal/FeaturePreviews.java

GROOVY_COMPILATION_AVOIDANCE(true, null),
TYPESAFE_PROJECT_ACCESSORS(true, null),
STABLE_CONFIGURATION_CACHE(true, "org.gradle.configuration-cache.stable");

https://github.com/gradle/gradle/blob/v7.2.0/subprojects/core/src/main/java/org/gradle/api/internal/FeaturePreviews.java

GROOVY_COMPILATION_AVOIDANCE(true),
ONE_LOCKFILE_PER_PROJECT(false),
VERSION_ORDERING_V2(false),
VERSION_CATALOGS(true),
TYPESAFE_PROJECT_ACCESSORS(true);

https://github.com/gradle/gradle/blob/v6.9.4/subprojects/core/src/main/java/org/gradle/api/internal/FeaturePreviews.java

GRADLE_METADATA(false),
GROOVY_COMPILATION_AVOIDANCE(true),
ONE_LOCKFILE_PER_PROJECT(true),
VERSION_ORDERING_V2(true);

https://github.com/gradle/gradle/blob/v5.6.4/subprojects/core/src/main/java/org/gradle/api/internal/FeaturePreviews.java

IMPROVED_POM_SUPPORT(false),
GRADLE_METADATA(true),
STABLE_PUBLISHING(false),
INCREMENTAL_ARTIFACT_TRANSFORMATIONS(false),
GROOVY_COMPILATION_AVOIDANCE(true);

https://github.com/gradle/gradle/blob/v4.10.3/subprojects/core/src/main/java/org/gradle/api/internal/FeaturePreviews.java

IMPROVED_POM_SUPPORT(true),
GRADLE_METADATA(true),
STABLE_PUBLISHING(true);

[shanman190]

This looks good to me, @timtebeek. Just the one test that I kinda feel is valid from a testing standpoint, but invalid from a semantics standpoint.

[shanman190]

I do like the idea of starting some migration recipes for Gradle. We have enough of the low level components to make it work fairly well.

[timtebeek]

Great, then let's follow up with a declarative yaml recipe in a separate PR that uses the above recipe and list of feature flags in #4191 (comment), and adds in updating the Gradle wrapper too.