From 13a2612e201a4c253e00a0e894fb62bb1bf62aaf Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Wed, 14 Jun 2017 00:10:21 +0300 Subject: [PATCH] Add inspection for ranges with start > endInclusive #KT-18438 Fixed --- .../inspectionDescriptions/EmptyRange.html | 5 ++ idea/src/META-INF/plugin.xml | 8 +++ .../AbstractPrimitiveRangeToInspection.kt | 64 +++++++++++++++++++ .../idea/inspections/EmptyRangeInspection.kt | 64 +++++++++++++++++++ .../ReplaceRangeToWithUntilInspection.kt | 47 ++++---------- .../emptyRange/inspectionData/expected.xml | 56 ++++++++++++++++ .../inspectionData/inspections.test | 1 + idea/testData/inspections/emptyRange/test.kt | 17 +++++ .../inspectionsLocal/emptyRange/.inspection | 1 + .../inspectionsLocal/emptyRange/simple.kt | 5 ++ .../emptyRange/simple.kt.after | 5 ++ .../codeInsight/InspectionTestGenerated.java | 6 ++ .../LocalInspectionTestGenerated.java | 15 +++++ 13 files changed, 261 insertions(+), 33 deletions(-) create mode 100644 idea/resources/inspectionDescriptions/EmptyRange.html create mode 100644 idea/src/org/jetbrains/kotlin/idea/inspections/AbstractPrimitiveRangeToInspection.kt create mode 100644 idea/src/org/jetbrains/kotlin/idea/inspections/EmptyRangeInspection.kt create mode 100644 idea/testData/inspections/emptyRange/inspectionData/expected.xml create mode 100644 idea/testData/inspections/emptyRange/inspectionData/inspections.test create mode 100644 idea/testData/inspections/emptyRange/test.kt create mode 100644 idea/testData/inspectionsLocal/emptyRange/.inspection create mode 100644 idea/testData/inspectionsLocal/emptyRange/simple.kt create mode 100644 idea/testData/inspectionsLocal/emptyRange/simple.kt.after diff --git a/idea/resources/inspectionDescriptions/EmptyRange.html b/idea/resources/inspectionDescriptions/EmptyRange.html new file mode 100644 index 00000000000..0ae3feab473 --- /dev/null +++ b/idea/resources/inspectionDescriptions/EmptyRange.html @@ -0,0 +1,5 @@ + + +This inspection reports ranges that are empty because the 'start' value is greater than the 'endInclusive' value. + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 1d32b6bf800..834c9c8c6e6 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2222,6 +2222,14 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/AbstractPrimitiveRangeToInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/AbstractPrimitiveRangeToInspection.kt new file mode 100644 index 00000000000..cd1c16d77ac --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/AbstractPrimitiveRangeToInspection.kt @@ -0,0 +1,64 @@ +/* + * 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.ProblemsHolder +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.intentions.getCallableDescriptor +import org.jetbrains.kotlin.psi.KtBinaryExpression +import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtVisitorVoid +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.callUtil.getType +import org.jetbrains.kotlin.resolve.constants.ConstantValue +import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode + +abstract class AbstractPrimitiveRangeToInspection : AbstractKotlinInspection() { + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : KtVisitorVoid() { + override fun visitExpression(expression: KtExpression) { + super.visitExpression(expression) + + if (expression !is KtBinaryExpression && expression !is KtDotQualifiedExpression) return + + val fqName = expression.getCallableDescriptor()?.fqNameUnsafe?.asString() ?: return + if (!fqName.matches(REGEX_RANGE_TO)) return + + visitRangeToExpression(expression, holder) + } + } + } + + abstract fun visitRangeToExpression(expression: KtExpression, holder: ProblemsHolder) + + companion object { + private val REGEX_RANGE_TO = """kotlin.(Char|Byte|Short|Int|Long).rangeTo""".toRegex() + + fun KtExpression.constantValueOrNull(context: BindingContext? = null): ConstantValue? { + val c = context ?: this.analyze(BodyResolveMode.PARTIAL) + + val constant = ConstantExpressionEvaluator.getConstant(this, c) ?: return null + + return constant.toConstantValue(getType(c) ?: return null) + } + } +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/EmptyRangeInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/EmptyRangeInspection.kt new file mode 100644 index 00000000000..074430840de --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/EmptyRangeInspection.kt @@ -0,0 +1,64 @@ +/* + * 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.LocalQuickFix +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.project.Project +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.intentions.getArguments +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtPsiFactory +import org.jetbrains.kotlin.psi.createExpressionByPattern +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode + +class EmptyRangeInspection : AbstractPrimitiveRangeToInspection() { + override fun visitRangeToExpression(expression: KtExpression, holder: ProblemsHolder) { + val (left, right) = expression.getArguments() ?: return + + val context = expression.analyze(BodyResolveMode.PARTIAL) + val startValue = left?.longValueOrNull(context) ?: return + val endValue = right?.longValueOrNull(context) ?: return + + if (startValue <= endValue) return + + holder.registerProblem( + expression, + "This range is empty. Did you mean to use 'downTo'?", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + ReplaceWithDownToFix()) + } + + class ReplaceWithDownToFix : LocalQuickFix { + override fun getFamilyName() = "Replace with 'downTo'" + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val element = descriptor.psiElement as? KtExpression ?: return + val (left, right) = element.getArguments() ?: return + if (left == null || right == null) return + + element.replace(KtPsiFactory(element).createExpressionByPattern("$0 downTo $1", left, right)) + } + } + + private fun KtExpression.longValueOrNull(context: BindingContext): Long? { + return (constantValueOrNull(context)?.value as? Number)?.toLong() + } +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceRangeToWithUntilInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceRangeToWithUntilInspection.kt index 8fe5e767115..33a97814150 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceRangeToWithUntilInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceRangeToWithUntilInspection.kt @@ -21,41 +21,24 @@ import com.intellij.codeInspection.ProblemDescriptor import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder import com.intellij.openapi.project.Project -import com.intellij.psi.PsiElementVisitor -import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.intentions.getArguments -import org.jetbrains.kotlin.idea.intentions.getCallableDescriptor import org.jetbrains.kotlin.lexer.KtTokens -import org.jetbrains.kotlin.psi.* -import org.jetbrains.kotlin.resolve.calls.callUtil.getType -import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator -import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe -import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.psi.KtBinaryExpression +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtPsiFactory +import org.jetbrains.kotlin.psi.createExpressionByPattern -private val REGEX_RANGE_TO = """kotlin.(Char|Byte|Short|Int|Long).rangeTo""".toRegex() +class ReplaceRangeToWithUntilInspection : AbstractPrimitiveRangeToInspection() { -class ReplaceRangeToWithUntilInspection : AbstractKotlinInspection() { + override fun visitRangeToExpression(expression: KtExpression, holder: ProblemsHolder) { + if (expression.getArguments()?.second?.isMinusOne() != true) return - override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { - return object : KtVisitorVoid() { - override fun visitExpression(expression: KtExpression) { - super.visitExpression(expression) - - if (expression !is KtBinaryExpression && expression !is KtDotQualifiedExpression) return - - val fqName = expression.getCallableDescriptor()?.fqNameUnsafe?.asString() ?: return - if (!fqName.matches(REGEX_RANGE_TO)) return - - if (expression.getArguments()?.second?.isMinusOne() != true) return - - holder.registerProblem( - expression, - "'rangeTo' or the '..' call can be replaced with 'until'", - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - ReplaceWithUntilQuickFix() - ) - } - } + holder.registerProblem( + expression, + "'rangeTo' or the '..' call can be replaced with 'until'", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + ReplaceWithUntilQuickFix() + ) } class ReplaceWithUntilQuickFix : LocalQuickFix { @@ -80,9 +63,7 @@ class ReplaceRangeToWithUntilInspection : AbstractKotlinInspection() { if (this !is KtBinaryExpression) return false if (operationToken != KtTokens.MINUS) return false - val right = right as? KtConstantExpression ?: return false - val context = right.analyze(BodyResolveMode.PARTIAL) - val constantValue = ConstantExpressionEvaluator.getConstant(right, context)?.toConstantValue(right.getType(context) ?: return false) + val constantValue = right?.constantValueOrNull() val rightValue = (constantValue?.value as? Number)?.toInt() ?: return false return rightValue == 1 } diff --git a/idea/testData/inspections/emptyRange/inspectionData/expected.xml b/idea/testData/inspections/emptyRange/inspectionData/expected.xml new file mode 100644 index 00000000000..3d8bfaa2ae8 --- /dev/null +++ b/idea/testData/inspections/emptyRange/inspectionData/expected.xml @@ -0,0 +1,56 @@ + + + test.kt + 5 + light_idea_test_case + + Range with start greater than endInclusive is empty + + This range is empty. Did you mean to use 'downTo'? + + + test.kt + 6 + light_idea_test_case + + Range with start greater than endInclusive is empty + + This range is empty. Did you mean to use 'downTo'? + + + test.kt + 7 + light_idea_test_case + + Range with start greater than endInclusive is empty + + This range is empty. Did you mean to use 'downTo'? + + + test.kt + 8 + light_idea_test_case + + Range with start greater than endInclusive is empty + + This range is empty. Did you mean to use 'downTo'? + + + test.kt + 9 + light_idea_test_case + + Range with start greater than endInclusive is empty + + This range is empty. Did you mean to use 'downTo'? + + + test.kt + 10 + light_idea_test_case + + Range with start greater than endInclusive is empty + + This range is empty. Did you mean to use 'downTo'? + + diff --git a/idea/testData/inspections/emptyRange/inspectionData/inspections.test b/idea/testData/inspections/emptyRange/inspectionData/inspections.test new file mode 100644 index 00000000000..dbcac24d379 --- /dev/null +++ b/idea/testData/inspections/emptyRange/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.EmptyRangeInspection diff --git a/idea/testData/inspections/emptyRange/test.kt b/idea/testData/inspections/emptyRange/test.kt new file mode 100644 index 00000000000..dfa51d2d9dd --- /dev/null +++ b/idea/testData/inspections/emptyRange/test.kt @@ -0,0 +1,17 @@ +// WITH_RUNTIME +const val ONE = 1 + +fun foo() { + 2..1 + 2.rangeTo(1) + 2..1L + 10L..-10L + 5..ONE + 10.toShort()..1.toShort() + + //valid + 1..1 + 1..10L + 1..2 + 1.rangeTo(2) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/emptyRange/.inspection b/idea/testData/inspectionsLocal/emptyRange/.inspection new file mode 100644 index 00000000000..2c50b6bb6cd --- /dev/null +++ b/idea/testData/inspectionsLocal/emptyRange/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.EmptyRangeInspection diff --git a/idea/testData/inspectionsLocal/emptyRange/simple.kt b/idea/testData/inspectionsLocal/emptyRange/simple.kt new file mode 100644 index 00000000000..eb4d5959bfe --- /dev/null +++ b/idea/testData/inspectionsLocal/emptyRange/simple.kt @@ -0,0 +1,5 @@ +// WITH_RUNTIME + +fun foo() { + for (i in 9..0) {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/emptyRange/simple.kt.after b/idea/testData/inspectionsLocal/emptyRange/simple.kt.after new file mode 100644 index 00000000000..daaca4dbdb3 --- /dev/null +++ b/idea/testData/inspectionsLocal/emptyRange/simple.kt.after @@ -0,0 +1,5 @@ +// WITH_RUNTIME + +fun foo() { + for (i in 9 downTo 0) {} +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 10d31b153c6..9ae9a9d1e48 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -197,6 +197,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { doTest(fileName); } + @TestMetadata("emptyRange/inspectionData/inspections.test") + public void testEmptyRange_inspectionData_Inspections_test() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/emptyRange/inspectionData/inspections.test"); + doTest(fileName); + } + @TestMetadata("equalsAndHashCode/inspectionData/inspections.test") public void testEqualsAndHashCode_inspectionData_Inspections_test() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/equalsAndHashCode/inspectionData/inspections.test"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index d07c8802a28..26e32210c38 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -63,6 +63,21 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/emptyRange") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class EmptyRange extends AbstractLocalInspectionTest { + public void testAllFilesPresentInEmptyRange() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/emptyRange"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("simple.kt") + public void testSimple() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/emptyRange/simple.kt"); + doTest(fileName); + } + } + @TestMetadata("idea/testData/inspectionsLocal/leakingThis") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)