Fix ChangeDependency routing Gradle build files to updateJavaSourceSet instead of gradleVisitor#7373
Merged
steve-aom-elliott merged 3 commits intomainfrom Apr 14, 2026
Merged
Conversation
…esent The outer TreeVisitor in getVisitor() checks `tree instanceof JavaSourceFile` to route Java source files to JavaSourceSet marker updates. However, G.CompilationUnit (Gradle build scripts) also implements JavaSourceFile, so when the scanner found the old dependency (making hasModulesWithOldDep true), Gradle files were routed to updateJavaSourceSet instead of gradleVisitor — causing no dependency changes. Add an explicit check for G.CompilationUnit and K.CompilationUnit before the JavaSourceFile branch to ensure Gradle/Kotlin build scripts are always handled by gradleVisitor.
K.CompilationUnit is used for both .gradle.kts build scripts and regular .kt source files. The previous fix using instanceof would incorrectly skip JavaSourceSet updates for Kotlin source files. Check the source path extension instead to precisely distinguish build scripts from source files.
Instead of checking file extensions to distinguish build scripts from source files, check whether the tree actually has a JavaSourceSet marker. Build scripts don't carry one, so they fall through to the gradleVisitor. This is both simpler and more semantically correct.
Jenson3210
reviewed
Apr 14, 2026
| @Test | ||
| void changeDependencyWhenJavaSourcesPresent() { | ||
| rewriteRun( | ||
| spec -> spec.recipe(new ChangeDependency("commons-lang", "commons-lang", "org.apache.commons", "commons-lang3", "3.11.x", null, null, true)), |
Contributor
There was a problem hiding this comment.
Did you check if LST's have this marker in a real example?
If they do, we should probably mock on in here also.
Contributor
Author
There was a problem hiding this comment.
The JavaParser adds the JavaSourceSet marker during parsing, so the java file is going to get it when it gets parsed as part of java(). The Gradle file is not going to get it.
Jenson3210
approved these changes
Apr 14, 2026
Contributor
|
1 question, but looks okay to me |
steve-aom-elliott
changed the title
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ChangeDependencymade no changes to Gradle build files when Java source files were also present in the source setTreeVisitoringetVisitor()routesJavaSourceFileinstances toupdateJavaSourceSetfor marker updates, butG.CompilationUnit(Gradle build scripts) also implementsJavaSourceFile, causing build files to be misrouted instead of being handled bygradleVisitorupdateJavaSourceSetwith a check for theJavaSourceSetmarker — build scripts don't carry one, so they naturally fall through togradleVisitorContext
This was introduced in #7202 which added
JavaSourceSetmarker updating toChangeDependency. TheisAcceptablemethod correctly distinguishes Gradle files from Java files, but thevisitmethod'sinstanceof JavaSourceFilecheck catches both.This breaks
ChangeDependencyfor any Gradle project where the scanner finds the old dependency (makinghasModulesWithOldDeptrue), which is the common case. Downstream, this causes test failures in rewrite-spring forRenameDeprecatedStartersManagedVersionsTest,MigrateToModularStartersTest, andBoot3UpgradeTest.Approach evolution
G.CompilationUnit/K.CompilationUnittype checks before theJavaSourceFilebranchK.CompilationUnitis used for both.gradle.ktsbuild scripts and regular.ktsource files — the type check would incorrectly skipJavaSourceSetupdates for Kotlin source filesJavaSourceSetmarker presence check — build scripts don't carry this marker, so they fall through togradleVisitor. This is both simpler and more semantically correct.Test plan
changeDependencyWhenJavaSourcesPresent— fails before fix, passes afterChangeDependencytests pass