Fix FindDependency to exclude constraints
#5663
Conversation
## Changes
- Modified `FindDependency.java` to skip matching dependencies inside `constraints { }` blocks
- Added `isWithinConstraintsBlock()` method to traverse AST and detect constraint context
- Added comprehensive tests verifying constraints are ignored while regular dependencies are still found
Fixes:
- #5599
rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindDependencyTest.java
Outdated
Show resolved
Hide resolved
rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindDependencyTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
@knutwannheden , any reason to not refactor this to the |
|
That would probably make sense. |
|
Figured give that a quick shot, as it does clean up the code nicely. But now we seem to fail on .kts. :( |
|
@timtebeek , I think I see what the issue is there. With these test cases, they don't add the Also note, I believe that this test case is an invalid Gradle Kotlin DSL script, but only marginally so. When applying plugins as a literal I believe it has to be a back ticked string versus a quoted string. I haven't tested this, but I've also never seen a script with a quoted string plugin application like is described here. |
|
Thanks for the help here @shanman190 ; your suggestions as usual where spot on. With that I think this is good to merge: we still pass all the tests we did before, we pass additional tests, and we now use the |
|
Previously we were also able to find some older Gradle version dependencies; should the Gradle Matcher trait be updated to allow for those as well? |
|
@timtebeek , the trait prefers using the Gradle model itself for this exact reason. Now the reason why the test fails when attempting to check for older configurations is because those don't exist within the Gradle version that we use in the tooling API by default and for this reason is exactly why the tooling API helped methods expose a version property to the test author. Gradle 7.0 removed these old configurations officially, so since when we don't specify the version we will use Gradle 8.12, this is why those test examples fail under the current conditions. |
| "kapt", | ||
| "ksp" |
There was a problem hiding this comment.
These are not Gradle native configurations and instead come from a custom configuration that come from a plugin not defined within this test case which explains why they fail.
| "debugImplementation", | ||
| "releaseImplementation", | ||
| "androidTestImplementation", | ||
| "featureImplementation", |
There was a problem hiding this comment.
And these all come from the Android plugin.
| "testCompile", // deprecated | ||
| "testRuntime" // deprecated | ||
| }) | ||
| void decprecatedMethods(String method) { |
There was a problem hiding this comment.
This is presently failing because Gradle 6.9.4 supports Java 15 at the time of its release... We may need to think about how to deal with these sorts of things...
Changes
ModifiedFindDependency.javato skip matching dependencies insideconstraints { }blocksAddedisWithinConstraintsBlock()method to traverse AST and detect constraint contextAdded comprehensive tests verifying constraints are ignored while regular dependencies are still found
Switch implementation to use GradleDependency trait, for consistent match and to remove duplicated logic.
Fixes
gradle.FindDependencyalso finds Gradle Dependency Constraints #5599