Added a new unit test to check the use of variables in the plugins section in a Gradle file
What's changed?
Added a new unit test to check the use of variables in the plugins section in a Gradle file. There was an error when using variables in this section, and that was what led to the creation of the new test.
Have you considered any alternatives or workarounds?
Yes, if the variables are removed from the plugins section everything works, however, this is not a good approach as it requires changing the Gradle file.
Checklist
- [X] I've added unit tests to cover both positive and negative cases
- [X] I've read and applied the recipe conventions and best practices
- [ ] I've used the IntelliJ IDEA auto-formatter on affected files
Thanks for adding this test @lnnery ! Helpful indeed to track support for this use case too. In parallel there's also some efforts underway on the use of gradle.properties for dependency in
- https://github.com/openrewrite/rewrite/pull/3665
You might be interested to follow those as well, and the plans there to create a ScanningRecipe to handle gradle.properties files, as well as the associated Slack thread.
Sharing the failure here for easier visibility
FindPluginsTest > findPluginWithVariable() STANDARD_OUT
doesntmatter: 3: [Static type checking] - The variable [jfrogBintrayVersion] is undeclared.
@ line 3, column 39.
id 'com.jfrog.bintray' version "${jfrogBintrayVersion}"
^
1 error
FindPluginsTest > findPluginWithVariable() FAILED
java.lang.ClassCastException: class org.openrewrite.groovy.tree.G$GString cannot be cast to class org.openrewrite.java.tree.J$Literal (org.openrewrite.groovy.tree.G$GString and org.openrewrite.java.tree.J$Literal are in unnamed module of loader 'app')
at org.openrewrite.gradle.search.FindPlugins.lambda$find$0(FindPlugins.java:95)
at org.openrewrite.gradle.search.FindPlugins$$Lambda$1101/0x00007f65d892abc0.apply(Unknown Source)
at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:273)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
at org.openrewrite.gradle.search.FindPlugins.find(FindPlugins.java:99)
at org.openrewrite.gradle.search.FindPluginsTest.lambda$findPluginWithVariable$4(FindPluginsTest.java:76)
at org.openrewrite.gradle.search.FindPluginsTest$$Lambda$1090/0x00007f65d891ca10.accept0(Unknown Source)
at org.openrewrite.internal.ThrowingConsumer.accept(ThrowingConsumer.java:26)
at org.openrewrite.test.SourceSpec.lambda$beforeRecipe$4(SourceSpec.java:153)
at org.openrewrite.test.SourceSpec$$Lambda$1091/0x00007f65d891cc68.apply0(Unknown Source)
at org.openrewrite.test.internal.ThrowingUnaryOperator.apply(ThrowingUnaryOperator.java:28)
at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:323)
at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:132)
at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:127)
at org.openrewrite.gradle.search.FindPluginsTest.findPluginWithVariable(FindPluginsTest.java:62)
You might want to keep an eye out on this PR as well, which adds similar support to UpgradePluginVersion:
- https://github.com/openrewrite/rewrite/pull/3933
The above PR https://github.com/openrewrite/rewrite/pull/3933 has now been merged, and the same approach can likely be replicated here to allow finding plugins which use properties in their version number.
Still the same failure here for FindPlugins; there's been a lot of changes recently to recipes affecting Gradle to support properties files, but I suspect FindPlugins was missed so far. We can likely copy the same approach here though to get this going.
Hi @ajohnsonz, adding you here as requested by @timtebeek :)
Ah yes, thanks @lnnery ! Figured @ajohnsonz might have some insights to share as to what's needed to make FindPlugins also work with plugin versions defined in a properties file.
We might need to be mindful that if FindPlugins is currently used as a precondition, then we can not simply convert it into a scanning recipe, as scanning recipes are not supported as preconditions.
Hi @lnnery @timtebeek - you can certainly take a look at UpgradePluginVersion which has now been upgraded to work with plugins whose version is in gradle.properties - I'm not very familiar with "FindPlugin" but I assume that perhaps UpgradePluginVersion is doing both a find and a replace, so could copy some of the code out of there and remove the "replace" part?
It really depends if you care about finding the plugin and also knowing the version, or if you just care about finding the groupId and artifactId - if the latter, a simple if instanceof GString block before the downcast would at least prevent that exception. If the former I'm not really sure how that would be achieved if it can't support ScanningRecipe 🤔
Thanks for chiming in! Looking at how FindPlugins is used in AddPluginVisitor it might make most sense indeed to add that instanceof for now.
https://github.com/search?q=org%3Aopenrewrite+FindPlugins&type=code
I've pushed up a minimal change to prevent that cast exception; figured easier to iterate if there's something to critique. This now returns the variable name, which I don't think we have to resolve to be able to satisfy FindPlugins responsibility. It seems we only use this from AddPluginVisitor, where we check for the existence of the plugin, not whatever version is used there already. Good enough?