Draft: Created a test for UpgradeTransitiveDependencyVersion that demonstrates the "Gradle visitor used for a Maven project" issue

gjd6640 commented 4 months ago

What's your motivation?

I'm learning to use OpenRewrite. My overall goal is to create tooling to resolve CVEs across many Maven Java projects.

I have been unsuccessfully trying to use "org.openrewrite.java.dependencies.UpgradeTransitiveDependencyVersion" to increase version numbers. Today I found that "org.openrewrite.maven.UpgradeTransitiveDependencyVersion" worked for my specific example but "org.openrewrite.java.dependencies.UpgradeTransitiveDependencyVersion" did nothing. I'm under the impression that the latter is the newer and more flexible recipe.

Anything in particular you'd like reviewers to focus on?

This PR contains a new test class that demonstrates the setup, expected results, and the comments show which recipes worked and which didn't. When I used a debugger to see where things went wrong it looked like the issue was that the gradle isAcceptable call on line 129 of UpgradeTransitiveDepedencyVersion evaluated to true which prevented it from running the maven visitor code on line 132.

                if(gradleUDV.isAcceptable(t, ctx)) {  // This was true
                    t = (SourceFile) gradleUDV.visitNonNull(t, ctx);  // This ran...
                } else if(mavenUTDV.isAcceptable(t, ctx)) {
                    t = (SourceFile) mavenUTDV.visitNonNull(t, ctx);  // This never ran
                }

Ability to help further

Unfortunately, I can't commit the time required to figure out how to get Gradle working and deliver a fix for this. I'm hoping that by creating this test I'm making it easy enough for some else to quickly grok this and code up a fix.

Workarounds

I plan to use the maven-specific recipe mentioned above in the meantime since that one seems to work.

Checklist

I'm not set up to work with Gradle projects so this code is just copy-pasted from where I worked on it in a fork of rewrite-recipe-starter. Hopefully it is clean enough for you to work with.

[ ] I've added unit tests to cover both positive and negative cases
[ ] I've read and applied the recipe conventions and best practices
[ ] I've used the IntelliJ IDEA auto-formatter on affected files

timtebeek commented 4 months ago

Hi @gjd6640 ; Somehow I didn't get notified of this PR, hence the delayed feedback here. Sorry about that!

You mentioned you're looking to resolve vulnerabilities in transitive dependencies; that's great!

For context: From the use case you've shared I can see the transitive version of snappy-java is 1.1.7.3 in https://repo1.maven.org/maven2/org/apache/spark/spark-core_2.12/2.4.4/spark-core_2.12-2.4.4.pom

We'll have to explore why indeed that Maven version is working for you, while the delegating Maven/Gradle agnostic version does not work.

org.openrewrite.java.dependencies.DependencyVulnerabilityCheck

Do note that we also have a dedicated recipe to resolve vulnerable dependencies, which leverages the recipes you use here: https://docs.openrewrite.org/recipes/java/dependencies/dependencyvulnerabilitycheck One caveat:

This recipe only upgrades to the latest patch version. If a minor or major upgrade is required to reach the fixed version, this recipe will not make any changes.

In addition to bumping .patch versions the recipe also produces a CSV data table in target/ named org.openrewrite.java.dependencies.table.VulnerabilityReport.csv.

timtebeek commented 4 months ago

Thanks @gjd6640 ; I've traced it to this issue, which will then likely resolve the issues you're seeing here.

https://github.com/openrewrite/rewrite/pull/4327

gjd6640 commented 4 months ago

Nice! Thank you!

openrewrite / rewrite-java-dependencies