Leave Kotlin and Groovy sources to ListFirstAndLast - #1222
Merged
Conversation
Kotlin models `xs[0]` as a `get` invocation carrying an index-access marker, so the matcher fires and emptying the argument list prints `xs[]`. Groovy fails more quietly: `collection.get(0)` loses its parentheses and becomes a property read that only fails at runtime. Neither language can use `getFirst()` anyway — it is not declared on their mapped list types — so the file checkers already used by the neighbouring collection recipes are the right guard. The Lombok annotation processor test no longer needs to be marked as expected to fail now that the reactor handling it depends on has been corrected.
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
1 participant
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.
ListFirstAndLastrewriteslist.get(0)tolist.getFirst(). Kotlin and Groovy sources reach it through the sharedJmodel, and in both the rewrite produces something that does not work.Kotlin
xs[0]is modelled as aJ.MethodInvocationnamed<get>carrying anorg.openrewrite.kotlin.marker.IndexedAccessmarker, with methodTypejava.util.List.get(int).MethodMatcherdispatches on the method type alone and cannot see the syntactic form, so the recipe fires, renames it, and empties the argument list. The marker then faithfully prints:Groovy
collection.get(0)loses its parentheses when the argument list empties:collection.getFirstThat parses, and Groovy compiles it as a property read, so it fails at runtime with
MissingPropertyExceptionrather than at build time. Groovy's subscript formcollection[0]is unaffected — it resolves togetAt, notget.Fix
Preconditions.not(new KotlinFileChecker<>())andGroovyFileChecker, which is what the sevenMigrateCollections*recipes in this same package already do. No new mechanism, andrewrite-kotlinandrewrite-groovyare already dependencies.Guarding rather than teaching the recipe to emit valid Kotlin is the right call on the merits, not just for convenience:
getFirst()is not declared onkotlin.collections.List, so there is no correct output for this recipe to produce there. Kotlin's own idiom isfirst(), which would be a separate Kotlin-native recipe.The sibling
SequencedCollectionrecipes were checked and left alone —IteratorNextandStreamFindFirstgate onjava.util.SequencedCollectionappearing in the receiver's type hierarchy, which Kotlin's mapped collection types do not carry, so a guard there would be untestable dead code.Tests
Three cases added to the existing
NoChangenested class covering Kotlin index access, Kotlinget(0), and Groovyget(0). Each fails without the guard, producingcollection[],collection.getFirst()andcollection.getFirstrespectively. Fullmigrate.utilandmigrate.langpackages: 669 tests, 0 failures.Found by compiling the output of
UpgradeToJava25across a set of open source repositories.