Support Kotlin sources in AssertThrowsOnLastStatement#1048
Conversation
|
@timtebeek , I attempted to revive apache/jmeter#6217 (which is mixed Java+Kotlin codebase), and it turns out several recipes crash on JMeter codebase. I might try update the other failing recipes in a similar way, however, I would like to know in advance if you are willing to accept those types of contributions. Looks like currently only one recipe supports Kotlin: https://github.com/search?q=repo%3Aopenrewrite%2Frewrite-testing-frameworks%20KotlinTemplate.builder&type=code while there are 100+ usages of The effective prompt was behind the lines of "reproduce the issue, support Kotlin, and add tests exploring weird corner-cases for the recipe" |
a265a58 to
6092202
Compare
|
@timtebeek , I see you removed several tests. Sure it is your call, however they were not duplicate. For example, Every other test lands in the So without this test, removing the special-casing (or the type-based fallback) silently names the extracted variable |
`AssertThrowsOnLastStatement` is a `JavaTemplate`-based recipe, and Java visitors also run over Kotlin because `K.CompilationUnit` implements `JavaSourceFile`. On a Kotlin file the recipe emitted Java syntax for the extracted variable declaration, producing invalid code such as `baz: String = baz()`, or crashing with "generated 0 statements". This surfaced in mixed Java/Kotlin projects, where the recipe runs as part of the `junit-jupiter` migration. The statement-hoisting logic already operates on shared `J` nodes and needs no change. Only the extracted variable declaration is language-specific, so it now branches: Kotlin emits `val name = expr` and relies on type inference, skipping the Java type-name and import plumbing. The Kotlin template is anchored on the enclosing method body as an added statement, which forces a statement context; anchoring on the lambda's trailing call would wrap it as an expression (`var o = ...`) and fail to parse. Groovy and other languages remain excluded via `isAcceptable`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6092202 to
e93ea56
Compare
|
Thanks a lot for the help here; I'd originally trimmed our contribution a bit as Claude loves to Definitely open to accept more contributions that expand support for Kotlin in this set of recipes; you'll find a mixture of things that work out of the box, whilst other areas might indeed need small changes. Wherever possible we try to solve these generically, even when using JavaTemplate for other languages. So definitely let us know if you spot repeated patterns that we'd rather pull up rather than fix in individual recipes. |
Two follow-ups to the in-house OpenRewrite runner.
disabledRecipes lets a single recipe be switched off by name, e.g.
openrewrite {
disabledRecipes.add("org.openrewrite.java.testing.junit5.AssertThrowsOnLastStatement")
}
The compiled composites (JUnit5BestPractices and friends) expose an immutable
recipeList, so the runner wraps the recipe tree in a FilteringRecipe that hides
the disabled names instead of mutating the list. This removes the need to fork a
recipe module or drop a whole composite; the JUnit 5 recipes are now enabled for
every test module and only AssertThrowsOnLastStatement is switched off, pending
openrewrite/rewrite-testing-frameworks#1048
rewriteDiagnose runs each active leaf recipe on its own and reports whether it
fails, would change files, or does nothing, so the problematic recipes can be
found without reading a full run. On jorphan: 183 leaf recipes, 0 fail in Java
mode, 21 would make changes. Note that a recipe which only fails as part of an
ordered composite (AssertThrowsOnLastStatement on Kotlin) passes in isolation, so
the per-leaf report is a lower bound; the composite run surfaces the rest as
non-fatal "Error while rewriting" warnings thanks to the lenient ExecutionContext.
-PopenrewriteKotlin=true flips Kotlin processing on from the command line for
experimenting with rewrite-kotlin.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Why
AssertThrowsOnLastStatementis aJavaTemplate-based recipe, and Java visitors also run over Kotlin becauseK.CompilationUnitimplementsJavaSourceFile. On a Kotlin file the recipe emitted Java syntax for the extracted variable declaration — producing invalid code such asbaz: String = baz()— or failed outright withExpected a template that would generate exactly one statement to replace one statement, but generated 0. This shows up in mixed Java/Kotlin projects, where the recipe runs as part of thejunit-jupitermigration rather than in isolation.What
The statement-hoisting logic already works on shared
Jnodes, so it needs no change. Only the extracted variable declaration is language-specific, so it now branches:val name = exprand relies on type inference, skipping the Java type-name and import plumbing.var o = ...) and fail to parse, because Kotlin models a bareassertThrows(...)call-statement as an expression.isAcceptablekeeps Groovy and other languages excluded, sinceJavaTemplatewould still corrupt them.How to verify
./gradlew test --tests 'org.openrewrite.java.testing.junit5.AssertThrowsOnLastStatementTest'The Kotlin behaviour is pinned by new tests covering argument extraction as an inferred
val, leading-statement hoisting, unique names for multiple extracted arguments, collision with an existing variable, a preserved message argument, a single-statement lambda, field-access arguments left in place, and the reifiedassertThrows<T> { }form left untouched. Java edge cases (nestedassertThrows, factory-method naming) are covered too.🤖 Generated with Claude Code