Detect and report inner classes capturing script instance

compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/DiagnosticReporter.kt

@@ -68,6 +68,16 @@ open class KtDiagnosticReporterWithContext(
             sourceElement?.let { report(factory.on(it, a, positioningStrategy), this) }
         }
 
+        @OptIn(InternalDiagnosticFactoryMethod::class)
+        fun <A1 : Any, A2: Any> report(
+            factory: KtDiagnosticFactory2<A1, A2>,
+            a1: A1,
+            a2: A2,
+            positioningStrategy: AbstractSourceElementPositioningStrategy? = null
+        ) {
+            sourceElement?.let { report(factory.on(it, a1, a2, positioningStrategy), this) }
+        }
+
         override fun equals(other: Any?): Boolean {
             if (this === other) return true
             if (other !is DiagnosticContextImpl) return false

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/JvmBackendErrors.kt

@@ -11,6 +11,7 @@ import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactoryToRendererMap
 import org.jetbrains.kotlin.diagnostics.SourceElementPositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT
 import org.jetbrains.kotlin.diagnostics.error0
 import org.jetbrains.kotlin.diagnostics.error1
+import org.jetbrains.kotlin.diagnostics.error2
 import org.jetbrains.kotlin.diagnostics.rendering.*
 import org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers.STRING
 import org.jetbrains.kotlin.resolve.MemberComparator
@@ -27,6 +28,7 @@ object JvmBackendErrors {
 
     val SUSPENSION_POINT_INSIDE_MONITOR by error1<PsiElement, String>()
 
+    val SCRIPT_CAPTURING_NESTED_CLASS by error2<PsiElement, String, String>()
     val SCRIPT_CAPTURING_OBJECT by error1<PsiElement, String>()
     val SCRIPT_CAPTURING_INTERFACE by error1<PsiElement, String>()
     val SCRIPT_CAPTURING_ENUM by error1<PsiElement, String>()
@@ -66,6 +68,7 @@ object KtDefaultJvmErrorMessages : BaseDiagnosticRendererFactory() {
         map.put(JvmBackendErrors.TYPEOF_NON_REIFIED_TYPE_PARAMETER_WITH_RECURSIVE_BOUND, "Non-reified type parameters with recursive bounds are not supported yet: {0}", STRING)
         map.put(JvmBackendErrors.SUSPENSION_POINT_INSIDE_MONITOR, "A suspension point at {0} is inside a critical section", STRING)
 
+        map.put(JvmBackendErrors.SCRIPT_CAPTURING_NESTED_CLASS, "Nested class {0} captures the script class instance. Try to use explicit inner modifier for both nested {0} and outer {1}", STRING, STRING)
         map.put(JvmBackendErrors.SCRIPT_CAPTURING_OBJECT, "Object {0} captures the script class instance. Try to use class or anonymous object instead", STRING)
         map.put(JvmBackendErrors.SCRIPT_CAPTURING_INTERFACE, "Interface {0} captures the script class instance. Try to use class instead", STRING)
         map.put(JvmBackendErrors.SCRIPT_CAPTURING_ENUM, "Enum class {0} captures the script class instance. Try to use class or anonymous object instead", STRING)

compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ScriptLowering.kt

@@ -36,6 +36,8 @@ import org.jetbrains.kotlin.ir.symbols.impl.IrValueParameterSymbolImpl
 import org.jetbrains.kotlin.ir.types.*
 import org.jetbrains.kotlin.ir.util.*
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformer
+import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid
+import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.name.SpecialNames
@@ -98,24 +100,45 @@ private class ScriptsToClassesLowering(val context: JvmBackendContext, val inner
     private fun collectCapturingClasses(irScript: IrScript): Set<IrClassImpl> {
         val annotator = ClosureAnnotator(irScript, irScript)
         val capturingClasses = mutableSetOf<IrClassImpl>()
-        for (clazz in irScript.statements) {
-            if (clazz is IrClassImpl && !clazz.isInner ) {
-                val closure = annotator.getClassClosure(clazz)
-                if (closure.capturedValues.singleOrNull()?.owner?.type == irScript.thisReceiver.type) {
-                    fun reportError(factory: KtDiagnosticFactory1<String>, name: Name? = null) {
-                        context.ktDiagnosticReporter.at(clazz).report(factory, (name ?: clazz.name).asString())
-                    }
-                    when {
-                        clazz.isInterface -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_INTERFACE)
-                        clazz.isEnumClass -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_ENUM)
-                        clazz.isEnumEntry -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_ENUM_ENTRY)
-                        // TODO: ClosureAnnotator is not catching companion's closures, so the following reporting never happens. Make it work or drop
-                        clazz.isCompanion -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_OBJECT, SpecialNames.DEFAULT_NAME_FOR_COMPANION_OBJECT)
-                        clazz.kind.isSingleton -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_OBJECT)
+        val collector = object : IrElementVisitorVoid {
+            override fun visitElement(element: IrElement) {
+                element.acceptChildrenVoid(this)
+            }
 
-                        clazz.isClass -> capturingClasses.add(clazz)
+            override fun visitClass(declaration: IrClass) {
+                if (declaration is IrClassImpl && !declaration.isInner) {
+                    val closure = annotator.getClassClosure(declaration)
+                    if (closure.capturedValues.singleOrNull()?.owner?.type == irScript.thisReceiver.type) {
+                        fun reportError(factory: KtDiagnosticFactory1<String>, name: Name? = null) {
+                            context.ktDiagnosticReporter.at(declaration).report(factory, (name ?: declaration.name).asString())
+                        }
+                        when {
+                            declaration.isInterface -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_INTERFACE)
+                            declaration.isEnumClass -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_ENUM)
+                            declaration.isEnumEntry -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_ENUM_ENTRY)
+                            // TODO: ClosureAnnotator is not catching companion's closures, so the following reporting never happens. Make it work or drop
+                            declaration.isCompanion -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_OBJECT, SpecialNames.DEFAULT_NAME_FOR_COMPANION_OBJECT)
+                            declaration.kind.isSingleton -> reportError(JvmBackendErrors.SCRIPT_CAPTURING_OBJECT)
+
+                            declaration.isClass ->
+                                if (declaration.parent != irScript) {
+                                    context.ktDiagnosticReporter.at(declaration).report(
+                                        JvmBackendErrors.SCRIPT_CAPTURING_NESTED_CLASS,
+                                        declaration.name.asString(),
+                                        ((declaration.parent as? IrDeclarationWithName)?.name ?: SpecialNames.NO_NAME_PROVIDED).asString()
+                                    )
+                                } else {
+                                    capturingClasses.add(declaration)
+                                }
+                        }
                     }
                 }
+                super.visitClass(declaration)
+            }
+        }
+        for (statement in irScript.statements) {
+            if (statement is IrClassImpl) {
+                collector.visitClass(statement)
             }
         }
         return capturingClasses

compiler/testData/codegen/script/scriptInstanceCapturing/classCapturesProperty.kts

@@ -5,7 +5,7 @@
 // KT-19423
 val used = "abc"
 class User {
-    val property = used // error: Expression is inaccessible from a nested class
+    val property = used
 }
 
 val rv = User().property

compiler/testData/codegen/script/scriptInstanceCapturing/innerClassesHierarchyCaptureProperty.kts

@@ -0,0 +1,15 @@
+// IGNORE_BACKEND: JS, JS_IR, JS_IR_ES6, NATIVE, WASM
+
+// expected: rv: abcabc
+
+// KT-19423 variation
+val used = "abc"
+
+inner class Outer {
+    val middle = used
+    inner class User {
+        val property = used
+    }
+}
+
+val rv = Outer().User().property + Outer().middle

compiler/testData/codegen/script/scriptInstanceCapturing/nestedAndOuterClassesCaptureProperty.kts

@@ -0,0 +1,16 @@
+// IGNORE_BACKEND: JS, JS_IR, JS_IR_ES6, NATIVE, WASM
+// IGNORE_BACKEND: JVM, JVM_IR
+
+// expected: rv: abcabc
+
+// KT-19423 variation
+val used = "abc"
+
+class Outer {
+    val middle = used
+    class User {
+        val property = used
+    }
+}
+
+val rv = Outer.User().property + Outer().middle

compiler/testData/codegen/script/scriptInstanceCapturing/nestedClassCapturesProperty.kts

@@ -0,0 +1,15 @@
+// IGNORE_BACKEND: JS, JS_IR, JS_IR_ES6, NATIVE, WASM
+// IGNORE_BACKEND: JVM, JVM_IR
+
+// expected: rv: abc
+
+// KT-19423 variation
+val used = "abc"
+
+class Outer {
+    class User {
+        val property = used
+    }
+}
+
+val rv = Outer.User().property

compiler/testData/codegen/script/scriptInstanceCapturing/nestedInnerClassCapturesProperty.kts

@@ -0,0 +1,15 @@
+// IGNORE_BACKEND: JS, JS_IR, JS_IR_ES6, NATIVE, WASM
+
+// expected: rv: abc!
+
+// KT-19423 variation
+val used = "abc"
+
+class Outer {
+    val bang = "!"
+    inner class User {
+        val property = used + bang
+    }
+}
+
+val rv = Outer().User().property

compiler/testData/codegen/script/scriptInstanceCapturing/nestedToObjectClassCapturesProperty.kts

@@ -0,0 +1,15 @@
+// IGNORE_BACKEND: JS, JS_IR, JS_IR_ES6, NATIVE, WASM
+// IGNORE_BACKEND: JVM, JVM_IR
+
+// expected: rv: abc
+
+// KT-19423 variation
+val used = "abc"
+
+object Outer {
+    class User {
+        val property = used
+    }
+}
+
+val rv = Outer.User().property

compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedAndOuterClassesCaptureProperty.kts

@@ -0,0 +1,12 @@
+// !RENDER_DIAGNOSTICS_FULL_TEXT
+// TARGET_BACKEND: JVM_IR
+
+// KT-19423 variation
+val used = "abc"
+
+class Outer {
+    val middle = used
+    <!SCRIPT_CAPTURING_NESTED_CLASS!>class User {
+        val property = used
+    }<!>
+}

compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedAndOuterClassesCaptureProperty.txt

@@ -0,0 +1,26 @@
+package
+
+public final class NestedAndOuterClassesCaptureProperty : kotlin.script.templates.standard.ScriptTemplateWithArgs {
+    public constructor NestedAndOuterClassesCaptureProperty(/*0*/ args: kotlin.Array<kotlin.String>)
+    public final override /*1*/ /*fake_override*/ val args: kotlin.Array<kotlin.String>
+    public final val used: kotlin.String = "abc"
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+
+    public final class Outer {
+        public constructor Outer()
+        public final val middle: kotlin.String = "abc"
+        public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+        public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+        public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+
+        public final class User {
+            public constructor User()
+            public final val property: kotlin.String = "abc"
+            public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+            public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+            public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+        }
+    }
+}

compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedClassCapturesProperty.kts

@@ -0,0 +1,11 @@
+// !RENDER_DIAGNOSTICS_FULL_TEXT
+// TARGET_BACKEND: JVM_IR
+
+// KT-19423 variation
+val used = "abc"
+
+class Outer {
+    <!SCRIPT_CAPTURING_NESTED_CLASS!>class User {
+        val property = used
+    }<!>
+}

compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedClassCapturesProperty.txt

@@ -0,0 +1,25 @@
+package
+
+public final class NestedClassCapturesProperty : kotlin.script.templates.standard.ScriptTemplateWithArgs {
+    public constructor NestedClassCapturesProperty(/*0*/ args: kotlin.Array<kotlin.String>)
+    public final override /*1*/ /*fake_override*/ val args: kotlin.Array<kotlin.String>
+    public final val used: kotlin.String = "abc"
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+
+    public final class Outer {
+        public constructor Outer()
+        public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+        public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+        public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+
+        public final class User {
+            public constructor User()
+            public final val property: kotlin.String = "abc"
+            public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+            public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+            public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+        }
+    }
+}

compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedToObjectClassCapturesProperty.kts

@@ -0,0 +1,11 @@
+// !RENDER_DIAGNOSTICS_FULL_TEXT
+// TARGET_BACKEND: JVM_IR
+
+// KT-19423 variation
+val used = "abc"
+
+object Outer {
+    <!SCRIPT_CAPTURING_NESTED_CLASS!>class User {
+        val property = used
+    }<!>
+}

compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedToObjectClassCapturesProperty.txt

@@ -0,0 +1,25 @@
+package
+
+public final class NestedToObjectClassCapturesProperty : kotlin.script.templates.standard.ScriptTemplateWithArgs {
+    public constructor NestedToObjectClassCapturesProperty(/*0*/ args: kotlin.Array<kotlin.String>)
+    public final override /*1*/ /*fake_override*/ val args: kotlin.Array<kotlin.String>
+    public final val used: kotlin.String = "abc"
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+
+    public object Outer {
+        private constructor Outer()
+        public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+        public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+        public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+
+        public final class User {
+            public constructor User()
+            public final val property: kotlin.String = "abc"
+            public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+            public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+            public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+        }
+    }
+}

compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticsTestWithJvmIrBackendGenerated.java

@@ -690,6 +690,24 @@ public class DiagnosticsTestWithJvmIrBackendGenerated extends AbstractDiagnostic
             runTest("compiler/testData/diagnostics/testsWithJvmBackend/scripts/interfaceCapturesProperty.kts");
         }
 
+        @Test
+        @TestMetadata("nestedAndOuterClassesCaptureProperty.kts")
+        public void testNestedAndOuterClassesCaptureProperty() throws Exception {
+            runTest("compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedAndOuterClassesCaptureProperty.kts");
+        }
+
+        @Test
+        @TestMetadata("nestedClassCapturesProperty.kts")
+        public void testNestedClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedClassCapturesProperty.kts");
+        }
+
+        @Test
+        @TestMetadata("nestedToObjectClassCapturesProperty.kts")
+        public void testNestedToObjectClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/diagnostics/testsWithJvmBackend/scripts/nestedToObjectClassCapturesProperty.kts");
+        }
+
         @Test
         @TestMetadata("objectCapturesProperty.kts")
         public void testObjectCapturesProperty() throws Exception {

compiler/tests-gen/org/jetbrains/kotlin/codegen/ScriptCodegenTestGenerated.java

@@ -252,11 +252,36 @@ public class ScriptCodegenTestGenerated extends AbstractScriptCodegenTest {
             runTest("compiler/testData/codegen/script/scriptInstanceCapturing/enumEntryCapturesProperty.kts");
         }
 
+        @TestMetadata("innerClassesHierarchyCaptureProperty.kts")
+        public void testInnerClassesHierarchyCaptureProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/innerClassesHierarchyCaptureProperty.kts");
+        }
+
         @TestMetadata("interfaceCapturesProperty.kts")
         public void testInterfaceCapturesProperty() throws Exception {
             runTest("compiler/testData/codegen/script/scriptInstanceCapturing/interfaceCapturesProperty.kts");
         }
 
+        @TestMetadata("nestedAndOuterClassesCaptureProperty.kts")
+        public void testNestedAndOuterClassesCaptureProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedAndOuterClassesCaptureProperty.kts");
+        }
+
+        @TestMetadata("nestedClassCapturesProperty.kts")
+        public void testNestedClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedClassCapturesProperty.kts");
+        }
+
+        @TestMetadata("nestedInnerClassCapturesProperty.kts")
+        public void testNestedInnerClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedInnerClassCapturesProperty.kts");
+        }
+
+        @TestMetadata("nestedToObjectClassCapturesProperty.kts")
+        public void testNestedToObjectClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedToObjectClassCapturesProperty.kts");
+        }
+
         @TestMetadata("objectCapturesProperty.kts")
         public void testObjectCapturesProperty() throws Exception {
             runTest("compiler/testData/codegen/script/scriptInstanceCapturing/objectCapturesProperty.kts");

compiler/tests-gen/org/jetbrains/kotlin/codegen/ir/IrScriptCodegenTestGenerated.java

@@ -252,11 +252,36 @@ public class IrScriptCodegenTestGenerated extends AbstractIrScriptCodegenTest {
             runTest("compiler/testData/codegen/script/scriptInstanceCapturing/enumEntryCapturesProperty.kts");
         }
 
+        @TestMetadata("innerClassesHierarchyCaptureProperty.kts")
+        public void testInnerClassesHierarchyCaptureProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/innerClassesHierarchyCaptureProperty.kts");
+        }
+
         @TestMetadata("interfaceCapturesProperty.kts")
         public void testInterfaceCapturesProperty() throws Exception {
             runTest("compiler/testData/codegen/script/scriptInstanceCapturing/interfaceCapturesProperty.kts");
         }
 
+        @TestMetadata("nestedAndOuterClassesCaptureProperty.kts")
+        public void testNestedAndOuterClassesCaptureProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedAndOuterClassesCaptureProperty.kts");
+        }
+
+        @TestMetadata("nestedClassCapturesProperty.kts")
+        public void testNestedClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedClassCapturesProperty.kts");
+        }
+
+        @TestMetadata("nestedInnerClassCapturesProperty.kts")
+        public void testNestedInnerClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedInnerClassCapturesProperty.kts");
+        }
+
+        @TestMetadata("nestedToObjectClassCapturesProperty.kts")
+        public void testNestedToObjectClassCapturesProperty() throws Exception {
+            runTest("compiler/testData/codegen/script/scriptInstanceCapturing/nestedToObjectClassCapturesProperty.kts");
+        }
+
         @TestMetadata("objectCapturesProperty.kts")
         public void testObjectCapturesProperty() throws Exception {
             runTest("compiler/testData/codegen/script/scriptInstanceCapturing/objectCapturesProperty.kts");