diff --git a/compiler/testData/diagnostics/tests/constructorConsistency/inspection.kt b/compiler/testData/diagnostics/tests/constructorConsistency/inspection.kt new file mode 100644 index 00000000000..91c74f34021 --- /dev/null +++ b/compiler/testData/diagnostics/tests/constructorConsistency/inspection.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! +} diff --git a/compiler/testData/diagnostics/tests/constructorConsistency/inspection.txt b/compiler/testData/diagnostics/tests/constructorConsistency/inspection.txt new file mode 100644 index 00000000000..71fbb2e8b45 --- /dev/null +++ b/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 +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 670b1309e94..637983fe3c2 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/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"); diff --git a/idea/resources/inspectionDescriptions/LeakingThis.html b/idea/resources/inspectionDescriptions/LeakingThis.html new file mode 100644 index 00000000000..4d5fddb03ba --- /dev/null +++ b/idea/resources/inspectionDescriptions/LeakingThis.html @@ -0,0 +1,24 @@ + + +This inspection reports dangerous operations inside constructors including: + +These operations are dangerous because your class can be inherited, +and derived class is not yet initialized at this moment. Typical example: +
+abstract class Base {
+    val code = calculate()
+    abstract fun calculate(): Int
+}
+class Derived(private val x: Int) : Base() {
+    override fun calculate() = x
+}
+fun testIt() {
+    println(Derived(42).code) // Expected: 42, actual: 0
+}
+
 + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index f3d69badf87..3fdd12e8d0d 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1546,6 +1546,14 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/LeakingThisInspection.kt new file mode 100644 index 00000000000..5e2c08d9fc2 --- /dev/null +++ b/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 + } + ) + } + } + } + } +} \ No newline at end of file diff --git a/idea/testData/inspections/leakingThis/inspectionData/expected.xml b/idea/testData/inspections/leakingThis/inspectionData/expected.xml new file mode 100644 index 00000000000..ce14409a7c1 --- /dev/null +++ b/idea/testData/inspections/leakingThis/inspectionData/expected.xml @@ -0,0 +1,26 @@ + + + test.kt + 18 + light_idea_test_case + + Leaking 'this' in constructor + Leaking 'this' in constructor of non-final class Second + + + test.kt + 20 + light_idea_test_case + + Leaking 'this' in constructor + Calling non-final function foo in constructor + + + test.kt + 40 + light_idea_test_case + + Leaking 'this' in constructor + Accessing non-final property x in constructor + + diff --git a/idea/testData/inspections/leakingThis/inspectionData/inspections.test b/idea/testData/inspections/leakingThis/inspectionData/inspections.test new file mode 100644 index 00000000000..3a0a7c421cd --- /dev/null +++ b/idea/testData/inspections/leakingThis/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.LeakingThisInspection diff --git a/idea/testData/inspections/leakingThis/test.kt b/idea/testData/inspections/leakingThis/test.kt new file mode 100644 index 00000000000..deb9cb25b2b --- /dev/null +++ b/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! +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 90c74ccc155..06a94bae274 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/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");