Add inspection to detect use of callable reference as a lambda body

So #KT-17053 Fixed
2017-04-20 00:03:06 +09:00
parent 53e11cbeb5
commit 0eceef1519
21 changed files with 292 additions and 5 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports a lambda expression with one callable reference because it is a common error to replace a lambda with a callable reference without changing curly braces to parentheses.
+</body>
+</html>
@@ -2128,6 +2128,13 @@
                     language="JAVA"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.MoveSuspiciousCallableReferenceIntoParenthesesInspection"
+                     displayName="Suspicious callable reference used as lambda result"
+                     groupName="Kotlin"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />

    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

@@ -0,0 +1,65 @@
+/*
+ * 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.IntentionWrapper
+import com.intellij.codeInspection.LocalInspectionToolSession
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.intentions.ConvertLambdaToReferenceIntention
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
+import org.jetbrains.kotlin.psi.KtLambdaExpression
+import org.jetbrains.kotlin.psi.KtVisitorVoid
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+
+class MoveSuspiciousCallableReferenceIntoParenthesesInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
+        return object : KtVisitorVoid() {
+            override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) {
+                val callableReference = lambdaExpression.bodyExpression?.statements?.singleOrNull() as? KtCallableReferenceExpression
+                if (callableReference != null) {
+                    holder.registerProblem(
+                            lambdaExpression,
+                            "Suspicious callable reference as the only lambda element",
+                            ProblemHighlightType.WEAK_WARNING,
+                            IntentionWrapper(MoveIntoParenthesesIntention(), lambdaExpression.containingFile)
+                    )
+
+                }
+            }
+        }
+    }
+
+    class MoveIntoParenthesesIntention : ConvertLambdaToReferenceIntention(
+            "Move suspicious callable reference into parentheses '()'"
+    ) {
+        override fun buildReferenceText(element: KtLambdaExpression): String? {
+            val callableReferenceExpression =
+                    element.bodyExpression?.statements?.singleOrNull() as? KtCallableReferenceExpression ?: return null
+            val callableReference = callableReferenceExpression.callableReference
+            val resolvedCall = callableReference.getResolvedCall(callableReference.analyze())
+            val receiverValue = resolvedCall?.let { it.extensionReceiver ?: it.dispatchReceiver }
+            return (receiverValue?.let { "${it.type}" } ?: "") + "::${callableReference.text}"
+        }
+
+        override fun isApplicableTo(element: KtLambdaExpression) = true
+    }
+}
+
@@ -47,10 +47,15 @@ class ConvertLambdaToReferenceInspection : IntentionBasedInspection<KtLambdaExpr
    override fun inspectionTarget(element: KtLambdaExpression) = element.bodyExpression?.statements?.singleOrNull()
 }

-class ConvertLambdaToReferenceIntention : SelfTargetingOffsetIndependentIntention<KtLambdaExpression>(
-        KtLambdaExpression::class.java, "Convert lambda to reference"
-) {
-    private fun KtLambdaArgument.outerCalleeDescriptor(): FunctionDescriptor? {
+open class ConvertLambdaToReferenceIntention(text: String) :
+        SelfTargetingOffsetIndependentIntention<KtLambdaExpression>(KtLambdaExpression::class.java, text) {
+
+    @Suppress("unused")
+    constructor(): this("Convert lambda to reference")
+
+    open fun buildReferenceText(element: KtLambdaExpression) = buildReferenceText(lambdaExpression = element, shortTypes = false)
+
+    protected fun KtLambdaArgument.outerCalleeDescriptor(): FunctionDescriptor? {
        val outerCallExpression = parent as? KtCallExpression ?: return null
        val context = outerCallExpression.analyze()
        val outerCallee = outerCallExpression.calleeExpression as? KtReferenceExpression ?: return null
@@ -163,7 +168,7 @@ class ConvertLambdaToReferenceIntention : SelfTargetingOffsetIndependentIntentio
    }

    override fun applyTo(element: KtLambdaExpression, editor: Editor?) {
-        val referenceName = buildReferenceText(lambdaExpression = element, shortTypes = false) ?: return
+        val referenceName = buildReferenceText(element) ?: return
        val factory = KtPsiFactory(element)
        val lambdaArgument = element.parent as? KtLambdaArgument
        if (lambdaArgument == null) {
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.MoveSuspiciousCallableReferenceIntoParenthesesInspection
@@ -0,0 +1,6 @@
+// PROBLEM: Suspicious callable reference as the only lambda element
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map {<caret> it::toString }
+}
@@ -0,0 +1,6 @@
+// PROBLEM: Suspicious callable reference as the only lambda element
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map(Int::toString)
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map {<caret> it::toString }
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map(Int::toString)
+}
@@ -0,0 +1,13 @@
+// WITH_RUNTIME
+
+fun foo() {
+    x(1) {<caret> ::y }
+}
+
+fun y() {
+
+}
+
+fun x(number: Int, func: () -> Unit) {
+
+}
@@ -0,0 +1,13 @@
+// WITH_RUNTIME
+
+fun foo() {
+    x(1, ::y)
+}
+
+fun y() {
+
+}
+
+fun x(number: Int, func: () -> Unit) {
+
+}
@@ -0,0 +1,9 @@
+// WITH_RUNTIME
+// PROBLEM: none
+
+fun foo() {
+    listOf(1,2,3).map {<caret>
+        println(it)
+        Int::toString
+    }
+}
@@ -0,0 +1,6 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map {<caret>}
+}
@@ -0,0 +1,6 @@
+// PROBLEM: none
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map {<caret> println(it) }
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map {<caret> Int::toString }
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map(Int::toString)
+}
@@ -0,0 +1,6 @@
+// FIX: Move suspicious callable reference into parentheses '()'
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map {<caret> bar -> bar::toString }
+}
@@ -0,0 +1,6 @@
+// FIX: Move suspicious callable reference into parentheses '()'
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map(Int::toString)
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map { bar -> bar::toString }
+    listOf(4,5).map {<caret> bar -> bar::toString }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+
+fun foo() {
+    listOf(1,2,3).map { bar -> bar::toString }
+    listOf(4,5).map(Int::toString)
+}
@@ -0,0 +1,101 @@
+/*
+ * 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.testFramework.TestDataPath;
+import org.jetbrains.kotlin.test.JUnit3RunnerWithInners;
+import org.jetbrains.kotlin.test.KotlinTestUtils;
+import org.jetbrains.kotlin.test.TargetBackend;
+import org.jetbrains.kotlin.test.TestMetadata;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.regex.Pattern;
+
+/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
+@SuppressWarnings("all")
+@TestMetadata("idea/testData/inspectionsLocal")
+@TestDataPath("$PROJECT_ROOT")
+@RunWith(JUnit3RunnerWithInners.class)
+public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
+    public void testAllFilesPresentInInspectionsLocal() throws Exception {
+        KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+    }
+
+    @TestMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class MoveSuspiciousCallableReferenceIntoParentheses extends AbstractLocalInspectionTest {
+        public void testAllFilesPresentInMoveSuspiciousCallableReferenceIntoParentheses() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("defaultParameter.kt")
+        public void testDefaultParameter() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/defaultParameter.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("it.kt")
+        public void testIt() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/it.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("lambdaWithArg.kt")
+        public void testLambdaWithArg() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/lambdaWithArg.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("multipleLines.kt")
+        public void testMultipleLines() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/multipleLines.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("noBody.kt")
+        public void testNoBody() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/noBody.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("noneCallableRef.kt")
+        public void testNoneCallableRef() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/noneCallableRef.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("normal.kt")
+        public void testNormal() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/normal.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("parameter.kt")
+        public void testParameter() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/parameter.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("parameter2.kt")
+        public void testParameter2() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/moveSuspiciousCallableReferenceIntoParentheses/parameter2.kt");
+            doTest(fileName);
+        }
+    }
+}
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.MoveSuspiciousCallableReferenceIntoParenthesesInspection`