MissingOverrideAnnotation: NPE in classDecl.getBody().getMarkers() with Kotlin files

[vlsi]

How are you running OpenRewrite?

See apache/jmeter#6217, ./gradlew :src:jorphan:rewriteRun

What is the smallest, simplest way to reproduce the problem?

No idea

What did you expect to see?

MissingOverrideAnnotation should either skip Kotlin files or it should add override respectively.

What did you see instead?

The issue might be caused by withBody(null) (see https://github.com/openrewrite/rewrite/blob/fb7dbd6c95ef5492301dcb941e7f8eefd223621e/rewrite-java/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java#L226 ), however, I'm not sure what triggers it.

The problematic class is https://github.com/apache/jmeter/blob/09c3f810dda7efaf1510ac062454d978d90a0741/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/CardLayoutWithSizeOfCurrentVisibleElement.kt#L31

org.openrewrite.internal.RecipeRunException: java.lang.NullPointerException: Cannot invoke "org.openrewrite.java.tree.J$Block.getStatements()" because the return value of "org.openrewrite.java.tree.J$ClassDeclaration.getBody()" is null
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:329)
        at org.openrewrite.kotlin.internal.KotlinPrinter$KotlinJavaPrinter.visit(KotlinPrinter.java:495)
        at org.openrewrite.kotlin.internal.KotlinPrinter.visit(KotlinPrinter.java:56)
        at org.openrewrite.kotlin.internal.KotlinPrinter.visit(KotlinPrinter.java:40)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:184)
        at org.openrewrite.Tree.print(Tree.java:94)
        at org.openrewrite.Tree.print(Tree.java:90)
        at org.openrewrite.Tree.printTrimmed(Tree.java:109)
        at org.openrewrite.java.internal.template.AnnotationTemplateGenerator.classDeclaration(AnnotationTemplateGenerator.java:227)
        at org.openrewrite.java.internal.template.AnnotationTemplateGenerator.template(AnnotationTemplateGenerator.java:141)
        at org.openrewrite.java.internal.template.AnnotationTemplateGenerator.lambda$template$0(AnnotationTemplateGenerator.java:73)
        at io.micrometer.core.instrument.composite.CompositeTimer.record(CompositeTimer.java:65)
        at org.openrewrite.java.internal.template.AnnotationTemplateGenerator.template(AnnotationTemplateGenerator.java:69)
        at org.openrewrite.java.internal.template.JavaTemplateParser.lambda$parseAnnotations$10(JavaTemplateParser.java:203)
        at org.openrewrite.java.internal.template.JavaTemplateParser.cache(JavaTemplateParser.java:295)
        at org.openrewrite.java.internal.template.JavaTemplateParser.parseAnnotations(JavaTemplateParser.java:202)
        at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitMethodDeclaration(JavaTemplateJavaExtension.java:281)
        at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitMethodDeclaration(JavaTemplateJavaExtension.java:56)
        at org.openrewrite.java.tree.J$MethodDeclaration.acceptJava(J.java:3670)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:184)
        at org.openrewrite.java.JavaTemplate.apply(JavaTemplate.java:67)
        at org.openrewrite.java.JavaTemplate.apply(JavaTemplate.java:108)
        at org.openrewrite.staticanalysis.MissingOverrideAnnotation$MissingOverrideAnnotationVisitor.visitMethodDeclaration(MissingOverrideAnnotation.java:86)
        at org.openrewrite.staticanalysis.MissingOverrideAnnotation$MissingOverrideAnnotationVisitor.visitMethodDeclaration(MissingOverrideAnnotation.java:70)
        at org.openrewrite.staticanalysis.MissingOverrideAnnotation_MissingOverrideAnnotationVisitor_KotlinVisitor.visitMethodDeclaration(MissingOverrideAnnotation_MissingOverrideAnnotationVisitor_KotlinVisitor.zig:123)
        at org.openrewrite.java.tree.J$MethodDeclaration.acceptJava(J.java:3670)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:361)
        at org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:1375)
        at org.openrewrite.java.JavaVisitor.lambda$visitBlock$4(JavaVisitor.java:401)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:176)
        at org.openrewrite.java.JavaVisitor.visitBlock(JavaVisitor.java:400)
        at org.openrewrite.java.tree.J$Block.acceptJava(J.java:838)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:361)
        at org.openrewrite.java.JavaVisitor.visitClassDeclaration(JavaVisitor.java:488)
        at org.openrewrite.java.tree.J$ClassDeclaration.acceptJava(J.java:1290)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:361)
        at org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:1375)
        at org.openrewrite.java.JavaVisitor.lambda$visitBlock$4(JavaVisitor.java:401)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:176)
        at org.openrewrite.java.JavaVisitor.visitBlock(JavaVisitor.java:400)
        at org.openrewrite.java.tree.J$Block.acceptJava(J.java:838)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:361)
        at org.openrewrite.java.JavaVisitor.visitClassDeclaration(JavaVisitor.java:488)
        at org.openrewrite.java.tree.J$ClassDeclaration.acceptJava(J.java:1290)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:361)
        at org.openrewrite.kotlin.KotlinVisitor.lambda$visitCompilationUnit$2(KotlinVisitor.java:57)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:176)
        at org.openrewrite.kotlin.KotlinVisitor.visitCompilationUnit(KotlinVisitor.java:56)
        at org.openrewrite.kotlin.tree.K$CompilationUnit.acceptKotlin(K.java:246)
        at org.openrewrite.kotlin.tree.K.accept(K.java:60)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:184)
        at org.openrewrite.scheduling.RecipeRunCycle.lambda$editSources$5(RecipeRunCycle.java:164)
        at io.micrometer.core.instrument.AbstractTimer.recordCallable(AbstractTimer.java:147)
        at org.openrewrite.table.RecipeRunStats.recordEdit(RecipeRunStats.java:68)
        at org.openrewrite.scheduling.RecipeRunCycle.lambda$editSources$6(RecipeRunCycle.java:161)
        at org.openrewrite.scheduling.RecipeStack.reduce(RecipeStack.java:57)
        at org.openrewrite.scheduling.RecipeRunCycle.lambda$editSources$7(RecipeRunCycle.java:134)
        at org.openrewrite.internal.InMemoryLargeSourceSet.lambda$edit$0(InMemoryLargeSourceSet.java:62)
        at org.openrewrite.internal.ListUtils.map(ListUtils.java:176)
        at org.openrewrite.internal.InMemoryLargeSourceSet.edit(InMemoryLargeSourceSet.java:61)
        at org.openrewrite.scheduling.RecipeRunCycle.editSources(RecipeRunCycle.java:133)
        at org.openrewrite.RecipeScheduler.runRecipeCycles(RecipeScheduler.java:86)
        at org.openrewrite.RecipeScheduler.scheduleRun(RecipeScheduler.java:41)
        at org.openrewrite.Recipe.run(Recipe.java:344)
        at org.openrewrite.Recipe.run(Recipe.java:340)
        at org.openrewrite.Recipe.run(Recipe.java:336)
        at org.apache.jmeter.buildtools.openrewrite.OpenRewriteWork.listResults(OpenRewriteWork.kt:291)
        at org.apache.jmeter.buildtools.openrewrite.OpenRewriteWork.execute(OpenRewriteWork.kt:162)
        at app//org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
        at app//org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:54)
        at app//org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:48)
        at app//org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:100)
        at app//org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:48)
        at app//org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:49)
        at app//org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:30)
        at app//org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:96)
        at app//org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:65)
        at app//org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:138)
        at app//org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
        at app//org.gradle.process.internal.worker.request.WorkerAction.lambda$run$0(WorkerAction.java:135)
        at app//org.gradle.internal.operations.CurrentBuildOperationRef.with(CurrentBuildOperationRef.java:80)
        at app//org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:127)
        at java.base@17.0.9/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base@17.0.9/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base@17.0.9/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base@17.0.9/java.lang.reflect.Method.invoke(Method.java:568)
        at app//org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
        at app//org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
        at app//org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
        at app//org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
        at app//org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
        at app//org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
        at app//org.gradle.internal.concurrent.AbstractManagedExecutor$1.run(AbstractManagedExecutor.java:47)
        at java.base@17.0.9/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base@17.0.9/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base@17.0.9/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.NullPointerException: Cannot invoke "org.openrewrite.java.tree.J$Block.getStatements()" because the return value of "org.openrewrite.java.tree.J$ClassDeclaration.getBody()" is null
        at org.openrewrite.kotlin.internal.KotlinPrinter$KotlinJavaPrinter.visitClassDeclaration0(KotlinPrinter.java:675)
        at org.openrewrite.kotlin.internal.KotlinPrinter$KotlinJavaPrinter.visitClassDeclaration(KotlinPrinter.java:646)
        at org.openrewrite.kotlin.internal.KotlinPrinter$KotlinJavaPrinter.visitClassDeclaration(KotlinPrinter.java:482)
        at org.openrewrite.java.tree.J$ClassDeclaration.acceptJava(J.java:1290)
        at org.openrewrite.java.tree.J.accept(J.java:59)
        at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
        ... 108 more

[knutwannheden]

Yes, what you have identified here is indeed problematic. I've made the code more robust there, by not passing in a null to withBody().

The MissingOverrideAnnotation recipe would currently not work correctly anyway for Kotlin, as it adds an @Override annotation. For Kotlin it would have to add the override keyword. I will for now just change it, so it doesn't modify Kotlin sources, as I think the Kotlin compiler would complain anyway, if that is missing.

[knutwannheden]

@vlsi Thanks for reporting!

[vlsi]

@knutwannheden , do you think it makes sense to fix the other two withBody(null) calls?

https://github.com/openrewrite/rewrite/blob/c625f30ca32f8af7a196dd3b00b720919db22dcf/rewrite-java/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java#L165
https://github.com/openrewrite/rewrite/blob/c625f30ca32f8af7a196dd3b00b720919db22dcf/rewrite-java/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java#L198

[knutwannheden]

Note: We try to explicitly declare all parameters as non-null (using our own @NonNullApi annotation using JSR-305 meta annotations). Unfortunately this doesn't work for methods generated by Lombok (e.g. with methods). We could of course make that explicit, but that would also be ugly:

        @With(onParam_ = @NonNull)
        @Getter
        Block body;

[knutwannheden]

@knutwannheden , do you think it makes sense to fix the other two withBody(null) calls?

Yes, that would definitely make sense. I will look into it.

[vlsi]

We try to explicitly declare all parameters as non-null (using our own @NonNullApi annotation using JSR-305 ..

That is strange. I guess you have @NonNullApi at the package level, so it should apply to all the methods within the package. It would be weird to annotate every method with @NonNull.

For reference, the decompiled code is

        @NonNull
        public @NonNull ClassDeclaration withBody(final Block body) {
            return this.body == body ? this : new ClassDeclaration(this.padding, this.annotations, this.id, this.prefix, this.markers, this.leadingAnnotations, this.modifiers, this.kind, this.name, this.typeParameters, this.primaryConstructor, this.extendings, this.implementings, this.permitting, body, this.type);
        }

Kotlin already rejects withBody(null):

lateinit var cu: J.ClassDeclaration
cu.withBody(null) // Null can not be a value of a non-null type J.Block

So if you miss null can't be passed to withBody(...) inspection, it might be an issue with the IDE.

[knutwannheden]

Interesting. I would have to look at the byte code. But if I declare the @With annotation as shown or add an explicit @NonNull annotation to the field, it works. Possibly this is due to projectlombok/lombok#2654

[knutwannheden]

I can at least confirm that in my IDEA (2023.3.2) it behaves this way. For Java calls our @NonNullApi at the package (or class level) has no effect on the parameter of the Lombok generated with methods, while for calls from Kotlin it does appear to make a difference.