Introduce "add parameter names to following arguments"

This commit also refactors relevant intentions & inspections, like "add name to argument", "add names to call arguments", "unnamed boolean literal argument" #KT-30622 Fixed
2019-05-06 15:45:51 +03:00
parent f26a4e06bd
commit 955bfd6e7b
21 changed files with 189 additions and 77 deletions
@@ -1261,6 +1261,11 @@
      <category>Kotlin</category>
    </intentionAction>

+    <intentionAction>
+      <className>org.jetbrains.kotlin.idea.intentions.AddNamesToFollowingArgumentsIntention</className>
+      <category>Kotlin</category>
+    </intentionAction>
+
    <intentionAction>
      <className>org.jetbrains.kotlin.idea.intentions.ReplaceUnderscoreWithParameterNameIntention</className>
      <category>Kotlin</category>
@@ -0,0 +1,7 @@
+fun foo(x: Int, y: Int, comment: String) {
+
+}
+
+fun test() {
+    foo(24, <spot>y = 42, comment = "Hello"</spot>)
+}
@@ -0,0 +1,7 @@
+fun foo(x: Int, y: Int, comment: String) {
+
+}
+
+fun test() {
+    foo(24, <spot>42, "Hello"</spot>)
+}
@@ -0,0 +1,6 @@
+<html>
+<body>
+This intention adds names to all positional arguments of a function call according to the named argument syntax,
+starting with the current argument.
+</body>
+</html>
@@ -6,54 +6,71 @@
 package org.jetbrains.kotlin.idea.inspections

 import com.intellij.codeInspection.IntentionWrapper
-import com.intellij.codeInspection.LocalQuickFix
-import com.intellij.codeInspection.ProblemDescriptor
 import com.intellij.codeInspection.ProblemHighlightType.GENERIC_ERROR_OR_WARNING
 import com.intellij.codeInspection.ProblemHighlightType.INFORMATION
 import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel
-import com.intellij.openapi.project.Project
 import org.jetbrains.kotlin.KtNodeTypes
 import org.jetbrains.kotlin.diagnostics.Severity
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
 import org.jetbrains.kotlin.idea.intentions.AddNameToArgumentIntention
-import org.jetbrains.kotlin.psi.KtCallExpression
-import org.jetbrains.kotlin.psi.KtConstantExpression
-import org.jetbrains.kotlin.psi.KtValueArgument
+import org.jetbrains.kotlin.idea.intentions.AddNamesToCallArgumentsIntention
+import org.jetbrains.kotlin.idea.intentions.AddNamesToFollowingArgumentsIntention
+import org.jetbrains.kotlin.idea.project.languageVersionSettings
+import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
-import org.jetbrains.kotlin.psi.valueArgumentVisitor
 import javax.swing.JComponent

 class BooleanLiteralArgumentInspection(
    @JvmField var reportSingle: Boolean = false
 ) : AbstractKotlinInspection() {
+    private fun KtExpression.isBooleanLiteral(): Boolean =
+        this is KtConstantExpression && node.elementType == KtNodeTypes.BOOLEAN_CONSTANT
+
+    private fun KtValueArgument.isUnnamedBooleanLiteral(): Boolean =
+        !isNamed() && getArgumentExpression()?.isBooleanLiteral() == true
+
    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) =
        valueArgumentVisitor(fun(argument: KtValueArgument) {
-            if (argument.getArgumentName() != null) return
-            val argumentExpression = argument.getArgumentExpression() as? KtConstantExpression ?: return
-            if (argumentExpression.node.elementType != KtNodeTypes.BOOLEAN_CONSTANT) return
+            if (argument.isNamed()) return
+            val argumentExpression = argument.getArgumentExpression() ?: return
+            if (!argumentExpression.isBooleanLiteral()) return
            val call = argument.getStrictParentOfType<KtCallExpression>() ?: return
            val valueArguments = call.valueArguments
-            if (valueArguments.takeLastWhile { it != argument }.any { !it.isNamed() }) return

            if (argumentExpression.analyze().diagnostics.forElement(argumentExpression).any { it.severity == Severity.ERROR }) return
-            if (AddNameToArgumentIntention.detectNameToAdd(argument) == null) return
+            if (AddNameToArgumentIntention.detectNameToAdd(argument, shouldBeLastUnnamed = false) == null) return

-            val hasPreviousUnnamedBoolean = valueArguments.asSequence().windowed(size = 2, step = 1).any { (prev, next) ->
-                next == argument && !prev.isNamed() &&
-                        (prev.getArgumentExpression() as? KtConstantExpression)?.node?.elementType == KtNodeTypes.BOOLEAN_CONSTANT
+            val resolvedCall = call.resolveToCall() ?: return
+            if (!resolvedCall.resultingDescriptor.hasStableParameterNames()) return
+            val languageVersionSettings = call.languageVersionSettings
+            if (valueArguments.any {
+                    !AddNameToArgumentIntention.argumentMatchedAndCouldBeNamedInCall(it, resolvedCall, languageVersionSettings)
+                }
+            ) return
+
+            val highlightType = if (reportSingle) {
+                GENERIC_ERROR_OR_WARNING
+            } else {
+                val hasNeighbourUnnamedBoolean = valueArguments.asSequence().windowed(size = 2, step = 1).any { (prev, next) ->
+                    prev == argument && next.isUnnamedBooleanLiteral() ||
+                            next == argument && prev.isUnnamedBooleanLiteral()
+                }
+                if (hasNeighbourUnnamedBoolean) GENERIC_ERROR_OR_WARNING else INFORMATION
            }
-            val fixes = mutableListOf<LocalQuickFix>()
-            if (hasPreviousUnnamedBoolean) {
-                fixes += AddNamesToLastBooleanArgumentsFix()
+            val fix = if (argument != valueArguments.lastOrNull { !it.isNamed() }) {
+                if (argument == valueArguments.firstOrNull()) {
+                    IntentionWrapper(AddNamesToCallArgumentsIntention(), argument.containingKtFile)
+                } else {
+                    IntentionWrapper(AddNamesToFollowingArgumentsIntention(), argument.containingKtFile)
+                }
+            } else {
+                IntentionWrapper(AddNameToArgumentIntention(), argument.containingKtFile)
            }
-            fixes += IntentionWrapper(AddNameToArgumentIntention(), argument.containingKtFile)
            holder.registerProblemWithoutOfflineInformation(
-                argument,
-                "Boolean literal argument without parameter name",
-                isOnTheFly,
-                if (reportSingle || hasPreviousUnnamedBoolean) GENERIC_ERROR_OR_WARNING else INFORMATION,
-                *fixes.toTypedArray()
+                argument, "Boolean literal argument without parameter name",
+                isOnTheFly, highlightType, fix
            )
        })

@@ -62,28 +79,4 @@ class BooleanLiteralArgumentInspection(
        panel.addCheckbox("Report also on call with single boolean literal argument", "reportSingle")
        return panel
    }
-
-    private class AddNamesToLastBooleanArgumentsFix : LocalQuickFix {
-        override fun getFamilyName(): String = name
-
-        override fun getName() = "Add names to boolean arguments"
-
-        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
-            val argument = descriptor.psiElement as? KtValueArgument ?: return
-            val call = argument.getStrictParentOfType<KtCallExpression>() ?: return
-            val valueArguments = call.valueArguments
-
-            var problemArgumentFound = false
-            for (currentArgument in valueArguments.reversed()) {
-                if (currentArgument == argument) {
-                    problemArgumentFound = true
-                } else if (!problemArgumentFound) continue
-                if (currentArgument.isNamed()) return
-                if ((currentArgument.getArgumentExpression() as? KtConstantExpression)?.node?.elementType != KtNodeTypes.BOOLEAN_CONSTANT) {
-                    return
-                }
-                if (!AddNameToArgumentIntention.apply(currentArgument)) return
-            }
-        }
-    }
 }
@@ -40,7 +40,7 @@ class AddNameToArgumentIntention : SelfTargetingIntention<KtValueArgument>(

    override fun isApplicableTo(element: KtValueArgument, caretOffset: Int): Boolean {
        val expression = element.getArgumentExpression() ?: return false
-        val name = detectNameToAdd(element) ?: return false
+        val name = detectNameToAdd(element, shouldBeLastUnnamed = true) ?: return false

        text = "Add '$name =' to argument"

@@ -60,23 +60,23 @@ class AddNameToArgumentIntention : SelfTargetingIntention<KtValueArgument>(
    }

    companion object {
-        fun apply(element: KtValueArgument): Boolean {
-            val name = detectNameToAdd(element) ?: return false
+        fun apply(element: KtValueArgument, givenResolvedCall: ResolvedCall<*>? = null): Boolean {
+            val name = detectNameToAdd(element, shouldBeLastUnnamed = false, givenResolvedCall = givenResolvedCall) ?: return false
            val argumentExpression = element.getArgumentExpression() ?: return false
            val newArgument = KtPsiFactory(element).createArgument(argumentExpression, name, element.getSpreadElement() != null)
            element.replace(newArgument)
            return true
        }

-        fun detectNameToAdd(argument: KtValueArgument): Name? {
+        fun detectNameToAdd(argument: KtValueArgument, shouldBeLastUnnamed: Boolean, givenResolvedCall: ResolvedCall<*>? = null): Name? {
            if (argument.isNamed()) return null
            if (argument is KtLambdaArgument) return null

            val argumentList = argument.parent as? KtValueArgumentList ?: return null
-            if (argument != argumentList.arguments.last { !it.isNamed() }) return null
+            if (shouldBeLastUnnamed && argument != argumentList.arguments.last { !it.isNamed() }) return null

            val callExpr = argumentList.parent as? KtCallElement ?: return null
-            val resolvedCall = callExpr.resolveToCall() ?: return null
+            val resolvedCall = givenResolvedCall ?: callExpr.resolveToCall() ?: return null
            if (!resolvedCall.resultingDescriptor.hasStableParameterNames()) return null

            if (!argumentMatchedAndCouldBeNamedInCall(argument, resolvedCall, callExpr.languageVersionSettings)) return null
@@ -21,7 +21,8 @@ import com.intellij.openapi.util.TextRange
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
 import org.jetbrains.kotlin.idea.project.languageVersionSettings
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.resolve.calls.model.ArgumentMatch
+import org.jetbrains.kotlin.psi.psiUtil.endOffset
+import org.jetbrains.kotlin.psi.psiUtil.startOffset

 class AddNamesToCallArgumentsIntention : SelfTargetingRangeIntention<KtCallElement>(
    KtCallElement::class.java,
@@ -38,7 +39,10 @@ class AddNamesToCallArgumentsIntention : SelfTargetingRangeIntention<KtCallEleme
                AddNameToArgumentIntention.argumentMatchedAndCouldBeNamedInCall(it, resolvedCall, element.languageVersionSettings)
            }
        ) {
-            return element.calleeExpression?.textRange
+            val calleeExpression = element.calleeExpression ?: return null
+            if (arguments.size < 2) return calleeExpression.textRange
+            val endOffset = (arguments.firstOrNull() as? KtValueArgument)?.endOffset ?: return null
+            return TextRange(calleeExpression.startOffset, endOffset)
        }

        return null
@@ -49,14 +53,7 @@ class AddNamesToCallArgumentsIntention : SelfTargetingRangeIntention<KtCallEleme
        val resolvedCall = element.resolveToCall() ?: return
        for (argument in arguments) {
            if (argument !is KtValueArgument || argument is KtLambdaArgument) continue
-            val argumentMatch = resolvedCall.getArgumentMapping(argument) as? ArgumentMatch ?: continue
-            val name = argumentMatch.valueParameter.name
-            val newArgument = KtPsiFactory(element).createArgument(
-                argument.getArgumentExpression()!!,
-                name,
-                argument.getSpreadElement() != null
-            )
-            argument.replace(newArgument)
+            AddNameToArgumentIntention.apply(argument, resolvedCall)
        }
    }
 }
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * 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.intentions
+
+import com.intellij.codeInsight.intention.LowPriorityAction
+import com.intellij.openapi.editor.Editor
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
+import org.jetbrains.kotlin.idea.conversion.copy.end
+import org.jetbrains.kotlin.idea.conversion.copy.start
+import org.jetbrains.kotlin.psi.KtCallElement
+import org.jetbrains.kotlin.psi.KtLambdaExpression
+import org.jetbrains.kotlin.psi.KtValueArgument
+import org.jetbrains.kotlin.psi.KtValueArgumentList
+
+class AddNamesToFollowingArgumentsIntention : SelfTargetingIntention<KtValueArgument>(
+    KtValueArgument::class.java, "Add names to this argument and following arguments"
+), LowPriorityAction {
+    override fun isApplicableTo(element: KtValueArgument, caretOffset: Int): Boolean {
+        val argumentList = element.parent as? KtValueArgumentList ?: return false
+        // Shadowed by AddNamesToCallArguments
+        if (argumentList.arguments.firstOrNull() == element) return false
+        // Shadowed by AddNameToArgument
+        if (argumentList.arguments.lastOrNull { !it.isNamed() } == element) return false
+        val expression = element.getArgumentExpression() ?: return false
+        AddNameToArgumentIntention.detectNameToAdd(element, shouldBeLastUnnamed = false) ?: return false
+
+        if (expression is KtLambdaExpression) {
+            val range = expression.textRange
+            return caretOffset == range.start || caretOffset == range.end
+        }
+
+        return true
+    }
+
+    override fun applyTo(element: KtValueArgument, editor: Editor?) {
+        val argumentList = element.parent as? KtValueArgumentList ?: return
+        val callElement = argumentList.parent as? KtCallElement ?: return
+        val resolvedCall = callElement.resolveToCall() ?: return
+        for (argument in argumentList.arguments.dropWhile { it != element }) {
+            AddNameToArgumentIntention.apply(argument, resolvedCall)
+        }
+    }
+}
@@ -1,7 +1,7 @@
 // HIGHLIGHT: GENERIC_ERROR_OR_WARNING
-// FIX: Add names to boolean arguments
+// FIX: Add names to call arguments
 fun foo(a: Boolean, b: Boolean, c: Boolean) {}

 fun test() {
-    foo(true, true, true<caret>)
+    foo(true<caret>, true, true)
 }
@@ -1,5 +1,5 @@
 // HIGHLIGHT: GENERIC_ERROR_OR_WARNING
-// FIX: Add names to boolean arguments
+// FIX: Add names to call arguments
 fun foo(a: Boolean, b: Boolean, c: Boolean) {}

 fun test() {
@@ -1,7 +1,7 @@
 // HIGHLIGHT: GENERIC_ERROR_OR_WARNING
-// FIX: Add names to boolean arguments
+// FIX: Add names to call arguments
 fun foo(a: Boolean, b: Boolean, c: Boolean) {}

 fun test() {
-    foo(true, <caret>true, c = true)
+    foo(<caret>true, true, c = true)
 }
@@ -1,5 +1,5 @@
 // HIGHLIGHT: GENERIC_ERROR_OR_WARNING
-// FIX: Add names to boolean arguments
+// FIX: Add names to call arguments
 fun foo(a: Boolean, b: Boolean, c: Boolean) {}

 fun test() {
@@ -1,5 +1,5 @@
 fun foo(s: String, b: Boolean){}

 fun bar() {
-    <caret>foo("", true)
+    foo(<caret>"", true)
 }
@@ -1,5 +1,5 @@
 fun foo(s: String, b: Boolean){}

 fun bar() {
-    <caret>foo(s = "", b = true)
+    foo(<caret>s = "", b = true)
 }
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.intentions.AddNamesToFollowingArgumentsIntention
@@ -0,0 +1,6 @@
+// IS_APPLICABLE: false
+fun foo(first: Int, second: Boolean, last: String) {}
+
+fun test() {
+    foo(<caret>1, true, "")
+}
@@ -0,0 +1,6 @@
+// IS_APPLICABLE: false
+fun foo(first: Int, second: Boolean, last: String) {}
+
+fun test() {
+    foo(1, true, <caret>"")
+}
@@ -0,0 +1,5 @@
+fun foo(first: Int, second: Boolean, last: String) {}
+
+fun test() {
+    foo(1, <caret>true, "")
+}
@@ -0,0 +1,5 @@
+fun foo(first: Int, second: Boolean, last: String) {}
+
+fun test() {
+    foo(1, second = true, last = "")
+}
@@ -1314,11 +1314,6 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
            runTest("idea/testData/intentions/addNamesToCallArguments/javaMethod.kt");
        }

-        @TestMetadata("notOnCallee.kt")
-        public void testNotOnCallee() throws Exception {
-            runTest("idea/testData/intentions/addNamesToCallArguments/notOnCallee.kt");
-        }
-
        @TestMetadata("notResolved.kt")
        public void testNotResolved() throws Exception {
            runTest("idea/testData/intentions/addNamesToCallArguments/notResolved.kt");
@@ -1329,6 +1324,11 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
            runTest("idea/testData/intentions/addNamesToCallArguments/simple.kt");
        }

+        @TestMetadata("singleArgument.kt")
+        public void testSingleArgument() throws Exception {
+            runTest("idea/testData/intentions/addNamesToCallArguments/singleArgument.kt");
+        }
+
        @TestMetadata("superClassConstructor.kt")
        public void testSuperClassConstructor() throws Exception {
            runTest("idea/testData/intentions/addNamesToCallArguments/superClassConstructor.kt");
@@ -1355,6 +1355,34 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
        }
    }

+    @TestMetadata("idea/testData/intentions/addNamesToFollowingArguments")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class AddNamesToFollowingArguments extends AbstractIntentionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInAddNamesToFollowingArguments() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/addNamesToFollowingArguments"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("first.kt")
+        public void testFirst() throws Exception {
+            runTest("idea/testData/intentions/addNamesToFollowingArguments/first.kt");
+        }
+
+        @TestMetadata("last.kt")
+        public void testLast() throws Exception {
+            runTest("idea/testData/intentions/addNamesToFollowingArguments/last.kt");
+        }
+
+        @TestMetadata("simple.kt")
+        public void testSimple() throws Exception {
+            runTest("idea/testData/intentions/addNamesToFollowingArguments/simple.kt");
+        }
+    }
+
    @TestMetadata("idea/testData/intentions/addOpenModifier")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.intentions.AddNamesToFollowingArgumentsIntention`