openrewrite / rewrite-gradle-plugin

OpenRewrite's Gradle plugin.
Apache License 2.0
60 stars 37 forks source link

Support per source set configurations #194

Closed Meijuh closed 1 year ago

Meijuh commented 1 year ago

If I'm correct, the Gradle plugin currently does not support per source set configurations. This means there can only be one set of recipes for all source sets. It would be useful to specify a set of recipes per source set, with defaults for the main and test source set. What I would like to see is something like the following.

rewrite {
    activeStyle "org.openrewrite.java.GoogleJavaFormat"
}

rewriteMain {
  activeRecipe "recipe.A"
}

rewriteTest {
  activeRecipe "recipe.B"
}

A practical use case is removing a particular set of annotations in the main source set, while removing a different set of annotations in the test source set.

shanman190 commented 1 year ago

@Meijuh, you can achieve this presently through the use of an applicability test from rewrite proper. So assuming that recipe.A and recipe.B modify the components with respect to the desired main vs test trait that you describe, you would then compose them in another recipe like below:

---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.MigrateMainAnnotations
displayName: Apply the main annotation migrations
description: Applies the main annotation migrations.
applicability:
  anySource:
    - org.openrewrite.java.search.HasSourceSet:
        sourceSet: "main"
recipeList:
  - recipe.A
---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.MigrateTestAnnotations
displayName: Apply the test annotation migrations
description: Applies the test annotation migrations.
applicability:
  anySource:
    - org.openrewrite.java.search.HasSourceSet:
        sourceSet: "test"
recipeList:
  - recipe.B
---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.MigrateAnnotations
displayName: Applies annotation migrations
description: Applies annotation migrations.
recipeList:
  - com.example.MigrateMainAnnotation
  - com.example.MigrateTestAnnotation

Finally, you would run the com.example.MigrateAnnotations recipe now which would apply correctly to all source code as desired.

You can read more about applicability tests here in the documentation: https://docs.openrewrite.org/reference/yaml-format-reference

sambsnyd commented 1 year ago

There are many different ways recipe application may need to be sliced and diced. By source set, by dependency usage, by type usage, by presence of some security vulnerability, etc. We cannot build every desirable criteria into the plugin DSL, as those desirable criteria are legion. I agree with @shanman190 that the way to address this is with applicability tests for source set on your recipe(s).

Meijuh commented 1 year ago

The applicability mechanism will work for me too :). Thanks for the great example.

Meijuh commented 1 year ago

@sambsnyd this solution is not supported anymore by OpenRewrite 8: https://docs.openrewrite.org/reference/yaml-format-reference#preconditions. Do you have a recommended solution for OpenRewrite 8? I could write a recipe myself in Java, but maybe there already exists some (skeleton) code I could use?

shanman190 commented 1 year ago

@Meijuh, yeah, I've also felt the pain of the removal of applicability. There's been a couple of proposals for adding it back, but nothing concrete yet. For this use case, using a Java recipe is the quickest way forward. It would look something like this:

package com.example;

...

class MigrateMainAnnotations extends Recipe {
    ...

    public TreeVisitor<?, ExecutionContext> getVisitor() {
        return Preconditions.check(
                new HasSourceSet("main"),
                new TreeVisitor<?, ExecutionContext>() {
                     public Tree visit(Tree tree, ExecutionContext ctx) {
                         doAfterVisit(recipe.B(...).getVisitor());
                         return tree;
                     }
                 }
        );
    }
}

However, if a target recipe is a ScanningRecipe then you'll probably have a few issues getting that one to behave appropriately. @sambsnyd might have a way to contend with those types in a similar way as the general purpose recipe types are with the above.

Meijuh commented 1 year ago

@shanman190 Thanks again for the example, I think I can make that work similar to what I have now.

However, I'd like to note my use case (which I forgot to mention earlier). We're using the groovy-gradle-plugin to bundle OpenRewrite into our own plugin with some recipes activated by default. We can then apply our own plugin to the multiple other Gradle projects that we have. In your example the name of source sets is fixed at compile time. In our case, we essentially know the name of the source set only at run time (i.e., when applying our own plugin to a project). I'm unsure how I could always evaluate the parameter of new HasSourceSet(...) properly at runtime. Do you think my use case can be considered when adding back source set based "applicability"?

shanman190 commented 1 year ago

So you could expose the source set as an option on the recipe and that should get you some configurability. I was just thinking of a direct replacement example for the first two recipes that do make use of the applicability feature. Then those two new Java recipe replacements would just get used within the last YAML recipe as they were before (if you notice I kept the same coordinates for them).

Meijuh commented 1 year ago

Such a direct replacement would be very helpful to me. How would it look like?

shanman190 commented 1 year ago

Adapting the previous example, it would look something like this:

package com.example;

...

class MigrateAnnotations extends Recipe {
    ...

    @Option(displayName = "Source set",
            description = "The source set to modify",
            example = "main")
    String sourceSet;

    public TreeVisitor<?, ExecutionContext> getVisitor() {
        return Preconditions.check(
                new HasSourceSet(sourceSet),
                new TreeVisitor<?, ExecutionContext>() {
                     public Tree visit(Tree tree, ExecutionContext ctx) {
                         doAfterVisit(recipe.B(...).getVisitor());
                         return tree;
                     }
                 }
        );
    }
}

Above changes:

Meijuh commented 1 year ago

Thanks for the updated example. Currently, recipe.B is configured in YAML, would I need to use the Java-based org.openrewrite.java.RemoveAnnotation instead?

shanman190 commented 1 year ago

So you can load and execute a declarative recipe from a Java recipe, but personally I find it easier to just use the native Java recipe directly myself.

Environment.builder().scanRuntimeClasspath().build().activateRecipes("recipe.B", "recipe.C", ...);

NOTE: if a recipe is a ScanningRecipe in your declarative form, then this probably won't work right for that recipe.