Boolean literal arguments: check previous arg, fix all args in a row

2018-12-03 10:00:55 +03:00
parent 235b50d86e
commit 13b09f532c
11 changed files with 129 additions and 34 deletions
@@ -6,10 +6,13 @@
 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
@@ -30,23 +33,28 @@ class BooleanLiteralArgumentInspection(
            if (argument.getArgumentName() != null) return
            val argumentExpression = argument.getArgumentExpression() as? KtConstantExpression ?: return
            if (argumentExpression.node.elementType != KtNodeTypes.BOOLEAN_CONSTANT) return
-            if (argumentExpression.analyze().diagnostics.forElement(argumentExpression).any { it.severity == Severity.ERROR }) 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 (call.resolveToCall()?.resultingDescriptor?.hasStableParameterNames() != true) return

-            val valueArguments = call.valueArguments
-            fun hasAnotherUnnamedBoolean() = valueArguments.asSequence().filter { it != argument }.any {
-                !it.isNamed() && (it.getArgumentExpression() as? KtConstantExpression)?.node?.elementType == KtNodeTypes.BOOLEAN_CONSTANT
+            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
            }
-            when {
-                valueArguments.takeLastWhile { it != argument }.none { !it.isNamed() } ->
-                    holder.registerProblem(
-                        argument,
-                        "Boolean literal argument without parameter name",
-                        if (reportSingle || hasAnotherUnnamedBoolean()) GENERIC_ERROR_OR_WARNING else INFORMATION,
-                        IntentionWrapper(AddNameToArgumentIntention(), argument.containingKtFile)
-                    )
+            val fixes = mutableListOf<LocalQuickFix>()
+            if (hasPreviousUnnamedBoolean) {
+                fixes += AddNamesToLastBooleanArgumentsFix()
            }
+            fixes += IntentionWrapper(AddNameToArgumentIntention(), argument.containingKtFile)
+            holder.registerProblem(
+                argument,
+                "Boolean literal argument without parameter name",
+                if (reportSingle || hasPreviousUnnamedBoolean) GENERIC_ERROR_OR_WARNING else INFORMATION,
+                *fixes.toTypedArray()
+            )
        })

    override fun createOptionsPanel(): JComponent? {
@@ -54,4 +62,28 @@ 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
+            }
+        }
+    }
 }
@@ -56,28 +56,34 @@ class AddNameToArgumentIntention : SelfTargetingIntention<KtValueArgument>(
        element !is KtValueArgumentList && element !is KtContainerNode && super.allowCaretInsideElement(element)

    override fun applyTo(element: KtValueArgument, editor: Editor?) {
-        val name = detectNameToAdd(element)!!
-        val newArgument = KtPsiFactory(element).createArgument(element.getArgumentExpression()!!, name, element.getSpreadElement() != null)
-        element.replace(newArgument)
-    }
-
-    private fun detectNameToAdd(argument: KtValueArgument): 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
-
-        val callExpr = argumentList.parent as? KtCallElement ?: return null
-        val resolvedCall = callExpr.resolveToCall() ?: return null
-        if (!resolvedCall.resultingDescriptor.hasStableParameterNames()) return null
-
-        if (!argumentMatchedAndCouldBeNamedInCall(argument, resolvedCall, callExpr.languageVersionSettings)) return null
-
-        return (resolvedCall.getArgumentMapping(argument) as? ArgumentMatch)?.valueParameter?.name
+        apply(element)
    }

    companion object {
+        fun apply(element: KtValueArgument): Boolean {
+            val name = detectNameToAdd(element) ?: return false
+            val argumentExpression = element.getArgumentExpression() ?: return false
+            val newArgument = KtPsiFactory(element).createArgument(argumentExpression, name, element.getSpreadElement() != null)
+            element.replace(newArgument)
+            return true
+        }
+
+        private fun detectNameToAdd(argument: KtValueArgument): 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
+
+            val callExpr = argumentList.parent as? KtCallElement ?: return null
+            val resolvedCall = callExpr.resolveToCall() ?: return null
+            if (!resolvedCall.resultingDescriptor.hasStableParameterNames()) return null
+
+            if (!argumentMatchedAndCouldBeNamedInCall(argument, resolvedCall, callExpr.languageVersionSettings)) return null
+
+            return (resolvedCall.getArgumentMapping(argument) as? ArgumentMatch)?.valueParameter?.name
+        }
+
        fun argumentMatchedAndCouldBeNamedInCall(
            argument: ValueArgument,
            resolvedCall: ResolvedCall<out CallableDescriptor>,
@@ -0,0 +1,7 @@
+// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
+// FIX: Add names to boolean arguments
+fun foo(a: Boolean, b: Boolean, c: Boolean) {}
+
+fun test() {
+    foo(true, true, true<caret>)
+}
@@ -0,0 +1,7 @@
+// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
+// FIX: Add names to boolean arguments
+fun foo(a: Boolean, b: Boolean, c: Boolean) {}
+
+fun test() {
+    foo(a = true, b = true, c = true)
+}
@@ -0,0 +1,7 @@
+// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
+// FIX: Add names to boolean arguments
+fun foo(a: Boolean, b: Boolean, c: Boolean) {}
+
+fun test() {
+    foo(true, <caret>true, c = true)
+}
@@ -0,0 +1,7 @@
+// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
+// FIX: Add names to boolean arguments
+fun foo(a: Boolean, b: Boolean, c: Boolean) {}
+
+fun test() {
+    foo(a = true, b = true, c = true)
+}
@@ -0,0 +1,7 @@
+// HIGHLIGHT: INFORMATION
+// FIX: Add 'c =' to argument
+fun foo(a: Boolean, b: Int, c: Boolean) {}
+
+fun test() {
+    foo(true, 0, true<caret>)
+}
@@ -0,0 +1,7 @@
+// HIGHLIGHT: INFORMATION
+// FIX: Add 'c =' to argument
+fun foo(a: Boolean, b: Int, c: Boolean) {}
+
+fun test() {
+    foo(true, 0, c = true)
+}
@@ -69,9 +69,24 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
            runTest("idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteral.kt");
        }

-        @TestMetadata("booleanLiteral3.kt")
-        public void testBooleanLiteral3() throws Exception {
-            runTest("idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteral3.kt");
+        @TestMetadata("booleanLiteralFixAll.kt")
+        public void testBooleanLiteralFixAll() throws Exception {
+            runTest("idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt");
+        }
+
+        @TestMetadata("booleanLiteralFixPart.kt")
+        public void testBooleanLiteralFixPart() throws Exception {
+            runTest("idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt");
+        }
+
+        @TestMetadata("booleanLiteralMiddle.kt")
+        public void testBooleanLiteralMiddle() throws Exception {
+            runTest("idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralMiddle.kt");
+        }
+
+        @TestMetadata("booleanLiteralNoPrevious.kt")
+        public void testBooleanLiteralNoPrevious() throws Exception {
+            runTest("idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt");
        }

        @TestMetadata("hasError.kt")