Add "map.get() with not-null assertion operator" inspection #KT-25171 Fixed

2018-10-25 23:46:30 +09:00
parent c5c0cbccde
commit d3908aeb2e
13 changed files with 156 additions and 1 deletions
@@ -425,7 +425,6 @@ fun returnExpressionVisitor(block: (KtReturnExpression) -> Unit) =
        }
    }

-
 fun delegatedSuperTypeEntry(block: (KtDelegatedSuperTypeEntry) -> Unit) =
    object : KtVisitorVoid() {
        override fun visitDelegatedSuperTypeEntry(delegatedSuperTypeEntry: KtDelegatedSuperTypeEntry) {
@@ -433,3 +432,11 @@ fun delegatedSuperTypeEntry(block: (KtDelegatedSuperTypeEntry) -> Unit) =
            block(delegatedSuperTypeEntry)
        }
    }
+
+fun postfixExpressionVisitor(block: (KtPostfixExpression) -> Unit) =
+    object : KtVisitorVoid() {
+        override fun visitPostfixExpression(expression: KtPostfixExpression) {
+            super.visitPostfixExpression(expression)
+            block(expression)
+        }
+    }
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports <b>map.get()!!</b> that can be replaced with <b>map.getValue()</b>.
+</body>
+</html>
@@ -3117,6 +3117,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.MapGetWithNotNullAssertionOperatorInspection"
+                     displayName="map.get() with not-null assertion operator (!!)"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="INFO"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,64 @@
+/*
+ * 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.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.project.Project
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
+import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.intentions.callExpression
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.endOffset
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
+
+class MapGetWithNotNullAssertionOperatorInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) =
+        postfixExpressionVisitor(fun(expression: KtPostfixExpression) {
+            if (expression.operationToken != KtTokens.EXCLEXCL) return
+            if (expression.getReplacementData() == null) return
+            if (expression.baseExpression?.resolveToCall()?.resultingDescriptor?.fqNameSafe != FqName("kotlin.collections.Map.get")) return
+            holder.registerProblem(
+                expression.operationReference,
+                "map.get() with not-null assertion operator (!!)",
+                ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                ReplaceWithGetValueCallFix()
+            )
+        })
+
+    private class ReplaceWithGetValueCallFix : LocalQuickFix {
+        override fun getName() = "Replace with 'getValue' call"
+        override fun getFamilyName() = name
+        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+            val expression = descriptor.psiElement.parent as? KtPostfixExpression ?: return
+            val (reference, index) = expression.getReplacementData() ?: return
+            val replaced = expression.replaced(KtPsiFactory(expression).createExpressionByPattern("$0.getValue($1)", reference, index))
+            replaced.findExistingEditor()?.caretModel?.moveToOffset(replaced.endOffset)
+        }
+    }
+}
+
+private fun KtPostfixExpression.getReplacementData(): Pair<KtExpression, KtExpression>? {
+    val base = baseExpression
+    when (base) {
+        is KtQualifiedExpression -> {
+            if (base.callExpression?.calleeExpression?.text != "get") return null
+            val reference = base.receiverExpression
+            val index = base.callExpression?.valueArguments?.firstOrNull()?.getArgumentExpression() ?: return null
+            return reference to index
+        }
+        is KtArrayAccessExpression -> {
+            val reference = base.arrayExpression ?: return null
+            val index = base.indexExpressions.firstOrNull() ?: return null
+            return reference to index
+        }
+        else -> return null
+    }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.MapGetWithNotNullAssertionOperatorInspection
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(map: Map<Int, String>) {
+    val s = map.get(1)!!<caret>
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(map: Map<Int, String>) {
+    val s = map.getValue(1)<caret>
+}
@@ -0,0 +1,5 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun test(map: Map<Int, String>) {
+    val s = map.getValue(1)!!<caret>
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(map: Map<Int, String>) {
+    val s = map[1]<caret>!!
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(map: Map<Int, String>) {
+    val s = map.getValue(1)<caret>
+}
@@ -0,0 +1,5 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun test(list: List<String>) {
+    val s = list.get(1)!!<caret>
+}
@@ -0,0 +1,5 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun test(map: Map<Int, String>) {
+    val s = map.get(1)<caret>
+}
@@ -3280,6 +3280,44 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class MapGetWithNotNullAssertionOperator extends AbstractLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInMapGetWithNotNullAssertionOperator() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("get.kt")
+        public void testGet() throws Exception {
+            runTest("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator/get.kt");
+        }
+
+        @TestMetadata("getValue.kt")
+        public void testGetValue() throws Exception {
+            runTest("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator/getValue.kt");
+        }
+
+        @TestMetadata("indexedAccess.kt")
+        public void testIndexedAccess() throws Exception {
+            runTest("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator/indexedAccess.kt");
+        }
+
+        @TestMetadata("list.kt")
+        public void testList() throws Exception {
+            runTest("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator/list.kt");
+        }
+
+        @TestMetadata("noNotNullAssersion.kt")
+        public void testNoNotNullAssersion() throws Exception {
+            runTest("idea/testData/inspectionsLocal/mapGetWithNotNullAssertionOperator/noNotNullAssersion.kt");
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/mayBeConstant")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.MapGetWithNotNullAssertionOperatorInspection`