Add inspection to replace not-null assertion with elvis return

#KT-30381 Fixed
2019-03-12 12:05:00 +09:00
parent 1c0c01725b
commit dffcfdc02f
24 changed files with 282 additions and 0 deletions
@@ -3355,6 +3355,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ReplaceNotNullAssertionWithElvisReturnInspection"
+                     displayName="Replace '!!' with '?: return'"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="false"
+                     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,13 @@
+<html>
+<body>
+This inspection reports not-null assertion (<b>!!</b>) calls that can be replaced with elvis and return (<b>?: return</b>). For example:
+<br /><br />
+<pre>
+val number: Int? = 42
+fun foo() {
+    val a = number!! // Replace '!!' with '?: return'
+    println(1 + a)
+}
+</pre>
+</body>
+</html>
@@ -0,0 +1,90 @@
+/*
+ * Copyright 2010-2019 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.codeInsight.intention.LowPriorityAction
+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.descriptors.FunctionDescriptor
+import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.intentions.getParentLambdaLabelName
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypes
+import org.jetbrains.kotlin.psi.psiUtil.getTopmostParentOfType
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+import org.jetbrains.kotlin.types.isNullable
+import org.jetbrains.kotlin.types.typeUtil.isUnit
+
+class ReplaceNotNullAssertionWithElvisReturnInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = postfixExpressionVisitor(fun(postfix) {
+        if (postfix.baseExpression == null) return
+        val operationReference = postfix.operationReference
+        if (operationReference.getReferencedNameElementType() != KtTokens.EXCLEXCL) return
+
+        if ((postfix.getTopmostParentOfType<KtParenthesizedExpression>() ?: postfix).parent is KtReturnExpression) return
+
+        val parent = postfix.getParentOfTypes(true, KtLambdaExpression::class.java, KtNamedFunction::class.java)
+        if (parent !is KtNamedFunction && parent !is KtLambdaExpression) return
+        val context = postfix.analyze(BodyResolveMode.PARTIAL_WITH_DIAGNOSTICS)
+        val (isNullable, returnLabelName) = when (parent) {
+            is KtNamedFunction -> {
+                val returnType = parent.descriptor(context)?.returnType ?: return
+                val isNullable = returnType.isNullable()
+                if (!returnType.isUnit() && !isNullable) return
+                isNullable to null
+            }
+            is KtLambdaExpression -> {
+                val functionLiteral = parent.functionLiteral
+                val returnType = functionLiteral.descriptor(context)?.returnType ?: return
+                if (!returnType.isUnit()) return
+                val lambdaLabelName = functionLiteral.bodyBlockExpression?.getParentLambdaLabelName() ?: return
+                false to lambdaLabelName
+            }
+            else -> return
+        }
+
+        if (context.diagnostics.forElement(operationReference).any { it.factory == Errors.UNNECESSARY_NOT_NULL_ASSERTION }) return
+
+        holder.registerProblem(
+            postfix.operationReference,
+            "Replace '!!' with '?: return'",
+            ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+            ReplaceWithElvisReturnFix(isNullable, returnLabelName)
+        )
+    })
+
+    private fun KtFunction.descriptor(context: BindingContext): FunctionDescriptor? {
+        return context[BindingContext.DECLARATION_TO_DESCRIPTOR, this] as? FunctionDescriptor
+    }
+
+    private class ReplaceWithElvisReturnFix(
+        private val returnNull: Boolean,
+        private val returnLabelName: String?
+    ) : LocalQuickFix, LowPriorityAction {
+        override fun getName() = "Replace with '?: return${if (returnNull) " null" else ""}'"
+        override fun getFamilyName() = name
+        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+            val postfix = descriptor.psiElement.parent as? KtPostfixExpression ?: return
+            val base = postfix.baseExpression ?: return
+            val psiFactory = KtPsiFactory(postfix)
+            postfix.replaced(
+                psiFactory.createExpressionByPattern(
+                    "$0 ?: return$1$2",
+                    base,
+                    returnLabelName?.let { "@$it" } ?: "",
+                    if (returnNull) " null" else ""
+                )
+            )
+        }
+    }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.ReplaceNotNullAssertionWithElvisReturnInspection
@@ -0,0 +1,3 @@
+fun test(i: Int?) {
+    val x = i!!<caret>
+}
@@ -0,0 +1,3 @@
+fun test(i: Int?) {
+    val x = i ?: return
+}
@@ -0,0 +1,5 @@
+fun foo(): Int? = null
+
+fun test() {
+    foo()!!<caret>
+}
@@ -0,0 +1,5 @@
+fun foo(): Int? = null
+
+fun test() {
+    foo() ?: return
+}
@@ -0,0 +1,6 @@
+class Foo(val i: Int?)
+
+fun test(foo: Foo): Int? {
+    val x = foo.i!!<caret>
+    return x
+}
@@ -0,0 +1,6 @@
+class Foo(val i: Int?)
+
+fun test(foo: Foo): Int? {
+    val x = foo.i ?: return null
+    return x
+}
@@ -0,0 +1,8 @@
+class Foo {
+    fun baz(): Int? = null
+}
+
+fun test(foo: Foo): Int? {
+    foo.baz()!!<caret>
+    return 0
+}
@@ -0,0 +1,8 @@
+class Foo {
+    fun baz(): Int? = null
+}
+
+fun test(foo: Foo): Int? {
+    foo.baz() ?: return null
+    return 0
+}
@@ -0,0 +1,3 @@
+fun test(i: Int?) {
+    val x = i!!<caret> + 1
+}
@@ -0,0 +1,3 @@
+fun test(i: Int?) {
+    val x = (i ?: return) + 1
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(list: List<String>, number: Int?) {
+    list.forEach {
+        number!!<caret>
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(list: List<String>, number: Int?) {
+    list.forEach {
+        number ?: return@forEach
+    }
+}
@@ -0,0 +1,7 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun test(list: List<String>, number: Int?) {
+    val x: List<Int> = list.map {
+        number!!<caret>
+    }
+}
@@ -0,0 +1,5 @@
+// PROBLEM: none
+fun test(i: Int?): Int {
+    val x = i!!<caret>
+    return x
+}
@@ -0,0 +1,4 @@
+// PROBLEM: none
+fun test(i: Int?): Int? {
+    return i!!<caret>
+}
@@ -0,0 +1,4 @@
+// PROBLEM: none
+fun test(i: Int?): Int? {
+    return (i!!<caret>)
+}
@@ -0,0 +1,5 @@
+fun test(a: Int?): Int? {
+    return check(a!!<caret>)
+}
+
+fun check(i: Int): Int = i
@@ -0,0 +1,5 @@
+fun test(a: Int?): Int? {
+    return check(a ?: return null)
+}
+
+fun check(i: Int): Int = i
@@ -0,0 +1,4 @@
+// PROBLEM: none
+fun test(i: Int) {
+    val x = i!!<caret>
+}
@@ -7117,6 +7117,79 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class ReplaceNotNullAssertionWithElvisReturn extends AbstractLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInReplaceNotNullAssertionWithElvisReturn() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("basic.kt")
+        public void testBasic() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/basic.kt");
+        }
+
+        @TestMetadata("basic2.kt")
+        public void testBasic2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/basic2.kt");
+        }
+
+        @TestMetadata("basic3.kt")
+        public void testBasic3() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/basic3.kt");
+        }
+
+        @TestMetadata("basic4.kt")
+        public void testBasic4() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/basic4.kt");
+        }
+
+        @TestMetadata("basic5.kt")
+        public void testBasic5() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/basic5.kt");
+        }
+
+        @TestMetadata("inLambda.kt")
+        public void testInLambda() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/inLambda.kt");
+        }
+
+        @TestMetadata("inLambda2.kt")
+        public void testInLambda2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/inLambda2.kt");
+        }
+
+        @TestMetadata("notNullFunction.kt")
+        public void testNotNullFunction() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/notNullFunction.kt");
+        }
+
+        @TestMetadata("onReturn.kt")
+        public void testOnReturn() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/onReturn.kt");
+        }
+
+        @TestMetadata("onReturn2.kt")
+        public void testOnReturn2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/onReturn2.kt");
+        }
+
+        @TestMetadata("onReturn3.kt")
+        public void testOnReturn3() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/onReturn3.kt");
+        }
+
+        @TestMetadata("unnecessaryNotNullAssertion.kt")
+        public void testUnnecessaryNotNullAssertion() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceNotNullAssertionWithElvisReturn/unnecessaryNotNullAssertion.kt");
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/replacePutWithAssignment")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.ReplaceNotNullAssertionWithElvisReturnInspection`