From 13b09f532c8c356f6d1198b94fe7fedadede1f2d Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 3 Dec 2018 10:00:55 +0300 Subject: [PATCH] Boolean literal arguments: check previous arg, fix all args in a row --- .../BooleanLiteralArgumentInspection.kt | 56 +++++++++++++++---- .../intentions/AddNameToArgumentIntention.kt | 44 ++++++++------- .../booleanLiteralFixAll.kt | 7 +++ .../booleanLiteralFixAll.kt.after | 7 +++ .../booleanLiteralFixPart.kt | 7 +++ .../booleanLiteralFixPart.kt.after | 7 +++ ...eanLiteral3.kt => booleanLiteralMiddle.kt} | 0 ...kt.after => booleanLiteralMiddle.kt.after} | 0 .../booleanLiteralNoPrevious.kt | 7 +++ .../booleanLiteralNoPrevious.kt.after | 7 +++ .../LocalInspectionTestGenerated.java | 21 ++++++- 11 files changed, 129 insertions(+), 34 deletions(-) create mode 100644 idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt create mode 100644 idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt.after create mode 100644 idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt create mode 100644 idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt.after rename idea/testData/inspectionsLocal/booleanLiteralArgument/{booleanLiteral3.kt => booleanLiteralMiddle.kt} (100%) rename idea/testData/inspectionsLocal/booleanLiteralArgument/{booleanLiteral3.kt.after => booleanLiteralMiddle.kt.after} (100%) create mode 100644 idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt create mode 100644 idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/BooleanLiteralArgumentInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/BooleanLiteralArgumentInspection.kt index 265f4c17a8c..8be30a0dce3 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/BooleanLiteralArgumentInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/BooleanLiteralArgumentInspection.kt @@ -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() ?: 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() + 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() ?: 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 + } + } + } } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/AddNameToArgumentIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/AddNameToArgumentIntention.kt index a1e498c7b2e..ad6234b2239 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/AddNameToArgumentIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/AddNameToArgumentIntention.kt @@ -56,28 +56,34 @@ class AddNameToArgumentIntention : SelfTargetingIntention( 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, diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt new file mode 100644 index 00000000000..ee322533b24 --- /dev/null +++ b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt @@ -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) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt.after b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt.after new file mode 100644 index 00000000000..14b502c7010 --- /dev/null +++ b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixAll.kt.after @@ -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) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt new file mode 100644 index 00000000000..774d64944c3 --- /dev/null +++ b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt @@ -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, c = true) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt.after b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt.after new file mode 100644 index 00000000000..14b502c7010 --- /dev/null +++ b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralFixPart.kt.after @@ -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) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteral3.kt b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralMiddle.kt similarity index 100% rename from idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteral3.kt rename to idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralMiddle.kt diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteral3.kt.after b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralMiddle.kt.after similarity index 100% rename from idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteral3.kt.after rename to idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralMiddle.kt.after diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt new file mode 100644 index 00000000000..bff3d9ff751 --- /dev/null +++ b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt @@ -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) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt.after b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt.after new file mode 100644 index 00000000000..f12422bec4e --- /dev/null +++ b/idea/testData/inspectionsLocal/booleanLiteralArgument/booleanLiteralNoPrevious.kt.after @@ -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) +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index d085d1310a8..02462b5f856 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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")