Name a ChangeDependency instance by the coordinates it changes - #196
Merged
Conversation
Both recipes this one delegates to already do this. `ChangeDependencyGroupIdAndArtifactId` and Gradle's `ChangeDependency` each render `` `oldGroupId:oldArtifactId` `` as their instance name suffix, so the unified recipe was the only one of the three that fell back to a bare display name. The default `getInstanceName()` interpolates an option only when exactly one is required. This recipe requires two, so every instance came out as "Change Gradle or Maven dependency" regardless of what it changed. A composite that lists several of them — the Spring Boot 4.0 migration lists seven — reads as the same line repeated, in the recipe list and in the `SourcesFileResults` and `RecipeRunStats` data tables alike.
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
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.
What's changed
ChangeDependencynow implementsgetInstanceNameSuffix(), rendering the coordinates it acts on:Why
The two recipes this one delegates to already do exactly this, byte for byte:
ChangeDependencyGroupIdAndArtifactIdorg.openrewrite.gradle.ChangeDependencySo the unified recipe was the only one of the three whose instance name did not name its
coordinates, and the same logical change read differently depending on which build tool it
dispatched to.
The default
Recipe.getInstanceName()interpolates an option value only when exactly oneoption is required.
ChangeDependencyrequires two (oldGroupId,oldArtifactId), so itfell through to the bare display name and every instance came out identical:
That is most visible in a composite that lists several.
UpgradeSpringBoot_4_0lists seven,and they are the starter renames that make up much of a Boot 4 migration:
spring-boot-starter-webspring-boot-starter-webmvcspring-boot-starter-aopspring-boot-starter-aspectjspring-boot-starter-oauth2-clientspring-boot-starter-security-oauth2-clientspring-boot-starter-oauth2-resource-serverspring-boot-starter-security-oauth2-resource-serverspring-boot-starter-oauth2-authorization-serverspring-boot-starter-security-oauth2-authorization-serverspring-boot-starter-web-servicesspring-boot-starter-webservicesjobrunr-spring-boot-3-starterjobrunr-spring-boot-4-starterInstance names also surface in
SourcesFileResultsandRecipeRunStats, where sevenidentical rows are no more useful to someone reading the data table than to someone reading
the recipe list.
After:
Note on the destination coordinate
This follows the two delegates and renders only the old coordinate, which is enough to tell
instances apart. Showing
old→newwould read better still, but it would make this recipeinconsistent with its delegates again, so it belongs in a change to all three rather than here.
Test plan
ChangeDependencyTest.instanceNameasserts the rendered name for thespring-boot-starter-web→spring-boot-starter-webmvccase./gradlew testfails to compileUpgradeTransitiveDependencyVersionagainst therewritesnapshot resolved in myenvironment, in a file this PR does not touch