Applying a Recipe to Test Sources only if Production Sources are Modified

[JLLeitschuh]

Context

As an OSS Security Researcher auto-generating security fix PRs at scale across OSS, I often run into maintainers upset when those PRs only modify tests. As such, I'm attempting to come up with a feature where OpenRewrite can filter the applicability of recipe's visitors so they are only applied if the changes will be made BOTH in production AND test. But if the recipe would only change test code, it won't generate a diff at all.

IE: If just a test source will be changed, don't modify anything at all. If a production source file is modified, then also apply the recipe to tests sources as well.

Community Question

Do you ever want to apply a recipe only to tests if production would be modified. Is this a use case you believe would be useful to you? Do you have a similar-but-different filtering problem you'd want to solve that is similar to this?

[sambsnyd]

It should be possible for us to create some sort of recipe which overrides Recipe.run() to provide this sort of functionality. Could be some challenges making it configurable from yaml/in the saas, but those could be surmounted

[JLLeitschuh]

Currently, all of the overloads of Recipe.run() are final

[shanman190]

Couldn't applicability tests be a reasonable way to handle this with some nesting?

Something like:

---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.main.SecurityModifyRecipe
displayName: Do all the things for main
description: Do all the things for main
applicability:
  anySource:
    - org.openrewrite.java.IsMainSourceSet
    - com.example.FindSecurityIssue
recipeList:
  - com.example.impl.SecurityModifyRecipe
---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.test.SecurityModifyRecipe
displayName: Do all the things for tests
description: Do all the things for tests.
applicability:
  - org.openrewrite.java.IsTestSourceSet
  - com.example.FindSecurityIssue
recipeList:
  - com.example.impl.SecurityModifyRecipe
---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.SecurityModifyRecipe
displayName: Do all the things for both
description: Do all the things for both.
applicability:
  anySource:
  - com.example.main.SecurityModifyRecipe
  - com.example.test.SecurityModifyRecipe
recipeList:
  - com.example.main.SecurityModifyRecipe
  - com.example.test.SecurityModifyRecipe

Then in the above canse, you'd run the recipe com.example.SecurityModifyRecipe.

[JLLeitschuh]

I tried your solution above, above and it doesn't work, but it did inspire me to try messing a bit more with the YAML syntax. This is what I came up with, and it seems to have most of the characteristics I'm looking for.

---
type: specs.openrewrite.org/v1beta/recipe
name: com.example.SecurityModifyRecipe
displayName: Fix Security Vulnerability
description: Only fix a security vulnerability if `main` is modified. Otherwise, fix nothing.
applicability:
  anySource:
  - org.openrewrite.java.search.HasSourceSet:
      sourceSet: "main"
  - org.openrewrite.java.search.FindMethods:
      methodPattern: "java.io.PrintStream println(..)"
recipeList:
  - org.openrewrite.java.search.FindMethods:
      methodPattern: "java.io.PrintStream println(..)"

The only thing it doesn't handle is if there is production code not in the main source set. I think we may need a filter that is IsTest that looks at things like SourceSet, if test annotations are present, ect...

Regardless, this gets me 90% of the way, so thanks for the inspiration!

[shanman190]

So I'll be honest, that I hadn't tested the yaml above, but was also using it as inspiration. Haha

I think if you have a similar structure it should get you all the way there.

The premise is that we need one recipe that acts upon all main source, then a second recipe that acts upon the test source, lastly with a recipe that combines the qualities together using the desired effect. So if you want to gate the execution based upon main being modified an anySource applicability test using your main modifying recipe should suffice for the desired goal, then you just execute both nested recipes. If you want to ensure that both source sets are changed then an anySource applicability test using both would become a logical AND (ie. main and test are modified, then...)

[JLLeitschuh]

Have a look at: #2856

[JLLeitschuh]

After some changes to OpenRewrite, the following now works to achieve the desired behaviour.

type: specs.openrewrite.org/v1beta/recipe
name: com.example.SecurityModifyRecipe
displayName: Apply `SecureTempFileCreation`
description: Applies the `SecureTempFileCreation` to non-test sources first, if changes are made, then apply to all sources.
applicability:
  anySource:
    - org.openrewrite.java.search.IsLikelyNotTest
    - org.openrewrite.java.security.SecureTempFileCreation
recipeList:
  - org.openrewrite.java.security.SecureTempFileCreation

[shanman190]

@JLLeitschuh, in the above case you can drop the second applicability test as it's not necessary for your example. In the previous discussion I had interpreted your question as the following (which would need a referential component):

If we modify main source, then modify all source.

In your case, you just wanted to say modify main source only.

So in completeness, this is the minimal form needed:

type: specs.openrewrite.org/v1beta/recipe
name: com.example.SecurityModifyRecipe
displayName: Apply `SecureTempFileCreation`
description: Applies the `SecureTempFileCreation` to non-test sources first, if changes are made, then apply to all sources.
applicability:
  anySource:
    - org.openrewrite.java.search.IsLikelyNotTest
recipeList:
  - org.openrewrite.java.security.SecureTempFileCreation

[JLLeitschuh]

If we modify main source, then modify all source.

This is the behaviour I want. Which does require both applicability tests.

[shanman190]

Gotcha! Then you're spot on! Sorry for the confusion there. 😄