Singlesource applicability with FindSourceFiles now applies changes to all files

[gsmet]

Hi,

I'm not sure if it's a problem with OpenRewrite itself or the Rewrite Maven plugin but after an upgrade to 4.41.0, the Rewrite Maven plugin starts pushing unwanted changes to my YAML and properties files.

When rewriting https://github.com/quarkiverse/quarkus-github-app with our Quarkus upgrade script, I end up with the following diff between a rewrite with 4.39.0 and a rewrite with 4.41.0:

(the private key is a test one that isn't used anywhere, no worries here)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 84703f7..b2452e3 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -30,6 +30,7 @@ jobs:
       fail-fast: false
       matrix:
         java:
+          
           - {
             version: "11"
           }
diff --git a/.github/workflows/downstream.yml b/.github/workflows/downstream.yml
index fdfd744..3a918c0 100644
--- a/.github/workflows/downstream.yml
+++ b/.github/workflows/downstream.yml
@@ -31,6 +31,7 @@ jobs:
       fail-fast: false
       matrix:
         project:
+          
           - {
             organization: "quarkusio",
             repository: "quarkus-github-bot"
-quarkus.github-app.private-key=-----BEGIN RSA PRIVATE KEY-----\
-MIIEowIBAAKCAQEAtsCKYkMzgqSsbkcPThTjeYnd6r5RAS4q5SyiOyIf34nYH5D0\
-jy8Dlt1s8u69KmBoM9Neesqf2Yh80b/UNUtea9knpgR4ME+MIg1LdsLnUWoZuPBl\
-lLaMfwRkN4+ZbTJ7Uo98UUlxoTcCjZVzopSoIazuUF5u+01sx6QEcHB312s42FVL\
-gkd8T36wvIywseOPKr2oyCMQ2AdcxL/cEHwx2hcQAe+eTipsxNy4atIhbDWrML5W\
-v7cYlyn9/sKgxkXtW3eInP3VI3FS+IH+cjwO4FS6ez8TT6USfaCjDb4LgdEKt/de\
-Ds4TbqwM2vjqedLNBhE7t4V2Uf3STdFq+cKHwwIDAQABAoIBAE07MUA1hh8/2F8C\
-SMWGrls+mDoME8+b4yTWp/i1gyLE7kDo0XFxPOMU0GYZ3nd6Jo9AVD0wRD16IMXD\
-e7rKDy0kqEzQtroz49TAKZQW6grN+/DcJxGh094ZzQBt/zjWjKdnW/I+R3cJ+Fo9\
-PpEGoccZfd0ZC23IWqBEAFxEK2Eth7CbJW5eMJElyqSZa9jiF6wdnryH4Xww38iE\
-zv5tL2Ieg03lxj0mdeyzFyIzJTk3PEAFLIjIiducIhSWyJHltpKQEZ3iFMzKfx7l\
-U0KGUYECgYEA5xcnO9nv58QdcMXobHWethh+a2QTWmEXbE/Lnunqgq+bTgJUbgXR\
-Fi+awb+6GI8lJh16oBbzSRGe2pFQcQa9S84+CnAyVVyras4WC5bcl38q46wh0IRW\
-2sqvJfAI/CJZxgLVceELJ4hn9t8/7Ucoh6ZYYSsVnwtvvrmzMsC1z/0CgYEAynOC\
-v0O+FC5MI+p6HQXdK8JWNecIMEoat5RpmF6nvYSRmHliggoeAV5Of7b6Y0JBRgGm\
-+wECWCKMfaJ1BdCOncci+YkLguSBeJGbDkr65En/zzPxtuvYqcg05x50RWdbLQ5N\
-PF6FHSC8FILpfpJ5f/W8rfNNmU4eTKS4DxUr4r8CgYEA4vfqoO48ovYLtGetEFm1\
-uEP2ZqO0HmCeENOOulYk7pZrgwLmyekMoy2+Ye1daiGt6vGpLvNbn7ievS1cRKbJ\
-5Vp7tOTditmpww0GuftCTcmo5lR6IcLZS6smu6w2Ju3WHpVJ7r+JpRpkgiRjNTle\
-pVzMESOv6LXi2wCo8IA2EkECgYAOyzQRr+yS4vMzaK31svj/epr8I17I0JF1OsYg\
-mUIeqjJNdwlIwV6B8RdBY+iWGkBU0kgWbXNzZ0rm31k3zI6vXt7iZy5NKU+AtPsk\
-pzwANJwZ0wzltgRGG9gpz2Lls3DJMRNZxvppL3wu74YKdr+kJxvbhjz0Z+304dCF\
-YaGsVwKBgA6KSf1pPD93W/v5tV0WnhBZGKb3sX34JLQoCJPS5w6nTjHnuwpP9g0K\
-C8HXKULXuX/rb/gz99PNcPQ9GqB0Rw/ho1KhO3VLJjBiSh/hiASKuFGs2y59UXYt\
-mvqX3jAqi6ksGD5WPG6G2CdL/n30e5/lc10hq7SiwV7z9njW/8Pu\
------END RSA PRIVATE KEY-----
+quarkus.github-app.private-key=-----BEGIN RSA PRIVATE KEY-----MIIEowIBAAKCAQEAtsCKYkMzgqSsbkcPThTjeYnd6r5RAS4q5SyiOyIf34nYH5D0jy8Dlt1s8u69KmBoM9Neesqf2Yh80b/UNUtea9knpgR4ME+MIg1LdsLnUWoZuPBllLaMfwRkN4+ZbTJ7Uo98UUlxoTcCjZVzopSoIazuUF5u+01sx6QEcHB312s42FVLgkd8T36wvIywseOPKr2oyCMQ2AdcxL/cEHwx2hcQAe+eTipsxNy4atIhbDWrML5Wv7cYlyn9/sKgxkXtW3eInP3VI3FS+IH+cjwO4FS6ez8TT6USfaCjDb4LgdEKt/deDs4TbqwM2vjqedLNBhE7t4V2Uf3STdFq+cKHwwIDAQABAoIBAE07MUA1hh8/2F8CSMWGrls+mDoME8+b4yTWp/i1gyLE7kDo0XFxPOMU0GYZ3nd6Jo9/I+R3cJ+Fo9PpEGoccZfd0ZC23IWqBEAFxEK2Eth7CbJW5eMJElyqSZa9jiF6wdnryH4Xww38iEzv5tL2Ieg03lxj0mdeyzFyIzJTk3PEAFLIjIiducIhSWyJHltpKQEZ3iFMzKfx7lqLIczsyLRouc64+T7lW1PO6SQshHlvc6CtJaHHf98iFMDwdDlSolF/PfNLE5C6lAU0KGUYECgYEA5xcnO9nv58QdcMXobHWethh+a2QTWmEXbE/Lnunqgq+bTgJUbgXRFi+awb+6GI8lJh16oBbzSRGe2pFQcQa9S84+CnAyVVyras4WC5bcl38q46wh0IRW2sqvJfAI/CJZxgLVceELJ4hn9t8/7Ucoh6ZYYSsVnwtvvrmzMsC1z/0CgYEAynOCv0O+FC5MI+p6HQXdK8JWNecIMEoat5RpmF6nvYSRmHliggoeAV5Of7b6Y0JBRgGm+wECWCKMfaJ1BdCOncci+YkLguSBeJGbDkr65En/zzPxtuvYqcg05x50RWdbLQ5NPF6FHSC8FILpfpJ5f/W8rfNNmU4eTKS4DxUr4r8CgYEA4vfqoO48ovYLtGetEFm1uEP2ZqO0HmCeENOOulYk7pZrgwLmyekMoy2+Ye1daiGt6vGpLvNbn7ievS1cRKbJ5Vp7tOTditmpww0GuftCTcmo5lR6IcLZS6smu6w2Ju3WHpVJ7r+JpRpkgiRjNTlepVzMESOv6LXi2wCo8IA2EkECgYAOyzQRr+yS4vMzaK31svj/epr8I17I0JF1OsYgmUIeqjJNdwlIwV6B8RdBY+iWGkBU0kgWbXNzZ0rm31k3zI6vXt7iZy5NKU+AtPskpzwANJwZ0wzltgRGG9gpz2Lls3DJMRNZxvppL3wu74YKdr+kJxvbhjz0Z+304dCFYaGsVwKBgA6KSf1pPD93W/v5tV0WnhBZGKb3sX34JLQoCJPS5w6nTjHnuwpP9g0KC8HXKULXuX/rb/gz99PNcPQ9GqB0Rw/ho1KhO3VLJjBiSh/hiASKuFGs2y59UXYtmvqX3jAqi6ksGD5WPG6G2CdL/n30e5/lc10hq7SiwV7z9njW/8Pu-----END RSA PRIVATE KEY-----

So OpenRewrite is somehow:

• adding new lines (with trailing spaces) to my YAML
• reformatting my properties, removing the new lines in values

That's not something I want so I'm wondering what is going on.
Let me know if it's more of a core issue that should be moved there.

Thanks.

[timtebeek]

Thank you for reporting this here! I'm adding a couple of hopefully helpful links here to aid troubleshooting:
You report going from openrewrite/rewrite-maven-plugin@v4.39.0...v4.41.0
Which in turn goes from rewrite v7.35.0...v7.37.3 .

I don't immediately see any relevant changes in the 261 commits. Would you mind testing any intermediate maven plugin versions to help narrow it down? Or provide the command you're using to perform your upgrades?

[gsmet]

hi @timtebeek ,

Sure I will bissect tomorrow and see if I can pinpoint it to one particular version.

[timtebeek]

Thanks a lot!

[gsmet]

I think I found out what the issue is.

My recipes targeting singleSource with wildcards are applied to all the files of the project, totally ignoring the applicability, AFAICS:

I get:

[WARNING] Changes have been made to runtime/src/main/java/io/quarkiverse/githubapp/runtime/github/GitHubConfigFileProviderImpl.java by:
[WARNING]     io.quarkus.updates.core.quarkus30.Quarkus3
[WARNING]         io.quarkus.updates.core.quarkus30.JavaxToJakartaDocumentation
[WARNING]             org.openrewrite.text.FindAndReplace: {find=javax.json.bind.config., replace=jakarta.json.bind.config.}
[WARNING]                 org.openrewrite.Recipe$AdHocRecipe
...

(so OpenRewrite adjusting a .java file)

while I have for io.quarkus.updates.core.quarkus30.JavaxToJakartaDocumentation the following recipe:

type: specs.openrewrite.org/v1beta/recipe
name: io.quarkus.updates.core.quarkus30.JavaxToJakartaDocumentation
applicability:
  singleSource:
    - org.openrewrite.FindSourceFiles:
        filePattern: "**/*.adoc"
recipeList:
  - org.openrewrite.text.FindAndReplace:
      find: javax.json.bind.config.
      replace: jakarta.json.bind.config.
  ...

So this rule should only be applied to *.adoc files of my project but it is applied to Java files, properties, YAML..., thus modifying files that shouldn't have been modified (and me noticing it).

The upgrade starts being extremely slow as it applies all our text transformations to all the files instead of targeting only the ones in the applicability.singleSource.

I bissected and AFAICS the regression was introduced in OpenRewrite 7.37.0 (and it is still a problem in 7.37.3). 7.36.1 is working as expected.

So unless I'm doing something incorrect (but that was working before), a regression was probably introduced in 7.37.0 regarding applicability singleSource.

For reference, our recipes are hosted here: https://github.com/quarkusio/quarkus-updates/blob/main/recipes/src/main/resources/quarkus-updates/core/3alpha.yaml

[timtebeek]

Thanks a lot both for bisecting where the behavior changed as well as providing the additional context around the recipe & applicability test used. It looks like you're using a pattern similar to what we have in the documentation, but curiously that's also the only place I see FindSourceFiles used in that way. 🤔 That kind of makes me doubt if that's the correct way to limit recipe execution for text replacements to certain files. For instance: elsewhere we use HasSourcePath to limit recipes to certain files, which could be helpful here as well.

Would you mind giving this a try with HasSourcePath? That could indicate this is a "bug" in the documentation instead.

[shanman190]

So this was because applicability tests didn't apply correctly and was fixed here (which is in the commit range of differences that @timtebeek tagged in his earlier comment):

d974a7d

Applicability tests were inappropriately selecting sources under the single source case, especially, which is what led to your issue here. If I were you, I'd try this again on the newest maven plugin and you should have much better luck.

[gsmet]

@shanman190 I don't think it's related given my problem is specifically with 7.37.0..7.37.3 and the commit you pointed out was introduced in 7.36.0.

@timtebeek I'll give it a try but IIRC, the pattern we are using was pointed out by someone from the OpenRewrite team so we are probably not the only ones doing that. I'll post about my findings soon.

[shanman190]

@gsmet, I missed that on the first read; apologies about missing that!

[gsmet]

@timtebeek the issue is that I cannot use HasSourcePath in my YAML descriptor directly as it is not a Recipe, it's a TreeVisitor.

My understanding is that using FindSourceFiles was the blessed way to apply recipes to a bunch of files.

[gsmet]

@timtebeek any news? Asking that because I'm hitting another issue with 7.36.0 that seems to be fixed with the latest and I'm in an uncomfortable situation.

Thanks!

[timtebeek]

Understandable that you're looking to see this resolved; I don't have any immediate answers, but I've asked @kunli2 if he can chime in to see what (if any) changes need to be made to get you on the latest versions again.

[jbessels]

I started with Open Rewrite 4.40.0 iirc and did a migrate Java 11 with it. Wanted to do a migrate Java 17 but got the "[OpenRewrite: The contents of a quark are unknown, so the charset is unknown] bug. This was fixed in 4.41.1, so upgraded. Could now do the migration. Also noticed that in the Yaml files all new lines were indeed removed and now its all on 1 line.

[gsmet]

I would bet it could be related to 9b72d9a .

I will try the version before this commit then the version after and report back.

[gsmet]

After some bisecting, I can confirm that 9b72d9a is the culprit.

[timtebeek]

Thanks a lot for your efforts to really pin this down; hope @kunli2 is able to use that information for a fix or workaround.

[kunli2]

Thanks for all the useful findings so far, I start working on this now.

[kunli2]

this is fixed by #2962, I have verified the https://github.com/quarkiverse/quarkus-github-app and didn't see any unexpected changes.

[gsmet]

Thanks @kunli2 , I tested latest main and AFAICS the issue is gone!

Looking forward to having a version of the Maven plugin coming with the fix!