Add "simplify if with constant" inspection #KT-17919 Fixed
This commit is contained in:
Kirill Rakhman
committed by
Mikhail Glukhikh
Mikhail Glukhikh
parent
7634673213
commit
cb19a86c44
@@ -0,0 +1,5 @@
|
|||||||
|
<html>
|
||||||
|
<body>
|
||||||
|
This inspection reports 'if' expressions whose condition is constant and therefore the expression can be simplified.
|
||||||
|
</body>
|
||||||
|
</html>
|
||||||
@@ -2387,6 +2387,15 @@
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ConstantConditionIfInspection"
|
||||||
|
displayName="Condition of 'if' expression is constant"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Redundant constructs"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -0,0 +1,127 @@
|
|||||||
|
/*
|
||||||
|
* 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.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.core.replaced
|
||||||
|
import org.jetbrains.kotlin.idea.intentions.branchedTransformations.unwrapBlockOrParenthesis
|
||||||
|
import org.jetbrains.kotlin.psi.*
|
||||||
|
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
|
||||||
|
import org.jetbrains.kotlin.psi.psiUtil.startOffset
|
||||||
|
import org.jetbrains.kotlin.resolve.BindingContext
|
||||||
|
import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
|
||||||
|
import org.jetbrains.kotlin.resolve.calls.callUtil.getType
|
||||||
|
import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
|
||||||
|
import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
|
||||||
|
|
||||||
|
class ConstantConditionIfInspection : AbstractKotlinInspection() {
|
||||||
|
|
||||||
|
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
|
||||||
|
return object : KtVisitorVoid() {
|
||||||
|
override fun visitIfExpression(expression: KtIfExpression) {
|
||||||
|
super.visitIfExpression(expression)
|
||||||
|
|
||||||
|
val condition = expression.condition ?: return
|
||||||
|
|
||||||
|
val context = condition.analyze(BodyResolveMode.PARTIAL)
|
||||||
|
val constantValue = condition.constantBooleanValue(context) ?: return
|
||||||
|
|
||||||
|
val fixes = mutableListOf<LocalQuickFix>()
|
||||||
|
|
||||||
|
if (expression.branch(constantValue) != null) {
|
||||||
|
fixes += SimplifyFix(constantValue, expression.isUsedAsExpression(context))
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!constantValue && expression.`else` == null) {
|
||||||
|
fixes += RemoveFix()
|
||||||
|
}
|
||||||
|
|
||||||
|
holder.registerProblem(condition,
|
||||||
|
"Condition is always '$constantValue'",
|
||||||
|
*fixes.toTypedArray())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private class SimplifyFix(
|
||||||
|
private val conditionValue: Boolean,
|
||||||
|
private val isUsedAsExpression: Boolean
|
||||||
|
) : LocalQuickFix {
|
||||||
|
override fun getFamilyName() = name
|
||||||
|
|
||||||
|
override fun getName() = "Simplify expression"
|
||||||
|
|
||||||
|
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
|
||||||
|
val ifExpression = descriptor.psiElement.getParentOfType<KtIfExpression>(strict = true) ?: return
|
||||||
|
val caretModel = ifExpression.findExistingEditor()?.caretModel
|
||||||
|
|
||||||
|
val branch = ifExpression.branch(conditionValue)?.unwrapBlockOrParenthesis() ?: return
|
||||||
|
|
||||||
|
val lastExpression = when {
|
||||||
|
branch !is KtBlockExpression -> ifExpression.replaced(branch)
|
||||||
|
isUsedAsExpression -> {
|
||||||
|
val factory = KtPsiFactory(ifExpression)
|
||||||
|
ifExpression.replaced(factory.createExpressionByPattern("run $0", branch.text))
|
||||||
|
}
|
||||||
|
else -> {
|
||||||
|
val firstChild = branch.firstChild.nextSibling
|
||||||
|
|
||||||
|
if (firstChild == branch.lastChild) {
|
||||||
|
ifExpression.delete()
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
val lastChild = branch.lastChild.prevSibling
|
||||||
|
val parent = ifExpression.parent
|
||||||
|
parent.addRangeAfter(firstChild, lastChild, ifExpression)
|
||||||
|
ifExpression.delete()
|
||||||
|
}
|
||||||
|
|
||||||
|
null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
caretModel?.moveToOffset(lastExpression?.startOffset ?: return)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private class RemoveFix : LocalQuickFix {
|
||||||
|
override fun getFamilyName() = name
|
||||||
|
|
||||||
|
override fun getName() = "Delete expression"
|
||||||
|
|
||||||
|
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
|
||||||
|
val ifExpression = descriptor.psiElement.getParentOfType<KtIfExpression>(strict = true) ?: return
|
||||||
|
ifExpression.delete()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private companion object {
|
||||||
|
private fun KtIfExpression.branch(thenBranch: Boolean) = if (thenBranch) then else `else`
|
||||||
|
|
||||||
|
private fun KtExpression.constantBooleanValue(context: BindingContext): Boolean? {
|
||||||
|
val type = getType(context) ?: return null
|
||||||
|
|
||||||
|
val constantValue = ConstantExpressionEvaluator.getConstant(this, context)?.toConstantValue(type)
|
||||||
|
return constantValue?.value as? Boolean
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,42 @@
|
|||||||
|
<problems>
|
||||||
|
<problem>
|
||||||
|
<file>test.kt</file>
|
||||||
|
<line>7</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
|
||||||
|
<description>Condition is always 'true'</description>
|
||||||
|
</problem>
|
||||||
|
<problem>
|
||||||
|
<file>test.kt</file>
|
||||||
|
<line>9</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
|
||||||
|
<description>Condition is always 'false'</description>
|
||||||
|
</problem>
|
||||||
|
<problem>
|
||||||
|
<file>test.kt</file>
|
||||||
|
<line>14</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
|
||||||
|
<description>Condition is always 'true'</description>
|
||||||
|
</problem>
|
||||||
|
<problem>
|
||||||
|
<file>test.kt</file>
|
||||||
|
<line>20</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
|
||||||
|
<description>Condition is always 'false'</description>
|
||||||
|
</problem>
|
||||||
|
<problem>
|
||||||
|
<file>test.kt</file>
|
||||||
|
<line>24</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
|
||||||
|
<description>Condition is always 'true'</description>
|
||||||
|
</problem>
|
||||||
|
</problems>
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.ConstantConditionIfInspection
|
||||||
@@ -0,0 +1,28 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
fun baz(s: String) {}
|
||||||
|
|
||||||
|
const val TRUE = true
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(if (TRUE) 1 else 2)
|
||||||
|
|
||||||
|
foo(if (false && TRUE) {
|
||||||
|
baz("a")
|
||||||
|
1
|
||||||
|
} else 2)
|
||||||
|
|
||||||
|
if (true) {
|
||||||
|
//asd
|
||||||
|
baz("a")
|
||||||
|
baz("b")
|
||||||
|
}
|
||||||
|
|
||||||
|
if (false) {
|
||||||
|
baz("a")
|
||||||
|
}
|
||||||
|
|
||||||
|
foo(if (TRUE) {
|
||||||
|
baz("a")
|
||||||
|
1
|
||||||
|
} else 2)
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
org.jetbrains.kotlin.idea.inspections.ConstantConditionIfInspection
|
||||||
@@ -0,0 +1,8 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
if (<caret>false) {
|
||||||
|
foo(1)
|
||||||
|
foo(2)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
}
|
||||||
@@ -0,0 +1,11 @@
|
|||||||
|
// WITH_RUNTIME
|
||||||
|
|
||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(if (<caret>true) {
|
||||||
|
foo(1)
|
||||||
|
foo(2)
|
||||||
|
1
|
||||||
|
} else 2)
|
||||||
|
}
|
||||||
@@ -0,0 +1,11 @@
|
|||||||
|
// WITH_RUNTIME
|
||||||
|
|
||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(run {
|
||||||
|
foo(1)
|
||||||
|
foo(2)
|
||||||
|
1
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
fun foo(s: String) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
if (<caret>true) {
|
||||||
|
} else 2
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(s: String) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(if (<caret>true) { 1 } else 2)
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(1)
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(if (<caret>false) 1 else 2)
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(2)
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(if (<caret>true) 1 else 2)
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
fun foo(x: Int) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo(1)
|
||||||
|
}
|
||||||
@@ -0,0 +1,8 @@
|
|||||||
|
fun foo(s: String) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
if (<caret>true) {
|
||||||
|
foo("a")
|
||||||
|
foo("b")
|
||||||
|
} else 2
|
||||||
|
}
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
fun foo(s: String) {}
|
||||||
|
|
||||||
|
fun bar() {
|
||||||
|
foo("a")
|
||||||
|
foo("b")
|
||||||
|
}
|
||||||
@@ -161,6 +161,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
|
|||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("constantConditionIf/inspectionData/inspections.test")
|
||||||
|
public void testConstantConditionIf_inspectionData_Inspections_test() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/constantConditionIf/inspectionData/inspections.test");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("convertLambdaToReference/inspectionData/inspections.test")
|
@TestMetadata("convertLambdaToReference/inspectionData/inspections.test")
|
||||||
public void testConvertLambdaToReference_inspectionData_Inspections_test() throws Exception {
|
public void testConvertLambdaToReference_inspectionData_Inspections_test() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/convertLambdaToReference/inspectionData/inspections.test");
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/convertLambdaToReference/inspectionData/inspections.test");
|
||||||
|
|||||||
@@ -408,6 +408,57 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("idea/testData/inspectionsLocal/constantConditionIf")
|
||||||
|
@TestDataPath("$PROJECT_ROOT")
|
||||||
|
@RunWith(JUnit3RunnerWithInners.class)
|
||||||
|
public static class ConstantConditionIf extends AbstractLocalInspectionTest {
|
||||||
|
public void testAllFilesPresentInConstantConditionIf() throws Exception {
|
||||||
|
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/constantConditionIf"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("delete.kt")
|
||||||
|
public void testDelete() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/delete.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("expression.kt")
|
||||||
|
public void testExpression() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/expression.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("noStatements.kt")
|
||||||
|
public void testNoStatements() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/noStatements.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("simpleBlock.kt")
|
||||||
|
public void testSimpleBlock() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/simpleBlock.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("simpleFalse.kt")
|
||||||
|
public void testSimpleFalse() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/simpleFalse.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("simpleTrue.kt")
|
||||||
|
public void testSimpleTrue() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/simpleTrue.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("statement.kt")
|
||||||
|
public void testStatement() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/statement.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("idea/testData/inspectionsLocal/copyWithoutNamedArguments")
|
@TestMetadata("idea/testData/inspectionsLocal/copyWithoutNamedArguments")
|
||||||
@TestDataPath("$PROJECT_ROOT")
|
@TestDataPath("$PROJECT_ROOT")
|
||||||
@RunWith(JUnit3RunnerWithInners.class)
|
@RunWith(JUnit3RunnerWithInners.class)
|
||||||
|
|||||||
Reference in New Issue
Block a user