Add inspection to simplify successive null checks #KT-14799 Fixed

2017-06-21 15:02:06 +03:00
parent fc12f37105
commit 7407083624
17 changed files with 307 additions and 0 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports when chained null-checks can be replaced with safe-calls
+</body>
+</html>
@@ -2378,6 +2378,14 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.NullChecksToSafeCallInspection"
+                     displayName="Replace chained null-checks with safe-calls"
+                     groupName="Kotlin"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,113 @@
+/*
+ * 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.codeInsight.FileModificationService
+import com.intellij.codeInspection.*
+import com.intellij.openapi.project.Project
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.intentions.branchedTransformations.isStable
+import org.jetbrains.kotlin.lexer.KtToken
+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.types.TypeUtils
+
+class NullChecksToSafeCallInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) =
+            object : KtVisitorVoid() {
+                override fun visitBinaryExpression(expression: KtBinaryExpression) {
+                    if (isNullChecksToSafeCallFixAvailable(expression)) {
+                        holder.registerProblem(expression,
+                                               "Null-checks replaceable with safe-calls",
+                                               ProblemHighlightType.WEAK_WARNING,
+                                               NullChecksToSafeCallCheckFix())
+                    }
+                }
+            }
+
+    private class NullChecksToSafeCallCheckFix : LocalQuickFix {
+        override fun getName() = "Replace chained null-checks with safe-calls"
+        override fun getFamilyName() = name
+
+        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+            applyFix(descriptor.psiElement as? KtBinaryExpression ?: return)
+        }
+
+        private fun applyFix(expression: KtBinaryExpression) {
+            if (!FileModificationService.getInstance().preparePsiElementForWrite(expression)) return
+            val (lte, rte, isAnd) = collectNullCheckExpressions(expression) ?: return
+            val parent = expression.parent
+            expression.replaced(KtPsiFactory(lte).buildExpression {
+                appendExpression(lte)
+                appendFixedText("?.")
+                appendExpression(rte.selectorExpression)
+                appendFixedText(if (isAnd) "!= null" else "== null")
+            })
+            if (isNullChecksToSafeCallFixAvailable(parent as? KtBinaryExpression ?: return)) {
+                applyFix(parent)
+            }
+        }
+    }
+
+    companion object {
+        private fun isNullChecksToSafeCallFixAvailable(expression: KtBinaryExpression): Boolean {
+            fun String.afterIgnoreCalls() = replace("?.", ".")
+
+            val (lte, rte) = collectNullCheckExpressions(expression) ?: return false
+            val context = expression.analyze()
+            if (!lte.isChainStable(context)) return false
+
+            val resolvedCall = rte.getResolvedCall(context) ?: return false
+            val extensionReceiver = resolvedCall.extensionReceiver
+            if (extensionReceiver != null && TypeUtils.isNullableType(extensionReceiver.type)) return false
+
+            return rte.receiverExpression.text.afterIgnoreCalls() == lte.text.afterIgnoreCalls()
+        }
+
+        private fun collectNullCheckExpressions(expression: KtBinaryExpression): Triple<KtExpression, KtQualifiedExpression, Boolean>? {
+            val isAnd = when (expression.operationToken) {
+                KtTokens.ANDAND -> true
+                KtTokens.OROR -> false
+                else -> return null
+            }
+            val lhs = expression.left as? KtBinaryExpression ?: return null
+            val rhs = expression.right as? KtBinaryExpression ?: return null
+            val expectedOperation = if (isAnd) KtTokens.EXCLEQ else KtTokens.EQEQ
+            val lte = lhs.getNullTestableExpression(expectedOperation) ?: return null
+            val rte = rhs.getNullTestableExpression(expectedOperation) as? KtQualifiedExpression ?: return null
+            return Triple(lte, rte, isAnd)
+        }
+
+        private fun KtBinaryExpression.getNullTestableExpression(expectedOperation: KtToken): KtExpression? {
+            if (operationToken != expectedOperation) return null
+            val lhs = left ?: return null
+            val rhs = right ?: return null
+            if (KtPsiUtil.isNullConstant(lhs)) return rhs
+            if (KtPsiUtil.isNullConstant(rhs)) return lhs
+            return null
+        }
+
+        private fun KtExpression.isChainStable(context: BindingContext): Boolean = when (this) {
+            is KtReferenceExpression -> isStable(context)
+            is KtQualifiedExpression -> selectorExpression?.isStable(context) == true && receiverExpression.isChainStable(context)
+            else -> false
+        }
+    }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.NullChecksToSafeCallInspection
@@ -0,0 +1,10 @@
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    if (a<caret> != null && a.a != null && a.a.a != null && a.a.a.a != null) {
+
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,10 @@
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    if (a?.a?.a?.a != null) {
+
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,7 @@
+class My {
+    fun foo(): String? = null
+}
+
+fun test(my: My?) {
+    if (<caret>my != null && my.foo() != null) {}
+}
@@ -0,0 +1,7 @@
+class My {
+    fun foo(): String? = null
+}
+
+fun test(my: My?) {
+    if (my?.foo() != null) {}
+}
@@ -0,0 +1,10 @@
+// PROBLEM: none
+
+class KotlinType
+class KotlinTypeInfo(val type: KotlinType?) {
+    fun foo(other: KotlinTypeInfo) {
+        if (<caret>type != null && other.type != null) {
+
+        }
+    }
+}
@@ -0,0 +1,13 @@
+// PROBLEM: none
+
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    // Controversial case, better to do nothing here
+    if (a<caret> != null && a.a?.a != null) {
+
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,10 @@
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    if (a<caret> == null || a.a == null || a.a.a == null || a.a.a.a == null) {
+
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,10 @@
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    if (a?.a?.a?.a == null) {
+
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,11 @@
+
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    if (a?.a<caret> != null && a.a.a != null) {
+ 
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,11 @@
+
+fun main(args: Array<String>) {
+    val a: Testtt? = Testtt()
+    if (a?.a?.a != null) {
+
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,12 @@
+// PROBLEM: none
+var a: Testtt? = Testtt()
+
+fun main(args: Array<String>) {
+    if (a<caret> != null && a?.a != null) {
+ 
+    }
+}
+
+class Testtt {
+    val a: Testtt? = null
+}
@@ -0,0 +1,12 @@
+// PROBLEM: none
+val a: Testtt? = Testtt()
+
+fun main(args: Array<String>) {
+    if (a<caret> != null && a.a != null) {
+
+    }
+}
+
+class Testtt
+
+val Testtt?.a: Testtt? get() = null
@@ -957,6 +957,63 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class NullChecksToSafeCall extends AbstractLocalInspectionTest {
+        public void testAllFilesPresentInNullChecksToSafeCall() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/nullChecksToSafeCall"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("andCase.kt")
+        public void testAndCase() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/andCase.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("function.kt")
+        public void testFunction() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/function.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kotlinType.kt")
+        public void testKotlinType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/kotlinType.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("longRightExpression.kt")
+        public void testLongRightExpression() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/longRightExpression.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("orCase.kt")
+        public void testOrCase() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/orCase.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("safeCall.kt")
+        public void testSafeCall() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/safeCall.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("unstableChain.kt")
+        public void testUnstableChain() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/unstableChain.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("unstableExtensionInChain.kt")
+        public void testUnstableExtensionInChain() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/nullChecksToSafeCall/unstableExtensionInChain.kt");
+            doTest(fileName);
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/nullableBooleanElvis")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.NullChecksToSafeCallInspection`