Kotlin recipe DSL: type the matcher's parameters, not just their count - #8694
Conversation
A `rewrite { x: Double -> Math.abs(x) } to { x -> kotlin.math.abs(x) }`
recipe matched every same-arity `abs` overload. The K2 plugin's
`computeArgsPattern` emitted `List(jvmArgCount) { "*" }`, so the declared
`Double` reached the after-template (`#{any(kotlin.Double)}`) but never the
`MethodMatcher` spec (`java.lang.Math abs(*)`).
That is a correctness bug, not just over-eager matching. `Math.round(double)`
returns `Long` while `Math.round(float)` returns `Int`, so a recipe targeting
the `Double` overload with `to { x -> x.roundToLong() }` also fired on
`fun r(x: Float): Int = Math.round(x)` and produced a `Long` assigned to an
`Int`. #7737 deliberately scoped itself to arity; the varargs branch added by
#7895 already types its fixed prefix, and this brings the non-varargs branch
in line.
Each value parameter is now rendered by `matcherParamType`. Naming a type
outright is unsafe, because `KotlinTypeMapping` only remaps Kotlin builtins to
their JVM FQN for methods declared in Java: the same `String` parameter reads
`java.lang.String` on a Java-declared callee and `kotlin.String` on a
Kotlin-declared one, and recipe authors routinely write Kotlin stand-in classes
for Java targets. Reference builtins therefore emit a package-wildcard token
(`*..String`) that names both spellings, primitives emit the JVM keyword, and
anything whose `JavaType` spelling isn't predictable keeps the old `*` —
type parameters, nullable primitives (`Int?` boxes), value classes, nested
classes, the `kotlin.` package (collections, arrays, `Function1`), and the
lifted extension-receiver slot.
Verified against moderneinc/recipes-kotlin (964 tests, 244 DSL recipes): green.
Its `UseDoubleRoundToLong` / `UseFloatRoundToInt` pair and the three
same-arity `Math.floorMod` recipes were cross-matching before this change.
Downstream check:
|
| tests | failures | |
|---|---|---|
| baseline | 964 | 0 |
| with this change | 964 | 0 |
Green both ways — but that's because each recipe's test only exercises its own overload, so the over-match is invisible to them. Dumping the generated MethodMatcher specs out of the compiled $KtRecipe classes from each run shows what actually changed:
| recipe | baseline | with this change |
|---|---|---|
UseDoubleRoundToLong |
Math round(*) |
Math round(double) |
UseFloatRoundToInt |
Math round(*) |
Math round(float) |
UseIntMod / UseLongMod / UseLongModInt |
Math floorMod(*,*) ×3 |
(int,int) / (long,long) / (long,int) |
UseIntFloorDiv / UseLongFloorDiv |
Math floorDiv(*,*) ×2 |
(int,int) / (long,long) |
UseAppendLineChar / …CharSequence / …WithValue |
StringsKt appendln(*,*) ×3 |
(*,char) / (*,*..CharSequence) / (*,*..String) |
UseCharLowercaseCharForCharacter |
Character toLowerCase(*) |
(char) |
UseLowercaseWithLocale |
StringsKt toLowerCase(*,*) |
(*,java.util.Locale) |
The round pair is the type-unsafe collision this PR is about: two recipes sharing one matcher but with incompatible after-templates (roundToLong returns Long, roundToInt returns Int), so whichever ran first miscompiled the other's call sites.
One collision survives by design. UseAppendLineAny stays appendln(*,*) and still overlaps the three appendln siblings — kotlin.Any and java.lang.Object share no simple name, so no single token matches both parsers' spellings.
Suggested follow-up for that repo: a test that runs the composite over a source holding several overloads at once. The per-overload tests they have now pass whether or not the matchers collide.
The K2 plugin builds its matcher spec with a wildcard per argument, so the type declared on the before lambda reaches the after-template but not the MethodMatcher. UseKotlinMathAbs therefore also rewrites Math.abs on Int and Long receivers, which openrewrite/rewrite#8694 will change once it ships.
* Replace Refaster module with Kotlin DSL in Fundamentals workshop
Module 3 of the Fundamentals of recipe development workshop now teaches
the Kotlin recipe DSL instead of Refaster templates. The DSL fills the
same pattern-shaped niche between declarative YAML and imperative
visitors, but it is compiler-checked Kotlin, needs no annotation
processor, and produces recipes that also rewrite Java, Groovy, and
Scala sources when the pattern names a pure-Java API.
The new module is sourced from the Kotlin recipe DSL documentation and
built around the kotlin-recipe-starter project:
* Exercise 3-1 sets up the starter, reads through the pattern-shaped
recipes it ships with, runs their tests, and shows one recipe
rewriting both Kotlin and Java sources.
* Exercise 3-2 has the reader write a UseKotlinMath recipe set covering
one-, two-, and zero-parameter patterns, compose it with recipes(...),
and drive each addition with tests including a no-change case.
* A closing section introduces the kotlin { visit... } imperative scope
as a bridge into Module 4.
Supporting updates: sidebar entry, a redirect from the old URL, the
module list and learning objectives in both workshop overviews, and the
Refaster references in Modules 1 and 4. Module 1 keeps a pointer to the
Refaster guide so that recipe type stays discoverable.
* Correct CLI version for Kotlin active recipe support
`mod config recipes active set` began accepting Kotlin sources in CLI
4.5.2, not 4.4.2. See the "Accept Kotlin recipe sources in `mod config
recipes active set`" entry under CLI / DX v4.5.2 in cli-releases.md.
* Correct the Kotlin DSL displayName/description constraint
The warning told authors to avoid `+` concatenation, but concatenating
literals is explicitly supported and constant-folds into the generated
recipe. The real constraint is that the value must be written inline as a
compile-time constant; referencing a `val`, parameter, or `const val`
fails the build rather than silently falling back.
Verified against rewrite-kotlin 8.91.1: literal, literal concatenation,
and a text block with `trimIndent()` all compile and reach the generated
`$KtRecipe` class; `val` and `const val` references both fail with
"Recipe `displayName` must be a compile-time constant String".
* Select the composite explicitly in module 3 step 7
Without --recipe, `mod config recipes active set` takes the first
declaration in the file, so readers following step 7 verbatim ran
UseKotlinMathAbs while believing they ran the UseKotlinMath composite
they had just built in step 5. The CLI does report the alternatives, but
that line is easy to scroll past.
Add --recipe to the command and fold the reason into the existing
$KtRecipe warning, which already covers the quoting the flag requires.
* Explain where Code Genome Project credentials come from
Module 3 told readers to set codegenomeUsername/codegenomePassword but
never said where to get them, and the one page in these docs that answers
that was not linked from here. Link it, name what Moderne issues, and show
the gradle.properties entries with the token as the password.
Also drop the claim that Maven Central "may not include the DSL". Central
currently serves rewrite-kotlin 8.90.4, which ships the K2 recipe compiler
plugin, so a reader without credentials is a release or two behind rather
than blocked.
* Answer two questions the module leaves a Java reader with
Name the symptom of a missing compiler plugin. Removing the
kotlinCompilerPluginClasspath entry fails every test inside Jackson with
nothing pointing at the plugin, so quote the message that readers will
actually search for.
Close the pure-Java loop opened in exercise 3-1. Exercise 3-2 targets
java.lang.Math, which is exactly the condition that let UseIsWhitespace
reach Java sources, so a reader who took that lesson has reason to fear
step 7 will mangle Java repositories. Verified the composite leaves Java
sources unchanged: the kotlin.math replacement is not valid Java, so the
second condition fails and nothing is rewritten.
* List module 3's prerequisites, which differ from module 1's
Module 3 is the only module that builds a different starter project, but
it never said so up front. Its requirements are not the ones module 1
established: build.gradle.kts pins a Java 21 toolchain, so a JDK 21 must
be installed even when the default JDK is newer, and kotlin-recipe-starter
ships no pom.xml, so module 1's and 2's Maven commands do not apply.
* Restore the Maven Central warning dropped in fdd7aae
fdd7aae softened this to "lags by a release or two" on the strength of a
point-in-time check: Central happened to serve a rewrite-kotlin that
included the DSL. That read a diverging trend as a steady state. OpenRewrite
is moving off Maven Central, so the gap widens from here - Central is on
8.90.4 (Aug 24) while the Code Genome Project is on 8.91.1 with
8.92.0-SNAPSHOT.
Restore the substance of the original warning: the fallback is silent, the
build still succeeds, and the version it resolves can predate the DSL.
* Correct the claim that a pattern's parameter type narrows the match
The K2 plugin builds its matcher spec with a wildcard per argument, so
the type declared on the before lambda reaches the after-template but
not the MethodMatcher. UseKotlinMathAbs therefore also rewrites
Math.abs on Int and Long receivers, which openrewrite/rewrite#8694 will
change once it ships.
---------
Co-authored-by: Tim te Beek <tim@moderne.io>
Co-authored-by: Sam Snyder <sam@moderne.io>
MethodMatcherspec withList(jvmArgCount) { "*" }, so arewrite { x: Double -> Math.abs(x) }recipe's declaredDoublereached the after-template but not the matcher (java.lang.Math abs(*)) and every same-arity overload matched — withMath.round, whose(double)overload returnsLongand(float)returnsInt, that turnedfun r(x: Float): Int = Math.round(x)into aLongassigned to anInt. Kotlin recipe DSL: emit precise arg-count matcher patterns instead of (..) #7737 scoped itself to arity and rewrite-kotlin: variadic-by-default matching in the recipe DSL #7895 already types the varargs branch's fixed prefix, so this just brings the non-varargs branch in line.Naming types outright is unsafe because
KotlinTypeMappingonly remaps Kotlin builtins to their JVM FQN for Java-declared methods — the sameStringparameter readsjava.lang.Stringon a Java-declared callee andkotlin.Stringon a Kotlin-declared one, and authors routinely write Kotlin stand-in classes for Java targets — so reference builtins emit a package-wildcard token (*..String) that names both spellings, primitives emit the JVM keyword, and anything unpredictable (type parameters, nullable primitives, value classes, nested classes, thekotlin.package, the lifted extension-receiver slot) keeps the old*.Four new tests in
RecipePluginRewriteTestcover the narrowed match, an untargeted sibling overload left alone, theMath.roundmiscompile, a Kotlin-declared callee, and the kotlin-recipe-starter recipes still firing; all four fail without the change, and one existing spec assertion moves fromsubstring(*, *, *)tosubstring(*, int, int).Verified downstream against
moderneinc/recipes-kotlin(964 tests, 244 DSL recipes): green, and itsUseDoubleRoundToLong/UseFloatRoundToIntpair plus three same-arityMath.floorModrecipes were silently cross-matching before this.