Introduce inspection detecting self-assignment of properties

So #KT-20714 Fixed
2017-10-22 19:14:54 +03:00
parent ffa9478d0c
commit 204d9e3423
34 changed files with 569 additions and 0 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection detects assignments of a variable to itself
+</body>
+</html>
@@ -2633,6 +2633,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SelfAssignmentInspection"
+                     displayName="Assignment of variable to itself"
+                     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,104 @@
+/*
+ * Copyright 2010-2017 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.*
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.descriptors.*
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitReceiver
+
+class SelfAssignmentInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
+        return object : KtVisitorVoid() {
+
+            private fun KtExpression.asNameReferenceExpression(): KtNameReferenceExpression? = when (this) {
+                is KtNameReferenceExpression ->
+                    this
+                is KtDotQualifiedExpression ->
+                    (selectorExpression as? KtNameReferenceExpression)?.takeIf { receiverExpression is KtThisExpression }
+                else ->
+                    null
+            }
+
+            private fun KtExpression.receiverDeclarationDescriptor(
+                    resolvedCall: ResolvedCall<out CallableDescriptor>,
+                    context: BindingContext
+            ): DeclarationDescriptor? {
+                val thisExpression = (this as? KtDotQualifiedExpression)?.receiverExpression as? KtThisExpression
+                if (thisExpression != null) {
+                    return thisExpression.getResolvedCall(context)?.resultingDescriptor?.containingDeclaration
+                }
+                val implicitReceiver = with (resolvedCall) { dispatchReceiver ?: extensionReceiver } as? ImplicitReceiver
+                return implicitReceiver?.declarationDescriptor
+            }
+
+            override fun visitBinaryExpression(expression: KtBinaryExpression) {
+                super.visitBinaryExpression(expression)
+
+                if (expression.operationToken != KtTokens.EQ) return
+                val left = expression.left
+                val leftRefExpr = left?.asNameReferenceExpression() ?: return
+                val right = expression.right
+                val rightRefExpr = right?.asNameReferenceExpression() ?: return
+                // To omit analyzing too much
+                if (leftRefExpr.text != rightRefExpr.text) return
+
+                val context = expression.analyze(BodyResolveMode.PARTIAL)
+                val leftResolvedCall = left.getResolvedCall(context)
+                val leftCallee = leftResolvedCall?.resultingDescriptor as? VariableDescriptor ?: return
+                val rightResolvedCall = right.getResolvedCall(context)
+                val rightCallee = rightResolvedCall?.resultingDescriptor as? VariableDescriptor ?: return
+                if (leftCallee != rightCallee) return
+
+                if (!rightCallee.isVar) return
+                if (rightCallee is PropertyDescriptor) {
+                    if (rightCallee.isOverridable) return
+                    if (rightCallee.accessors.any { !it.isDefault }) return
+                }
+
+                if (left.receiverDeclarationDescriptor(leftResolvedCall, context) !=
+                    right.receiverDeclarationDescriptor(rightResolvedCall, context)) {
+                    return
+                }
+
+                holder.registerProblem(right,
+                                       "Variable '${rightCallee.name}' is assigned to itself",
+                                       ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                                       RemoveSelfAssignmentFix())
+            }
+        }
+    }
+}
+
+private class RemoveSelfAssignmentFix : LocalQuickFix {
+    override fun getName() = "Remove self assignment"
+
+    override fun getFamilyName() = name
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val right = descriptor.psiElement as? KtExpression ?: return
+        right.parent.delete()
+    }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.SelfAssignmentInspection
@@ -0,0 +1,17 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+// Minimized from KT-20714 itself
+class ServerUser {
+    var id = ""
+    var city = ""
+
+    fun toClientUser() = ClientUser().apply {
+        id = <caret>this@ServerUser.id
+        city = this@ServerUser.city
+    }
+}
+
+class ClientUser {
+    var id = ""
+}
@@ -0,0 +1,18 @@
+// PROBLEM: Variable 'city' is assigned to itself
+// WITH_RUNTIME
+// FIX: Remove self assignment
+
+// Minimized from KT-20714 itself
+class ServerUser {
+    var id = ""
+    var city = ""
+
+    fun toClientUser() = ClientUser().apply {
+        id = this@ServerUser.id
+        city = <caret>this@ServerUser.city
+    }
+}
+
+class ClientUser {
+    var id = ""
+}
@@ -0,0 +1,17 @@
+// PROBLEM: Variable 'city' is assigned to itself
+// WITH_RUNTIME
+// FIX: Remove self assignment
+
+// Minimized from KT-20714 itself
+class ServerUser {
+    var id = ""
+    var city = ""
+
+    fun toClientUser() = ClientUser().apply {
+        id = this@ServerUser.id
+    }
+}
+
+class ClientUser {
+    var id = ""
+}
@@ -0,0 +1,11 @@
+// PROBLEM: none
+// SKIP_ERRORS_BEFORE
+// SKIP_ERRORS_AFTER
+
+class Point {
+    val x: Int
+
+    constructor(x: Int) {
+        x = <caret>x
+    }
+}
@@ -0,0 +1,12 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        with (Test()) {
+            this@Test.foo = <caret>this.foo // Different receivers
+        }
+    }
+}
@@ -0,0 +1,12 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        with (Test()) {
+            this@Test.foo = <caret>foo // Different receiver
+        }
+    }
+}
@@ -0,0 +1,7 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+fun test() {
+    val list = mutableListOf(1, 2, 3)
+    list[1] = <caret>list[1]
+}
@@ -0,0 +1,8 @@
+// PROBLEM: none
+// SKIP_ERRORS_BEFORE
+// SKIP_ERRORS_AFTER
+
+fun test() {
+    val bar = 1
+    bar = <caret>bar
+}
@@ -0,0 +1,7 @@
+// PROBLEM: Variable 'bar' is assigned to itself
+// FIX: Remove self assignment
+
+fun test() {
+    var bar = 1
+    bar = <caret>bar
+}
@@ -0,0 +1,6 @@
+// PROBLEM: Variable 'bar' is assigned to itself
+// FIX: Remove self assignment
+
+fun test() {
+    var bar = 1
+}
@@ -0,0 +1,6 @@
+// PROBLEM: none
+
+fun test() {
+    var bar = 1
+    bar += <caret>bar
+}
@@ -0,0 +1,12 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+class Bar(var foo: Int = 1)
+
+class Test(var foo: Int = 2) {
+    fun test() {
+        Bar().apply {
+            this.foo = <caret>this@Test.foo
+        }
+    }
+}
@@ -0,0 +1,9 @@
+// PROBLEM: none
+
+open class Test {
+    open var foo = 1
+
+    fun test() {
+        foo = <caret>this.foo
+    }
+}
@@ -0,0 +1,7 @@
+// PROBLEM: none
+// SKIP_ERRORS_BEFORE
+// SKIP_ERRORS_AFTER
+
+fun test(bar: Int) {
+    bar = <caret>bar
+}
@@ -0,0 +1,10 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        foo = <caret>foo
+    }
+}
@@ -0,0 +1,9 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+    }
+}
@@ -0,0 +1,10 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        foo = <caret>this.foo
+    }
+}
@@ -0,0 +1,9 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+    }
+}
@@ -0,0 +1,10 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        foo = <caret>this@Test.foo
+    }
+}
@@ -0,0 +1,9 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+    }
+}
@@ -0,0 +1,20 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+import kotlin.properties.ReadWriteProperty
+import kotlin.reflect.KProperty
+
+class Test {
+    var foo: Int by Delegate()
+
+    fun test() {
+        foo = <caret>foo
+    }
+}
+
+class Delegate : ReadWriteProperty<Test, Int> {
+    override fun getValue(thisRef: Test, property: KProperty<*>): Int = 1
+    override fun setValue(thisRef: Test, property: KProperty<*>, value: Int) {
+        println()
+    }
+}
@@ -0,0 +1,14 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+class Test {
+    var foo = 1
+        get() {
+            println()
+            return 2
+        }
+
+    fun test() {
+        foo = <caret>foo
+    }
+}
@@ -0,0 +1,13 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+class Test {
+    var foo = 1
+        set(value) {
+            println(value)
+        }
+
+    fun test() {
+        foo = <caret>foo
+    }
+}
@@ -0,0 +1,10 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        this.foo = <caret>this@Test.foo
+    }
+}
@@ -0,0 +1,9 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+    }
+}
@@ -0,0 +1,13 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// WITH_RUNTIME
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        with (Test()) {
+            this.foo = <caret>foo
+        }
+    }
+}
@@ -0,0 +1,12 @@
+// PROBLEM: Variable 'foo' is assigned to itself
+// WITH_RUNTIME
+// FIX: Remove self assignment
+
+class Test {
+    var foo = 1
+
+    fun test() {
+        with (Test()) {
+        }
+    }
+}
@@ -0,0 +1,5 @@
+// PROBLEM: none
+
+fun test(bar: Int) {
+    var bar = <caret>bar
+}
@@ -0,0 +1,7 @@
+// PROBLEM: none
+
+class Point(var x: Int) {
+    fun copyFrom(other: Point) {
+        x = <caret>other.x
+    }
+}
@@ -1914,6 +1914,147 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/selfAssignment")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class SelfAssignment extends AbstractLocalInspectionTest {
+        public void testAllFilesPresentInSelfAssignment() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/selfAssignment"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("applyCorrect.kt")
+        public void testApplyCorrect() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/applyCorrect.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("applyIncorrect.kt")
+        public void testApplyIncorrect() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/applyIncorrect.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("assignToProperty.kt")
+        public void testAssignToProperty() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/assignToProperty.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("differentThese.kt")
+        public void testDifferentThese() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/differentThese.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("differentThis.kt")
+        public void testDifferentThis() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/differentThis.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("list.kt")
+        public void testList() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/list.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("localVal.kt")
+        public void testLocalVal() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/localVal.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("localVar.kt")
+        public void testLocalVar() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/localVar.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("notAssignment.kt")
+        public void testNotAssignment() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/notAssignment.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("notSelf.kt")
+        public void testNotSelf() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/notSelf.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("openProperty.kt")
+        public void testOpenProperty() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/openProperty.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("parameter.kt")
+        public void testParameter() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/parameter.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("property1.kt")
+        public void testProperty1() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/property1.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("property2.kt")
+        public void testProperty2() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/property2.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("property3.kt")
+        public void testProperty3() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/property3.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("propertyHasDelegate.kt")
+        public void testPropertyHasDelegate() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/propertyHasDelegate.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("propertyHasGetter.kt")
+        public void testPropertyHasGetter() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/propertyHasGetter.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("propertyHasSetter.kt")
+        public void testPropertyHasSetter() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/propertyHasSetter.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("sameThese.kt")
+        public void testSameThese() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/sameThese.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("sameThis.kt")
+        public void testSameThis() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/sameThis.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("shadow.kt")
+        public void testShadow() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/shadow.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("withReceiver.kt")
+        public void testWithReceiver() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/selfAssignment/withReceiver.kt");
+            doTest(fileName);
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/unnecessaryVariable")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.SelfAssignmentInspection`