Add inspection for suspicious 'var' property #KT-23691 Fixed

2018-10-11 18:28:12 +03:00
parent 3ede93df11
commit 29cc727c5a
11 changed files with 140 additions and 2 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports <b>var</b> properties with default setter and getter that doesn't reference backing field.
+</body>
+</html>
@@ -3069,6 +3069,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SuspiciousVarPropertyInspection"
+                     displayName="Suspicious 'var' property: its setter does not influence its getter result"
+                     groupPath="Kotlin"
+                     groupName="Probable bugs"
+                     enabledByDefault="true"
+                     level="WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.inspections
+
+import com.intellij.codeInspection.IntentionWrapper
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import org.jetbrains.kotlin.descriptors.PropertyDescriptor
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.quickfix.ChangeVariableMutabilityFix
+import org.jetbrains.kotlin.idea.references.mainReference
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
+import org.jetbrains.kotlin.resolve.BindingContext
+
+class SuspiciousVarPropertyInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) =
+        propertyVisitor(fun(property: KtProperty) {
+            if (property.isLocal || !property.isVar || property.initializer == null || property.setter != null) return
+            val getter = property.getter ?: return
+            val context = property.analyze()
+            val descriptor = context[BindingContext.DECLARATION_TO_DESCRIPTOR, property] as? PropertyDescriptor ?: return
+            if (context[BindingContext.BACKING_FIELD_REQUIRED, descriptor] == false) return
+            if (getter.hasBackingFieldReference()) return
+
+            holder.registerProblem(
+                property.valOrVarKeyword,
+                "Suspicious 'var' property: its setter does not influence its getter result",
+                ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                IntentionWrapper(
+                    ChangeVariableMutabilityFix(property, makeVar = false, deleteInitializer = true),
+                    property.containingFile
+                )
+            )
+        })
+
+    companion object {
+        private fun KtPropertyAccessor.hasBackingFieldReference(): Boolean {
+            val bodyExpression = this.bodyExpression ?: return false
+            return bodyExpression.isBackingFieldReference(property) || bodyExpression.anyDescendantOfType<KtNameReferenceExpression> {
+                it.isBackingFieldReference(property)
+            }
+        }
+
+        fun KtExpression.isBackingFieldReference(property: KtProperty): Boolean =
+            this is KtNameReferenceExpression && isBackingFieldReference(property)
+
+        private fun KtNameReferenceExpression.isBackingFieldReference(property: KtProperty): Boolean {
+            return text == KtTokens.FIELD_KEYWORD.value && mainReference.resolve() == property
+        }
+    }
+}
@@ -33,10 +33,12 @@ import org.jetbrains.kotlin.util.OperatorNameConventions
 class ChangeVariableMutabilityFix(
    element: KtValVarKeywordOwner,
    private val makeVar: Boolean,
-    private val actionText: String? = null
+    private val actionText: String? = null,
+    private val deleteInitializer: Boolean = false
 ) : KotlinQuickFixAction<KtValVarKeywordOwner>(element) {

-    override fun getText() = actionText ?: if (makeVar) "Change to var" else "Change to val"
+    override fun getText() = actionText
+        ?: (if (makeVar) "Change to var" else "Change to val") + (if (deleteInitializer) " and delete initializer" else "")

    override fun getFamilyName(): String = text

@@ -51,6 +53,9 @@ class ChangeVariableMutabilityFix(
        val factory = KtPsiFactory(project)
        val newKeyword = if (makeVar) factory.createVarKeyword() else factory.createValKeyword()
        element.valOrVarKeyword!!.replace(newKeyword)
+        if (deleteInitializer) {
+            (element as? KtProperty)?.initializer = null
+        }
    }

    companion object {
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.SuspiciousVarPropertyInspection
@@ -0,0 +1,5 @@
+// PROBLEM: none
+class Test {
+    <caret>var foo: Int = 0
+        get() = field
+}
@@ -0,0 +1,7 @@
+// PROBLEM: none
+class Test {
+    <caret>var foo: Int = 0
+        get() {
+            return field
+        }
+}
@@ -0,0 +1,7 @@
+// PROBLEM: none
+class Test {
+    <caret>var foo: Int
+        get() = 1
+        set(value) {
+        }
+}
@@ -0,0 +1,5 @@
+// FIX: Change to val and delete initializer
+class Test {
+    <caret>var foo: Int = 0
+        get() = 1
+}
@@ -0,0 +1,5 @@
+// FIX: Change to val and delete initializer
+class Test {
+    val foo: Int
+        get() = 1
+}
@@ -6228,6 +6228,39 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/suspiciousVarProperty")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class SuspiciousVarProperty extends AbstractLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInSuspiciousVarProperty() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/suspiciousVarProperty"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("hasBackingFieldRef.kt")
+        public void testHasBackingFieldRef() throws Exception {
+            runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef.kt");
+        }
+
+        @TestMetadata("hasBackingFieldRef2.kt")
+        public void testHasBackingFieldRef2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/hasBackingFieldRef2.kt");
+        }
+
+        @TestMetadata("hasSetter.kt")
+        public void testHasSetter() throws Exception {
+            runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/hasSetter.kt");
+        }
+
+        @TestMetadata("noBackingFieldRef.kt")
+        public void testNoBackingFieldRef() throws Exception {
+            runTest("idea/testData/inspectionsLocal/suspiciousVarProperty/noBackingFieldRef.kt");
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/unlabeledReturnInsideLambda")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.SuspiciousVarPropertyInspection`