KT-12152 : leaking this inspection: accessing non-final member / this in non-final class

compiler/testData/diagnostics/tests/constructorConsistency/inspection.kt

@@ -0,0 +1,56 @@
+class First {
+    val x: String
+
+    init {
+        use(<!DEBUG_INFO_LEAKING_THIS!>this<!>) // NPE! Leaking this
+        x = <!DEBUG_INFO_LEAKING_THIS!>foo<!>() // NPE! Own function
+    }
+
+    fun foo() = x
+}
+
+fun use(first: First) = first.x.hashCode()
+
+abstract class Second {
+    val x: String
+
+    init {
+        use(<!DEBUG_INFO_LEAKING_THIS!>this<!>) // Leaking this in non-final
+        x = <!DEBUG_INFO_LEAKING_THIS!>bar<!>() // Own function in non-final
+        <!DEBUG_INFO_LEAKING_THIS!>foo<!>()     // Non-final function call
+    }
+
+    private fun bar() = foo()
+
+    abstract fun foo(): String
+}
+
+fun use(second: Second) = second.x
+
+class SecondDerived : Second() {
+    val y = x // null!
+
+    override fun foo() = y
+}
+
+open class Third {
+    open var x: String
+
+    constructor() {
+        <!DEBUG_INFO_LEAKING_THIS!>x<!> = "X" // Non-final property access
+    }
+}
+
+class ThirdDerived : Third() {
+    override var x: String = "Y"
+        set(arg) { field = "$arg$y" }
+
+    val y = ""
+}
+
+class Fourth {
+    val x: String
+        get() = y
+
+    val y = <!DEBUG_INFO_LEAKING_THIS!>x<!> // null!
+}

compiler/testData/diagnostics/tests/constructorConsistency/inspection.txt

@@ -0,0 +1,60 @@
+package
+
+public fun use(/*0*/ first: First): kotlin.Int
+public fun use(/*0*/ second: Second): kotlin.String
+
+public final class First {
+    public constructor First()
+    public final val x: kotlin.String
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final fun foo(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class Fourth {
+    public constructor Fourth()
+    public final val x: kotlin.String
+    public final val y: kotlin.String
+    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 abstract class Second {
+    public constructor Second()
+    public final val x: kotlin.String
+    private final fun bar(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public abstract fun foo(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class SecondDerived : Second {
+    public constructor SecondDerived()
+    public final override /*1*/ /*fake_override*/ val x: kotlin.String
+    public final val y: kotlin.String
+    invisible_fake final override /*1*/ /*fake_override*/ fun bar(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ fun foo(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public open class Third {
+    public constructor Third()
+    public open var x: kotlin.String
+    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 ThirdDerived : Third {
+    public constructor ThirdDerived()
+    public open override /*1*/ var x: kotlin.String
+    public final val y: kotlin.String = ""
+    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/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java

@@ -2973,6 +2973,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("inspection.kt")
+            public void testInspection() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/constructorConsistency/inspection.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("lambdaInObject.kt")
             public void testLambdaInObject() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/constructorConsistency/lambdaInObject.kt");

idea/resources/inspectionDescriptions/LeakingThis.html

@@ -0,0 +1,24 @@
+<html>
+<body>
+This inspection reports dangerous operations inside constructors including:
+<ul>
+  <li>Accessing non-final property in constructor</li>
+  <li>Calling non-final function in constructor</li>
+  <li>Using 'this' as function argument in constructor of non-final class</li>
+</ul>
+These operations are dangerous because your class can be inherited,
+and derived class is not yet initialized at this moment. Typical example:
+<code><pre>
+<b>abstract class</b> Base {
+    <b>val</b> code = calculate()
+    <b>abstract fun</b> calculate(): Int
+}
+<b>class</b> Derived(<b>private val</b> x: Int) : Base() {
+    <b>override fun</b> calculate() = x
+}
+<b>fun</b> testIt() {
+    println(Derived(42).code) <i>// Expected: 42, actual: 0</i>
+}
+</pre></code>
+</body>
+</html>

idea/src/META-INF/plugin.xml

@@ -1546,6 +1546,14 @@
                      language="kotlin"
     />
 
+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.LeakingThisInspection"
+                     displayName="Leaking 'this' in constructor"
+                     groupName="Kotlin"
+                     enabledByDefault="true"
+                     level="WARNING"
+                     language="kotlin"
+    />
+
     <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
 
     <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>

idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2010-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.idea.inspections
+
+import com.intellij.codeInspection.LocalInspectionToolSession
+import com.intellij.codeInspection.ProblemHighlightType.*
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.cfg.LeakingThisDescriptor.*
+import org.jetbrains.kotlin.idea.caches.resolve.analyzeFully
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.resolve.BindingContext.LEAKING_THIS
+
+class LeakingThisInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
+        return object : KtVisitorVoid() {
+            override fun visitExpression(expression: KtExpression) {
+                val context = expression.analyzeFully()
+                val leakingThisDescriptor = context.get(LEAKING_THIS, expression) ?: return
+                val description = when (leakingThisDescriptor) {
+                    is PropertyIsNull -> null // Not supported yet
+                    is NonFinalClass ->
+                        if (expression is KtThisExpression)
+                            "Leaking 'this' in constructor of non-final class ${leakingThisDescriptor.klass.name}"
+                        else
+                            null // Not supported yet
+                    is NonFinalProperty ->
+                        "Accessing non-final property ${leakingThisDescriptor.property.name} in constructor"
+                    is NonFinalFunction ->
+                        "Calling non-final function ${leakingThisDescriptor.function.name} in constructor"
+                }
+                if (description != null) {
+                    holder.registerProblem(
+                            expression, description,
+                            when (leakingThisDescriptor) {
+                                is NonFinalProperty, is NonFinalFunction -> GENERIC_ERROR_OR_WARNING
+                                else -> WEAK_WARNING
+                            }
+                    )
+                }
+            }
+        }
+    }
+}

idea/testData/inspections/leakingThis/inspectionData/expected.xml

@@ -0,0 +1,26 @@
+<problems>
+    <problem>
+        <file>test.kt</file>
+        <line>18</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Leaking 'this' in constructor</problem_class>
+        <description>Leaking 'this' in constructor of non-final class Second</description>
+    </problem>
+    <problem>
+        <file>test.kt</file>
+        <line>20</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Leaking 'this' in constructor</problem_class>
+        <description>Calling non-final function foo in constructor</description>
+    </problem>
+    <problem>
+        <file>test.kt</file>
+        <line>40</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Leaking 'this' in constructor</problem_class>
+        <description>Accessing non-final property x in constructor</description>
+    </problem>
+</problems>

idea/testData/inspections/leakingThis/inspectionData/inspections.test

@@ -0,0 +1 @@
+// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.LeakingThisInspection

idea/testData/inspections/leakingThis/test.kt

@@ -0,0 +1,56 @@
+class First {
+    val x: String
+
+    init {
+        use(this) // NPE! Leaking this
+        x = foo() // NPE! Own function
+    }
+
+    fun foo() = x
+}
+
+fun use(first: First) = first.x.hashCode()
+
+abstract class Second {
+    val x: String
+
+    init {
+        use(this) // Leaking this in non-final
+        x = bar() // Own function in non-final
+        foo()     // Non-final function call
+    }
+
+    private fun bar() = foo()
+
+    abstract fun foo(): String
+}
+
+fun use(second: Second) = second.x
+
+class SecondDerived : Second() {
+    val y = x // null!
+
+    override fun foo() = y
+}
+
+open class Third {
+    open var x: String
+
+    constructor() {
+        x = "X" // Non-final property access
+    }
+}
+
+class ThirdDerived : Third() {
+    override var x: String = "Y"
+        set(arg) { field = "$arg$y" }
+
+    val y = ""
+}
+
+class Fourth {
+    val x: String
+        get() = y
+
+    val y = x // null!
+}

idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java

@@ -148,6 +148,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("leakingThis/inspectionData/inspections.test")
+        public void testLeakingThis_inspectionData_Inspections_test() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/leakingThis/inspectionData/inspections.test");
+            doTest(fileName);
+        }
+
         @TestMetadata("overridingDeprecatedMember/inspectionData/inspections.test")
         public void testOverridingDeprecatedMember_inspectionData_Inspections_test() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/overridingDeprecatedMember/inspectionData/inspections.test");