Introduce "Useless call on collection type" inspection

Related to KT-12165 Supported functions: filterNotNull, filterIsInstance, mapNotNull, mapNotNullTo, mapIndexedNotNull, mapIndexedNotNullTo Also, "Useless cal on not-null" improved a bit
2017-06-19 16:00:30 +03:00
parent f80f41d254
commit bdb9f00c75
32 changed files with 411 additions and 73 deletions
@@ -2258,6 +2258,14 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.collections.UselessCallOnCollectionInspection"
+                     displayName="Useless call on collection type"
+                     groupName="Kotlin"
+                     enabledByDefault="true"
+                     level="WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,69 @@
+/*
+ * 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.collections
+
+import com.intellij.codeInspection.ProblemsHolder
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.inspections.AbstractKotlinInspection
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtQualifiedExpression
+import org.jetbrains.kotlin.psi.KtVisitorVoid
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+
+abstract class AbstractUselessCallInspection : AbstractKotlinInspection() {
+
+    protected abstract val uselessFqNames: Map<String, Conversion>
+
+    protected abstract val uselessNames: Set<String>
+
+    protected abstract fun QualifiedExpressionVisitor.suggestConversionIfNeeded(
+            expression: KtQualifiedExpression,
+            calleeExpression: KtExpression,
+            context: BindingContext,
+            conversion: Conversion
+    )
+
+    inner class QualifiedExpressionVisitor internal constructor(val holder: ProblemsHolder, val isOnTheFly: Boolean) : KtVisitorVoid() {
+        override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
+            super.visitQualifiedExpression(expression)
+            val selector = expression.selectorExpression as? KtCallExpression ?: return
+            val calleeExpression = selector.calleeExpression ?: return
+            if (calleeExpression.text !in uselessNames) return
+
+            val context = expression.analyze(BodyResolveMode.PARTIAL)
+            val resolvedCall = expression.getResolvedCall(context) ?: return
+            val conversion = uselessFqNames[resolvedCall.resultingDescriptor.fqNameOrNull()?.asString()] ?: return
+
+            suggestConversionIfNeeded(expression, calleeExpression, context, conversion)
+        }
+    }
+
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = QualifiedExpressionVisitor(holder, isOnTheFly)
+
+
+    protected companion object {
+        data class Conversion(val replacementName: String? = null)
+
+        val deleteConversion = Conversion()
+
+        fun Set<String>.toShortNames() = mapTo(mutableSetOf()) { fqName -> fqName.takeLastWhile { it != '.' } }
+    }
+}
@@ -20,10 +20,9 @@ import com.intellij.codeInspection.LocalQuickFix
 import com.intellij.codeInspection.ProblemDescriptor
 import com.intellij.openapi.project.Project
 import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.KtPsiFactory
 import org.jetbrains.kotlin.psi.KtQualifiedExpression
-import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression
-import org.jetbrains.kotlin.psi.createExpressionByPattern

 class RenameUselessCallFix(val newName: String) : LocalQuickFix {
    override fun getName() = "Rename useless call to '$newName'"
@@ -33,8 +32,9 @@ class RenameUselessCallFix(val newName: String) : LocalQuickFix {
    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
        (descriptor.psiElement as? KtQualifiedExpression)?.let {
            val factory = KtPsiFactory(it)
-            val sign = if (it is KtSafeQualifiedExpression) "?." else "."
-            it.replaced(factory.createExpressionByPattern("$0$sign$newName()", it.receiverExpression))
+            val selectorCallExpression = it.selectorExpression as? KtCallExpression
+            val calleeExpression = selectorCallExpression?.calleeExpression ?: return
+            calleeExpression.replaced(factory.createExpression(newName))
        }
    }
 }
@@ -0,0 +1,100 @@
+/*
+ * 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.collections
+
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.openapi.util.TextRange
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns
+import org.jetbrains.kotlin.builtins.getFunctionalClassKind
+import org.jetbrains.kotlin.builtins.isFunctionType
+import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtQualifiedExpression
+import org.jetbrains.kotlin.psi.psiUtil.endOffset
+import org.jetbrains.kotlin.psi.psiUtil.startOffset
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.calls.callUtil.getType
+import org.jetbrains.kotlin.types.TypeUtils
+import org.jetbrains.kotlin.types.isFlexible
+import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
+
+class UselessCallOnCollectionInspection : AbstractUselessCallInspection() {
+    override val uselessFqNames = mapOf("kotlin.collections.filterNotNull" to deleteConversion,
+                                        "kotlin.collections.filterIsInstance" to deleteConversion,
+                                        "kotlin.collections.mapNotNull" to Conversion("map"),
+                                        "kotlin.collections.mapNotNullTo" to Conversion("mapTo"),
+                                        "kotlin.collections.mapIndexedNotNull" to Conversion("mapIndexed"),
+                                        "kotlin.collections.mapIndexedNotNullTo" to Conversion("mapIndexedTo"))
+
+    override val uselessNames = uselessFqNames.keys.toShortNames()
+
+    override fun QualifiedExpressionVisitor.suggestConversionIfNeeded(
+            expression: KtQualifiedExpression,
+            calleeExpression: KtExpression,
+            context: BindingContext,
+            conversion: Conversion
+    ) {
+        val receiverType = expression.receiverExpression.getType(context) ?: return
+        val receiverTypeArgument = receiverType.arguments.singleOrNull()?.type ?: return
+        val resolvedCall = expression.getResolvedCall(context) ?: return
+        if (calleeExpression.text == "filterIsInstance") {
+            val typeParameterDescriptor = resolvedCall.candidateDescriptor.typeParameters.singleOrNull() ?: return
+            val argumentType = resolvedCall.typeArguments[typeParameterDescriptor] ?: return
+            if (receiverTypeArgument.isFlexible() || !receiverTypeArgument.isSubtypeOf(argumentType)) return
+        }
+        else {
+            // xxxNotNull
+            if (TypeUtils.isNullableType(receiverTypeArgument)) return
+            if (calleeExpression.text != "filterNotNull") {
+                // Also check last argument functional type to have not-null result
+                val lastParameter = resolvedCall.resultingDescriptor.valueParameters.lastOrNull() ?: return
+                val lastArgument = resolvedCall.valueArguments[lastParameter]?.arguments?.singleOrNull() ?: return
+                val functionalType = lastArgument.getArgumentExpression()?.getType(context) ?: return
+                // Both Function & KFunction must pass here
+                if (functionalType.constructor.declarationDescriptor?.getFunctionalClassKind() == null) return
+                val resultType = functionalType.arguments.lastOrNull()?.type ?: return
+                if (TypeUtils.isNullableType(resultType)) return
+            }
+        }
+
+        val newName = conversion.replacementName
+        if (newName != null) {
+            val descriptor = holder.manager.createProblemDescriptor(
+                    expression,
+                    TextRange(expression.operationTokenNode.startOffset - expression.startOffset,
+                              calleeExpression.endOffset - expression.startOffset),
+                    "Call on collection type may be reduced",
+                    ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                    isOnTheFly,
+                    RenameUselessCallFix(newName)
+            )
+            holder.registerProblem(descriptor)
+        }
+        else {
+            val descriptor = holder.manager.createProblemDescriptor(
+                    expression,
+                    TextRange(expression.operationTokenNode.startOffset - expression.startOffset,
+                              calleeExpression.endOffset - expression.startOffset),
+                    "Useless call on collection type",
+                    ProblemHighlightType.LIKE_UNUSED_SYMBOL,
+                    isOnTheFly,
+                    RemoveUselessCallFix()
+            )
+            holder.registerProblem(descriptor)
+        }
+    }
+}
@@ -18,88 +18,70 @@ package org.jetbrains.kotlin.idea.inspections.collections

 import com.intellij.codeInspection.IntentionWrapper
 import com.intellij.codeInspection.ProblemHighlightType
-import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.openapi.util.TextRange
-import com.intellij.psi.PsiElementVisitor
-import org.jetbrains.kotlin.idea.caches.resolve.analyze
-import org.jetbrains.kotlin.idea.inspections.AbstractKotlinInspection
 import org.jetbrains.kotlin.idea.quickfix.ReplaceWithDotCallFix
-import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtExpression
 import org.jetbrains.kotlin.psi.KtQualifiedExpression
 import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression
-import org.jetbrains.kotlin.psi.KtVisitorVoid
 import org.jetbrains.kotlin.psi.psiUtil.endOffset
 import org.jetbrains.kotlin.psi.psiUtil.startOffset
-import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.calls.callUtil.getType
-import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
-import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.types.TypeUtils

-class UselessCallOnNotNullInspection : AbstractKotlinInspection() {
+class UselessCallOnNotNullInspection : AbstractUselessCallInspection() {
+    override val uselessFqNames = mapOf("kotlin.collections.orEmpty" to deleteConversion,
+                                        "kotlin.text.orEmpty" to deleteConversion,
+                                        "kotlin.text.isNullOrEmpty" to Conversion("isEmpty"),
+                                        "kotlin.text.isNullOrBlank" to Conversion("isBlank"))

-    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
-        return object : KtVisitorVoid() {
-            override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
-                super.visitQualifiedExpression(expression)
-                val selector = expression.selectorExpression as? KtCallExpression ?: return
-                val calleeExpression = selector.calleeExpression ?: return
-                if (calleeExpression.text !in names) return
+    override val uselessNames = uselessFqNames.keys.toShortNames()

-                val context = expression.analyze(BodyResolveMode.PARTIAL)
-                val resolvedCall = expression.getResolvedCall(context) ?: return
-                val conversion = fqNames[resolvedCall.resultingDescriptor.fqNameOrNull()?.asString()] ?: return
-                val newName = conversion.replacementName
+    override fun QualifiedExpressionVisitor.suggestConversionIfNeeded(
+            expression: KtQualifiedExpression,
+            calleeExpression: KtExpression,
+            context: BindingContext,
+            conversion: Conversion
+    ) {
+        val newName = conversion.replacementName

-                val safeExpression = expression as? KtSafeQualifiedExpression
-                val notNullType = expression.receiverExpression.getType(context)?.let { TypeUtils.isNullableType(it) } == false
-                if (newName != null && (notNullType || safeExpression != null)) {
-                    val descriptor = holder.manager.createProblemDescriptor(
-                            expression,
-                            TextRange(expression.operationTokenNode.startOffset - expression.startOffset,
-                                      calleeExpression.endOffset - expression.startOffset),
-                            "Call on not-null type may be reduced",
-                            ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
-                            isOnTheFly,
-                            RenameUselessCallFix(newName)
-                    )
-                    holder.registerProblem(descriptor)
-                }
-                else if (notNullType) {
-                    val descriptor = holder.manager.createProblemDescriptor(
-                            expression,
-                            TextRange(expression.operationTokenNode.startOffset - expression.startOffset,
-                                      calleeExpression.endOffset - expression.startOffset),
-                            "Useless call on not-null type",
-                            ProblemHighlightType.LIKE_UNUSED_SYMBOL,
-                            isOnTheFly,
-                            RemoveUselessCallFix()
-                    )
-                    holder.registerProblem(descriptor)
-                }
-                else if (safeExpression != null) {
-                    holder.registerProblem(
-                            safeExpression.operationTokenNode.psi,
-                            "This call is useless with ?.",
-                            ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
-                            IntentionWrapper(ReplaceWithDotCallFix(safeExpression), safeExpression.containingKtFile)
-                    )
-                }
-            }
+        val safeExpression = expression as? KtSafeQualifiedExpression
+        val notNullType = expression.receiverExpression.getType(context)?.let { TypeUtils.isNullableType(it) } == false
+        val defaultRange = TextRange(expression.operationTokenNode.startOffset, calleeExpression.endOffset)
+                .shiftRight(-expression.startOffset)
+        if (newName != null && (notNullType || safeExpression != null)) {
+            val fixes = listOf(RenameUselessCallFix(newName)) + listOfNotNull(safeExpression?.let {
+                IntentionWrapper(ReplaceWithDotCallFix(safeExpression), safeExpression.containingKtFile)
+            })
+            val descriptor = holder.manager.createProblemDescriptor(
+                    expression,
+                    defaultRange,
+                    "Call on not-null type may be reduced",
+                    ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                    isOnTheFly,
+                    *fixes.toTypedArray()
+            )
+            holder.registerProblem(descriptor)
+        }
+        else if (notNullType) {
+            val descriptor = holder.manager.createProblemDescriptor(
+                    expression,
+                    defaultRange,
+                    "Useless call on not-null type",
+                    ProblemHighlightType.LIKE_UNUSED_SYMBOL,
+                    isOnTheFly,
+                    RemoveUselessCallFix()
+            )
+            holder.registerProblem(descriptor)
+        }
+        else if (safeExpression != null) {
+            holder.registerProblem(
+                    safeExpression.operationTokenNode.psi,
+                    "This call is useless with ?.",
+                    ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                    IntentionWrapper(ReplaceWithDotCallFix(safeExpression), safeExpression.containingKtFile)
+            )
        }
-    }
-
-    companion object {
-        private data class Conversion(val replacementName: String? = null)
-
-        private val deleteConversion = Conversion()
-
-        private val fqNames = mapOf("kotlin.collections.orEmpty" to deleteConversion,
-                                    "kotlin.text.orEmpty" to deleteConversion,
-                                    "kotlin.text.isNullOrEmpty" to Conversion("isEmpty"),
-                                    "kotlin.text.isNullOrBlank" to Conversion("isBlank"))
-
-        private val names = fqNames.keys.mapTo(mutableSetOf()) { fqName -> fqName.takeLastWhile { it != '.' } }
    }
 }