Skip to content

Commit 57aabe9

Browse files
flex-gwanbinbaggwanbintimtebeek
authored
Fix UpgradePluginVersion to handle plugins with 'apply false' (#6269)
* Fix UpgradePluginVersion to handle plugins with 'apply false' The previous implementation only checked the immediate parent of the version() method call for the plugins() method. This failed when other method calls (like 'apply false') were chained between version() and plugins(). This change walks up the entire method invocation chain to find the plugins() method, correctly handling patterns like: id(...) version "x.y.z" apply false Added test coverage for both Kotlin DSL and Groovy DSL with apply false. * Add missing newline at end of UpgradePluginVersionTest.java POSIX standard requires text files to end with a newline character. * Use GradlePlugin trait in UpgradePluginVersion to simplify plugin matching Replace manual VERSION_MATCHER, isPluginVersion(), and kotlin special-casing with the GradlePlugin trait which already understands all plugin declaration forms. Also fix the trait to unwrap J.Parentheses when extracting version literals and walk up chained method invocations in withinPlugins(). * Leverage trait's plugin ID matching and remove version method filter Address PR review feedback: - Remove "version" method name filter in scanner; let trait match any plugin chain element - Use trait's built-in pluginIdPattern filtering instead of custom matchesPluginId, translating "kotlin" shorthand to "org.jetbrains.kotlin.*" - Keep "version" check in visitor only where we modify arguments * Remove kotlin shorthand translation; use full plugin ID patterns The trait already expands kotlin("jvm") to org.jetbrains.kotlin.jvm, so tests should use org.jetbrains.kotlin.* as the pattern rather than relying on a "kotlin" shorthand translation. --------- Co-authored-by: baggwanbin <flex-gwanbin@flex.team> Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 916cb78 commit 57aabe9

3 files changed

Lines changed: 100 additions & 109 deletions

File tree

‎rewrite-gradle/src/main/java/org/openrewrite/gradle/plugins/UpgradePluginVersion.java‎

Lines changed: 18 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.openrewrite.internal.ListUtils;
2929
import org.openrewrite.internal.StringUtils;
3030
import org.openrewrite.java.JavaVisitor;
31-
import org.openrewrite.java.MethodMatcher;
3231
import org.openrewrite.java.tree.Expression;
3332
import org.openrewrite.java.tree.J;
3433
import org.openrewrite.java.tree.JavaType;
@@ -56,7 +55,6 @@
5655
@EqualsAndHashCode(callSuper = false)
5756
public class UpgradePluginVersion extends ScanningRecipe<UpgradePluginVersion.DependencyVersionState> {
5857
private static final String GRADLE_PROPERTIES_FILE_NAME = "gradle.properties";
59-
private static final MethodMatcher VERSION_MATCHER = new MethodMatcher("org.gradle.plugin.use.PluginDependencySpec version(..)", true);
6058

6159
@EqualsAndHashCode.Exclude
6260
transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
@@ -110,23 +108,6 @@ public DependencyVersionState getInitialValue(ExecutionContext ctx) {
110108
return new DependencyVersionState();
111109
}
112110

113-
@SuppressWarnings("BooleanMethodIsAlwaysInverted")
114-
private boolean isPluginVersion(Cursor cursor) {
115-
if (!(cursor.getValue() instanceof J.MethodInvocation)) {
116-
return false;
117-
}
118-
J.MethodInvocation maybeVersion = cursor.getValue();
119-
if (!VERSION_MATCHER.matches(maybeVersion, true)) {
120-
return false;
121-
}
122-
Cursor parent = cursor.dropParentUntil(it -> (it instanceof J.MethodInvocation) || it == Cursor.ROOT_VALUE);
123-
if (!(parent.getValue() instanceof J.MethodInvocation)) {
124-
return false;
125-
}
126-
J.MethodInvocation maybePlugins = parent.getValue();
127-
return "plugins".equals(maybePlugins.getSimpleName());
128-
}
129-
130111
@Override
131112
public TreeVisitor<?, ExecutionContext> getScanner(DependencyVersionState acc) {
132113

@@ -164,37 +145,18 @@ public J visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionC
164145
@Override
165146
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
166147
J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
167-
if (!isPluginVersion(getCursor())) {
168-
return m;
169-
}
170-
assert m.getSelect() != null;
171-
List<Expression> pluginArgs = ((J.MethodInvocation) m.getSelect()).getArguments();
172-
if (!(pluginArgs.get(0) instanceof J.Literal)) {
173-
return m;
174-
}
175-
String pluginId;
176-
if ("kotlin".equals(((J.MethodInvocation) m.getSelect()).getSimpleName())) {
177-
pluginId = "kotlin";
178-
} else {
179-
pluginId = literalValue(pluginArgs.get(0));
180-
}
181-
if (pluginId == null || !StringUtils.matchesGlob(pluginId, pluginIdPattern)) {
148+
149+
GradlePlugin plugin = new GradlePlugin.Matcher().pluginIdPattern(pluginIdPattern).get(getCursor()).orElse(null);
150+
if (plugin == null || plugin.getPluginId() == null) {
182151
return m;
183152
}
184153

154+
String pluginId = plugin.getPluginId();
185155
List<Expression> versionArgs = m.getArguments();
186156
try {
187-
String currentVersion = literalValue(versionArgs.get(0));
188-
if (currentVersion != null) {
189-
String resolvedVersion;
190-
if ("kotlin".equals(pluginId)) {
191-
String fullPluginId = String.format("org.jetbrains.%s.%s", pluginId, literalValue(pluginArgs.get(0)));
192-
resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, gradleSettings)
193-
.select(new GroupArtifactVersion(fullPluginId, fullPluginId + ".gradle.plugin", currentVersion), "classpath", newVersion, versionPattern, ctx);
194-
} else {
195-
resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, gradleSettings)
196-
.select(new GroupArtifactVersion(pluginId, pluginId + ".gradle.plugin", currentVersion), "classpath", newVersion, versionPattern, ctx);
197-
}
157+
if (plugin.getVersion() != null) {
158+
String resolvedVersion = new DependencyVersionSelector(metadataFailures, gradleProject, gradleSettings)
159+
.select(new GroupArtifactVersion(pluginId, pluginId + ".gradle.plugin", plugin.getVersion()), "classpath", newVersion, versionPattern, ctx);
198160
acc.pluginIdToNewVersion.put(pluginId, resolvedVersion);
199161
} else if (versionArgs.get(0) instanceof G.GString) {
200162
G.GString gString = (G.GString) versionArgs.get(0);
@@ -204,31 +166,23 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
204166

205167
G.GString.Value gStringValue = (G.GString.Value) gString.getStrings().get(0);
206168
String versionVariableName = gStringValue.getTree().toString();
207-
String resolverPluginId = pluginId;
208-
if ("kotlin".equals(pluginId)) {
209-
resolverPluginId = String.format("org.jetbrains.%s.%s", pluginId, literalValue(pluginArgs.get(0)));
210-
}
211169
String resolvedPluginVersion = new DependencyVersionSelector(metadataFailures, gradleProject, gradleSettings)
212-
.select(new GroupArtifact(resolverPluginId, resolverPluginId + ".gradle.plugin"), "classpath", newVersion, versionPattern, ctx);
170+
.select(new GroupArtifact(pluginId, pluginId + ".gradle.plugin"), "classpath", newVersion, versionPattern, ctx);
213171

214172
acc.versionPropNameToPluginId.put(versionVariableName, pluginId);
215173
assert resolvedPluginVersion != null;
216174
acc.pluginIdToNewVersion.put(pluginId, resolvedPluginVersion);
217175
} else if (versionArgs.get(0) instanceof J.Identifier) {
218176
J.Identifier identifier = (J.Identifier) versionArgs.get(0);
219177
String versionVariableName = identifier.getSimpleName();
220-
String resolverPluginId = pluginId;
221-
if ("kotlin".equals(pluginId)) {
222-
resolverPluginId = String.format("org.jetbrains.%s.%s", pluginId, literalValue(pluginArgs.get(0)));
223-
}
224178
String localCurrentVersion = localVariableValues.get(versionVariableName);
225179
String resolvedPluginVersion;
226180
if (localCurrentVersion != null) {
227181
resolvedPluginVersion = new DependencyVersionSelector(metadataFailures, gradleProject, gradleSettings)
228-
.select(new GroupArtifactVersion(resolverPluginId, resolverPluginId + ".gradle.plugin", localCurrentVersion), "classpath", newVersion, versionPattern, ctx);
182+
.select(new GroupArtifactVersion(pluginId, pluginId + ".gradle.plugin", localCurrentVersion), "classpath", newVersion, versionPattern, ctx);
229183
} else {
230184
resolvedPluginVersion = new DependencyVersionSelector(metadataFailures, gradleProject, gradleSettings)
231-
.select(new GroupArtifact(resolverPluginId, resolverPluginId + ".gradle.plugin"), "classpath", newVersion, versionPattern, ctx);
185+
.select(new GroupArtifact(pluginId, pluginId + ".gradle.plugin"), "classpath", newVersion, versionPattern, ctx);
232186
}
233187

234188
acc.versionPropNameToPluginId.put(versionVariableName, pluginId);
@@ -338,31 +292,21 @@ public Properties visitEntry(Properties.Entry entry, ExecutionContext ctx) {
338292

339293
@Override
340294
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
295+
// Match the trait before super to ensure the cursor is unmodified
296+
GradlePlugin plugin = new GradlePlugin.Matcher().pluginIdPattern(pluginIdPattern).get(getCursor()).orElse(null);
297+
341298
J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
342-
if (!isPluginVersion(getCursor())) {
343-
return m;
344-
}
345-
assert m.getSelect() != null;
346-
List<Expression> pluginArgs = ((J.MethodInvocation) m.getSelect()).getArguments();
347-
String pluginId;
348-
if ("kotlin".equals(((J.MethodInvocation) m.getSelect()).getSimpleName())) {
349-
pluginId = "kotlin";
350-
} else {
351-
pluginId = literalValue(pluginArgs.get(0));
352-
}
353-
if (pluginId == null || !StringUtils.matchesGlob(pluginId, pluginIdPattern)) {
354-
return m;
355-
}
356299

357-
List<Expression> versionArgs = m.getArguments();
358-
String currentVersion = literalValue(m.getArguments().get(0));
359-
if (currentVersion == null) {
300+
if (plugin == null || plugin.getPluginId() == null || plugin.getVersion() == null ||
301+
!"version".equals(m.getSimpleName())) {
360302
return m;
361303
}
362-
String resolvedVersion = acc.pluginIdToNewVersion.get(pluginId);
304+
305+
String resolvedVersion = acc.pluginIdToNewVersion.get(plugin.getPluginId());
363306
if (resolvedVersion == null) {
364307
return m;
365308
}
309+
List<Expression> versionArgs = m.getArguments();
366310
return m.withArguments(ListUtils.map(versionArgs, v -> ChangeStringLiteral.withStringValue(v, resolvedVersion)));
367311
}
368312

‎rewrite-gradle/src/main/java/org/openrewrite/gradle/trait/GradlePlugin.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -141,35 +141,38 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) {
141141

142142
return maybeGradlePlugin(cursor, null, null, null, true);
143143
} else if (APPLY_DSL_MATCHER.matches(m, true)) {
144-
if (!(m.getArguments().get(0) instanceof J.Literal) || !(m.getSelect() instanceof J.MethodInvocation)) {
144+
Expression applyArg = m.getArguments().get(0).unwrap();
145+
if (!(applyArg instanceof J.Literal) || !(m.getSelect() instanceof J.MethodInvocation)) {
145146
return null;
146147
}
147148

148149
J.MethodInvocation versionSelect = (J.MethodInvocation) m.getSelect();
150+
Expression versionArg = versionSelect.getArguments().get(0).unwrap();
149151
if (!PLUGIN_VERSION_DSL_MATCHER.matches(versionSelect, true) ||
150-
!(versionSelect.getArguments().get(0) instanceof J.Literal) ||
152+
!(versionArg instanceof J.Literal) ||
151153
!(versionSelect.getSelect() instanceof J.MethodInvocation)) {
152154
return null;
153155
}
154156

155157
J.MethodInvocation idSelect = (J.MethodInvocation) versionSelect.getSelect();
158+
Expression idArg = idSelect.getArguments().get(0).unwrap();
156159
if (!(PLUGIN_ID_DSL_MATCHER.matches(idSelect, true) || KOTLIN_PLUGIN_DSL_MATCHER.matches(idSelect, true)) ||
157-
!(idSelect.getArguments().get(0) instanceof J.Literal)) {
160+
!(idArg instanceof J.Literal)) {
158161
return null;
159162
}
160163

161-
J.Literal idLiteral = (J.Literal) idSelect.getArguments().get(0);
162-
J.Literal versionLiteral = (J.Literal) versionSelect.getArguments().get(0);
163-
J.Literal applyLiteral = (J.Literal) m.getArguments().get(0);
164+
J.Literal idLiteral = (J.Literal) idArg;
165+
J.Literal versionLiteral = (J.Literal) versionArg;
166+
J.Literal applyLiteral = (J.Literal) applyArg;
164167
String pluginId = "kotlin".equals(idSelect.getSimpleName()) ? "org.jetbrains.kotlin." + idLiteral.getValue() : (String) idLiteral.getValue();
165168
String version = (String) versionLiteral.getValue();
166169
boolean applied = Boolean.TRUE.equals(applyLiteral.getValue());
167170
return maybeGradlePlugin(cursor, pluginId, null, version, applied);
168171
} else if (PLUGIN_VERSION_DSL_MATCHER.matches(m, true)) {
169172
String version = null;
170-
if (m.getArguments().get(0) instanceof J.Literal) {
171-
J.Literal versionLiteral = (J.Literal) m.getArguments().get(0);
172-
version = (String) versionLiteral.getValue();
173+
Expression versionArg = m.getArguments().get(0).unwrap();
174+
if (versionArg instanceof J.Literal) {
175+
version = (String) ((J.Literal) versionArg).getValue();
173176
}
174177

175178
if (!(m.getSelect() instanceof J.MethodInvocation &&
@@ -178,19 +181,21 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) {
178181
}
179182

180183
J.MethodInvocation select = (J.MethodInvocation) m.getSelect();
181-
if (!(select.getArguments().get(0) instanceof J.Literal)) {
184+
Expression idArg = select.getArguments().get(0).unwrap();
185+
if (!(idArg instanceof J.Literal)) {
182186
return null;
183187
}
184188

185-
J.Literal idLiteral = (J.Literal) select.getArguments().get(0);
189+
J.Literal idLiteral = (J.Literal) idArg;
186190
String pluginId = "kotlin".equals(select.getSimpleName()) ? "org.jetbrains.kotlin." + idLiteral.getValue() : (String) idLiteral.getValue();
187191
return maybeGradlePlugin(cursor, pluginId, null, version, !withinBlock(cursor, "pluginManagement"));
188192
} else if (PLUGIN_ID_DSL_MATCHER.matches(m, true) || KOTLIN_PLUGIN_DSL_MATCHER.matches(m, true)) {
189-
if (!(m.getArguments().get(0) instanceof J.Literal)) {
193+
Expression arg = m.getArguments().get(0).unwrap();
194+
if (!(arg instanceof J.Literal)) {
190195
return null;
191196
}
192197

193-
J.Literal literal = (J.Literal) m.getArguments().get(0);
198+
J.Literal literal = (J.Literal) arg;
194199
String pluginId = "kotlin".equals(m.getSimpleName()) ? "org.jetbrains.kotlin." + literal.getValue() : (String) literal.getValue();
195200
return maybeGradlePlugin(cursor, pluginId, null, null, !withinBlock(cursor, "pluginManagement"));
196201
}
@@ -240,12 +245,15 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) {
240245

241246
private boolean withinPlugins(Cursor cursor) {
242247
Cursor parent = cursor.dropParentUntil(value -> value instanceof J.MethodInvocation || value == Cursor.ROOT_VALUE);
243-
if (parent.isRoot() || !"plugins".equals(((J.MethodInvocation) parent.getValue()).getSimpleName())) {
244-
return false;
248+
while (!parent.isRoot()) {
249+
J.MethodInvocation parentMethod = parent.getValue();
250+
if ("plugins".equals(parentMethod.getSimpleName())) {
251+
parent = parent.dropParentUntil(value -> value instanceof J.MethodInvocation || value == Cursor.ROOT_VALUE);
252+
return parent.isRoot() || "pluginManagement".equals(((J.MethodInvocation) parent.getValue()).getSimpleName());
253+
}
254+
parent = parent.dropParentUntil(value -> value instanceof J.MethodInvocation || value == Cursor.ROOT_VALUE);
245255
}
246-
247-
parent = parent.dropParentUntil(value -> value instanceof J.MethodInvocation || value == Cursor.ROOT_VALUE);
248-
return parent.isRoot() || "pluginManagement".equals(((J.MethodInvocation) parent.getValue()).getSimpleName());
256+
return false;
249257
}
250258

251259
private boolean isProjectReceiver(Cursor cursor) {

0 commit comments

Comments
 (0)
⚡