-
Notifications
You must be signed in to change notification settings - Fork 428
Bump plugin version for gradle in buildscript classpath dependency scope #5570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
assert m.getSelect() != null; | ||
List<Expression> pluginArgs = ((J.MethodInvocation) m.getSelect()).getArguments(); | ||
if (!(pluginArgs.get(0) instanceof J.Literal)) { | ||
Optional<GradleDependency> gradleDependency = Traits.gradleDependency().get(getCursor()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional<GradleDependency> gradleDependency = Traits.gradleDependency().get(getCursor()); | |
Optional<GradleDependency> gradleDependency = Traits.gradleDependency().configuration("classpath").get(getCursor()); |
private @Nullable String pluginIdOrNull(J.MethodInvocation m, Optional<GradleDependency> gradleDependency) { | ||
String pluginId = null; | ||
if (m.getSelect() != null) { | ||
List<Expression> pluginArgs = ((J.MethodInvocation) m.getSelect()).getArguments(); | ||
if ((pluginArgs.get(0) instanceof J.Literal)) { | ||
pluginId = literalValue(pluginArgs.get(0)); | ||
} | ||
} else if (gradleDependency.isPresent()) { | ||
pluginId = gradleDependency.get().getResolvedDependency().getGroupId(); | ||
} | ||
if (pluginId == null || !StringUtils.matchesGlob(pluginId, pluginIdPattern)) { | ||
return null; | ||
} | ||
return pluginId; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this only tracks when the imported Gradle dependency happens to be the actual plugin marker dependency and thus pulls in the actual dependency as a transitive.
In a majority of cases where the buildscript block is being utilized, the artifact being imported doesn't share in this quality...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just as an extension, it is for this reason why UpgradeDependencyVersion
handles the dependencies section of both the current project and it's buildscript block rather than trying to make UpgradePluginVersion
work when it only has the pluginId which may have nearly zero relationship to the classpath dependency declaration.
@Getter | ||
org.openrewrite.gradle.internal.Dependency requestedDependency; | ||
|
||
public J.MethodInvocation withVersion(String version) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably return the trait itself in order to continue updating this making the method invocation update to a new cursor which replaces the current one.
See YamlValue
for details.
currentVersion = dependency.getResolvedDependency().getVersion(); | ||
if (dependency.getRequestedDependency().getVersion() != null) { | ||
acc.versionAssignmentToPluginId.put(dependency.getRequestedDependency().getVersion(), pluginId); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resolved dependency will be out of the GradleProject
marker, if present, or directly from source if it's not available for some reason. I'm not sure that the requested dependency as a secondary level is needed (read: simplification).
What's changed?
Dependency Trait now allows for
withVersion
to easily bump the version of a Dependency J.MethodInvocation and return the resulting J.MethodInvocationPlugins can now be updated by the buildscript dependencies also.
What's your motivation?
The plugin dependencies in buildscript are methodinvocations. Rather than copying/gathering stuff from elsewhere I think this approach would allow other recipes to bump a version rather easily. (later perhaps also groups/artifactids?
Checklist