Kotlin recipe DSL: emit precise arg-count matcher patterns instead of (..)#7737
Merged
Conversation
… (..)
Recipe `xs.filter(p).any()` -> `xs.any(p)` was silently dropping code when
the source had `xs.filter(p1).any { p2 }` — the with-predicate `any`
overload. The K2 plugin's computed matcher spec was `kotlin.collections.CollectionsKt any(..)`,
where `(..)` matches any arity, so both `any()` and `any(predicate)` matched.
The substitution-source CSV references only the inner's arg slot, so the
runtime helper happily produced `xs.any(p1)`, throwing away the outer
`.any { p2 }` body the author wrote. Real-world surface: a `~15-line`
.any { bounds -> Rectangle(...).intersects(...) } in YiiGuxing/TranslationPlugin
collapsed to a one-line `.any { windowLocation == ... }`.
Tighten `computeMatcherSpec` to emit the precise JVM arg count instead of
`(..)`:
- 0 args -> `()`
- 1 arg -> `(*)`
- 2 args -> `(*,*)`
- etc.
Arg count is computed against the JVM-resolved signature, differing by
call shape:
- Member call (dispatch receiver, e.g. `String.lowercase()`): just the
source-level value args.
- Extension or top-level facade call (e.g. `Iterable<T>.any(p)`): the
receiver is lifted to the first arg of the static facade method, so
the JVM arg list is (extension) receiver + value args.
Overloaded names like `any` / `none` / `count` no longer cross-match
across overloads.
Regression test pins the user-reported case: the
`xs.filter(p).any() -> xs.any(p)` recipe leaves
`xs.filter { ... }.any { ... }` untouched instead of clobbering it.
Surfaced by recipes-kotlin Performance recipes against real corpora.
5768794 to
1cd4be0
Compare
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.
Summary
Recipe
xs.filter(p).any()→xs.any(p)was silently dropping code when the source hadxs.filter(p1).any { p2 }— the with-predicateanyoverload. The K2 plugin's computed matcher spec waskotlin.collections.CollectionsKt any(..), where(..)matches any arity, so bothany()andany(predicate)matched. The substitution-source CSV references only the inner's arg slot, so the runtime helper happily producedxs.any(p1), throwing away the outer.any { p2 }body the author wrote.Real-world surface: a ~15-line
.any { bounds -> Rectangle(...).intersects(...) }inYiiGuxing/TranslationPlugincollapsed to a one-line.any { windowLocation == ... }. The.any { }body was the actual intersect check; it just vanished.Tighten
computeMatcherSpecto emit the precise JVM arg count instead of(..):()(*)(*,*)Arg count is computed against the JVM-resolved signature, differing by call shape:
String.lowercase()): just the source-level value args.Iterable<T>.any(p)): the receiver is lifted to the first arg of the static facade method, so the JVM arg list is (extension) receiver + value args.Overloaded names like
any/none/countno longer cross-match across overloads.Test plan
recipe targeting no-arg overload does not match the with-predicate overloadtest inRecipePluginRewriteTestpins the user-reported case: thexs.filter(p).any() → xs.any(p)recipe leavesxs.filter { ... }.any { ... }untouched instead of clobbering it../gradlew :rewrite-kotlin:testgreen (no regression in the existing single-overload recipes).