Don't highlight "replace with +=" for read-only collections

Related to KT-20626
2018-12-04 12:39:25 +03:00
parent c560aada3d
commit ed8305995e
7 changed files with 63 additions and 3 deletions
@@ -16,6 +16,7 @@

 package org.jetbrains.kotlin.idea.inspections

+import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElement
@@ -24,10 +25,12 @@ import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.idea.analysis.analyzeAsReplacement
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.project.builtIns
 import org.jetbrains.kotlin.idea.util.psi.patternMatching.matches
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.callUtil.getType
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode

 class ReplaceWithOperatorAssignmentInspection : AbstractApplicabilityBasedInspection<KtBinaryExpression>(
@@ -57,6 +60,16 @@ class ReplaceWithOperatorAssignmentInspection : AbstractApplicabilityBasedInspec
    override fun fixText(element: KtBinaryExpression) =
        "Replace with '${(element.right as? KtBinaryExpression)?.operationReference?.operationSignTokenType?.value}='"

+    override fun inspectionHighlightType(element: KtBinaryExpression): ProblemHighlightType {
+        val left = element.left as? KtNameReferenceExpression
+        if (left != null) {
+            val context = left.analyze(BodyResolveMode.PARTIAL)
+            val leftType = left.getType(context)
+            if (leftType?.isReadOnlyCollectionOrMap(element.builtIns) == true) return ProblemHighlightType.INFORMATION
+        }
+        return ProblemHighlightType.GENERIC_ERROR_OR_WARNING
+    }
+
    private fun checkExpressionRepeat(
        variableExpression: KtNameReferenceExpression,
        expression: KtBinaryExpression,
@@ -11,6 +11,7 @@ import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.diagnostics.Severity
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
@@ -49,8 +50,7 @@ class SuspiciousCollectionReassignmentInspection : AbstractKotlinInspection() {
            val context = binaryExpression.analyze()
            val leftType = left.getType(context) ?: return
            val leftDefaultType = leftType.constructor.declarationDescriptor?.defaultType ?: return
-            val builtIns = binaryExpression.builtIns
-            if (leftDefaultType !in listOf(builtIns.list.defaultType, builtIns.set.defaultType, builtIns.map.defaultType)) return
+            if (!leftType.isReadOnlyCollectionOrMap(binaryExpression.builtIns)) return
            if (context.diagnostics.forElement(binaryExpression).any { it.severity == Severity.ERROR }) return

            val fixes = mutableListOf<LocalQuickFix>()
@@ -218,4 +218,9 @@ class SuspiciousCollectionReassignmentInspection : AbstractKotlinInspection() {
    }
 }

-private fun KotlinType.classDescriptor() = constructor.declarationDescriptor as? ClassDescriptor
+private fun KotlinType.classDescriptor() = constructor.declarationDescriptor as? ClassDescriptor
+
+internal fun KotlinType.isReadOnlyCollectionOrMap(builtIns: KotlinBuiltIns): Boolean {
+    val leftDefaultType = constructor.declarationDescriptor?.defaultType ?: return false
+    return leftDefaultType in listOf(builtIns.list.defaultType, builtIns.set.defaultType, builtIns.map.defaultType)
+}
@@ -0,0 +1,9 @@
+// WITH_RUNTIME
+// HIGHLIGHT: INFORMATION
+
+fun foo() {
+    var list = listOf(1, 2, 3)
+    // Should not be highlighted because it's the way we use to say explicitly
+    // "yes, we want to re-assign this immutable list"
+    list <caret>= list + 4
+}
@@ -0,0 +1,9 @@
+// WITH_RUNTIME
+// HIGHLIGHT: INFORMATION
+
+fun foo() {
+    var list = listOf(1, 2, 3)
+    // Should not be highlighted because it's the way we use to say explicitly
+    // "yes, we want to re-assign this immutable list"
+    list += 4
+}
@@ -0,0 +1,7 @@
+// WITH_RUNTIME
+// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
+
+fun foo() {
+    val list = mutableListOf(1, 2, 3)
+    list <caret>= list + 4
+}
@@ -0,0 +1,7 @@
+// WITH_RUNTIME
+// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
+
+fun foo() {
+    val list = mutableListOf(1, 2, 3)
+    list += 4
+}
@@ -5925,6 +5925,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
            runTest("idea/testData/inspectionsLocal/replaceWithOperatorAssignment/invalidSubtraction.kt");
        }

+        @TestMetadata("list.kt")
+        public void testList() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceWithOperatorAssignment/list.kt");
+        }
+
        @TestMetadata("multipleOperators.kt")
        public void testMultipleOperators() throws Exception {
            runTest("idea/testData/inspectionsLocal/replaceWithOperatorAssignment/multipleOperators.kt");
@@ -5935,6 +5940,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
            runTest("idea/testData/inspectionsLocal/replaceWithOperatorAssignment/multipleOperatorsRightSideRepeat.kt");
        }

+        @TestMetadata("mutableList.kt")
+        public void testMutableList() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceWithOperatorAssignment/mutableList.kt");
+        }
+
        @TestMetadata("nonCommutativeRepeat.kt")
        public void testNonCommutativeRepeat() throws Exception {
            runTest("idea/testData/inspectionsLocal/replaceWithOperatorAssignment/nonCommutativeRepeat.kt");