[java-inspections] IDEA-345255 Duplicate condition: improve detection of &&-chains

GitOrigin-RevId: e6b504e7665921e66c17690f6a144fb36dcd8124
This commit is contained in:
Tagir Valeev
2024-02-07 19:28:52 +00:00
committed by intellij-monorepo-bot
parent 0931d75584
commit d469c59435
6 changed files with 116 additions and 38 deletions
@@ -0,0 +1,34 @@
// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.codeInspection;
import com.intellij.java.JavaBundle;
import com.intellij.modcommand.ModCommand;
import com.intellij.modcommand.ModCommandQuickFix;
import com.intellij.openapi.project.Project;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.SmartPointerManager;
import com.intellij.psi.SmartPsiElementPointer;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
public class NavigateToDuplicateExpressionFix extends ModCommandQuickFix {
private final SmartPsiElementPointer<PsiExpression> myPointer;
public NavigateToDuplicateExpressionFix(@NotNull PsiExpression arg) {
myPointer = SmartPointerManager.getInstance(arg.getProject()).createSmartPsiElementPointer(arg);
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return JavaBundle.message("navigate.to.duplicate.fix");
}
@Override
public @NotNull ModCommand perform(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiExpression element = myPointer.getElement();
if (element == null) return ModCommand.nop();
return ModCommand.select(element);
}
}
@@ -15,11 +15,14 @@
*/
package com.siyeh.ig.controlflow;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.NavigateToDuplicateExpressionFix;
import com.intellij.codeInspection.options.OptPane;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.ThreeState;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
@@ -30,6 +33,7 @@ import com.siyeh.ig.psiutils.EquivalenceChecker;
import com.siyeh.ig.psiutils.SideEffectChecker;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
@@ -64,20 +68,41 @@ public final class DuplicateConditionInspection extends BaseInspection {
return new DuplicateConditionVisitor();
}
@Override
protected @Nullable LocalQuickFix buildFix(Object... infos) {
if (ArrayUtil.getFirstElement(infos) instanceof PsiExpression duplicate) {
return new NavigateToDuplicateExpressionFix(duplicate);
}
return super.buildFix(infos);
}
private class DuplicateConditionVisitor extends BaseInspectionVisitor {
private final Set<PsiIfStatement> myAnalyzedStatements = new HashSet<>();
private final Set<PsiIfStatement> myAnalyzedAndStatements = new HashSet<>();
private final Set<PsiIfStatement> myAnalyzedOrStatements = new HashSet<>();
@Override
public void visitIfStatement(@NotNull PsiIfStatement statement) {
super.visitIfStatement(statement);
if (ControlFlowUtils.isElseIf(statement)) return;
PsiElement parent = statement.getParent();
if (parent instanceof PsiIfStatement) return;
if (parent instanceof PsiCodeBlock codeBlock && ArrayUtil.getFirstElement(codeBlock.getStatements()) == statement &&
parent.getParent() instanceof PsiBlockStatement blockStatement && blockStatement.getParent() instanceof PsiIfStatement parentIf &&
parentIf.getThenBranch() == blockStatement) {
return;
}
final Set<PsiExpression> conditions = new LinkedHashSet<>();
collectConditionsForIfStatement(statement, conditions, 0);
if (conditions.size() < 2) return;
findDuplicatesAccordingToSideEffects(conditions);
collectConditionsForIfStatementOrChain(statement, conditions, 0);
if (conditions.size() >= 2) {
findDuplicatesAccordingToSideEffects(conditions);
}
conditions.clear();
collectConditionsForIfStatementAndChain(statement, conditions, 0);
if (conditions.size() >= 2) {
findDuplicatesAccordingToSideEffects(conditions);
}
}
@Override
@@ -100,20 +125,37 @@ public final class DuplicateConditionInspection extends BaseInspection {
findDuplicatesAccordingToSideEffects(conditions);
}
private void collectConditionsForIfStatement(PsiIfStatement statement, Set<? super PsiExpression> conditions, int depth) {
if (depth > LIMIT_DEPTH || !myAnalyzedStatements.add(statement)) return;
private void collectConditionsForIfStatementAndChain(PsiIfStatement statement, Set<? super PsiExpression> conditions, int depth) {
if (depth > LIMIT_DEPTH || !myAnalyzedAndStatements.add(statement)) return;
final PsiExpression condition = statement.getCondition();
collectConditionsForExpression(condition, conditions, JavaTokenType.ANDAND);
final PsiStatement branch = ControlFlowUtils.stripBraces(statement.getThenBranch());
if (branch instanceof PsiIfStatement ifStatement) {
collectConditionsForIfStatementAndChain(ifStatement, conditions, depth + 1);
}
if (branch instanceof PsiBlockStatement blockStatement) {
PsiStatement[] statements = blockStatement.getCodeBlock().getStatements();
if (statements.length == 0) return;
if (statements[0] instanceof PsiIfStatement ifStatement) {
collectConditionsForIfStatementAndChain(ifStatement, conditions, depth + 1);
}
}
}
private void collectConditionsForIfStatementOrChain(PsiIfStatement statement, Set<? super PsiExpression> conditions, int depth) {
if (depth > LIMIT_DEPTH || !myAnalyzedOrStatements.add(statement)) return;
final PsiExpression condition = statement.getCondition();
collectConditionsForExpression(condition, conditions, JavaTokenType.OROR);
final PsiStatement branch = ControlFlowUtils.stripBraces(statement.getElseBranch());
if (branch instanceof PsiIfStatement) {
collectConditionsForIfStatement((PsiIfStatement)branch, conditions, depth + 1);
collectConditionsForIfStatementOrChain((PsiIfStatement)branch, conditions, depth + 1);
}
if (branch == null) {
final PsiStatement thenBranch = statement.getThenBranch();
if (ControlFlowUtils.statementMayCompleteNormally(thenBranch)) return;
PsiElement next = PsiTreeUtil.skipWhitespacesAndCommentsForward(statement);
if (next instanceof PsiIfStatement) {
collectConditionsForIfStatement((PsiIfStatement)next, conditions, depth + 1);
collectConditionsForIfStatementOrChain((PsiIfStatement)next, conditions, depth + 1);
}
}
}
@@ -165,9 +207,9 @@ public final class DuplicateConditionInspection extends BaseInspection {
final PsiExpression testCondition = conditions.get(j);
final boolean areEquivalent = EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(condition, testCondition);
if (areEquivalent) {
registerError(testCondition);
registerError(testCondition, condition);
if (!matched.get(i)) {
registerError(condition);
registerError(condition, testCondition);
}
matched.set(i);
matched.set(j);
@@ -6,9 +6,6 @@ import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInspection.util.InspectionMessage;
import com.intellij.java.JavaBundle;
import com.intellij.modcommand.ModCommand;
import com.intellij.modcommand.ModCommandQuickFix;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
@@ -19,7 +16,6 @@ import com.siyeh.ig.psiutils.VariableAccessUtils;
import one.util.streamex.IntStreamEx;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -199,7 +195,7 @@ public final class OverwrittenKeyInspection extends AbstractBaseJavaLocalInspect
for (int i = 0; i < args.size(); i++) {
PsiExpression arg = args.get(i);
PsiExpression nextArg = args.get((i + 1) % args.size());
LocalQuickFix fix = new NavigateToDuplicateFix(nextArg);
LocalQuickFix fix = new NavigateToDuplicateExpressionFix(nextArg);
myHolder.registerProblem(arg, message, fix);
}
}
@@ -232,26 +228,4 @@ public final class OverwrittenKeyInspection extends AbstractBaseJavaLocalInspect
return null;
}
}
private static class NavigateToDuplicateFix extends ModCommandQuickFix {
private final SmartPsiElementPointer<PsiExpression> myPointer;
NavigateToDuplicateFix(PsiExpression arg) {
myPointer = SmartPointerManager.getInstance(arg.getProject()).createSmartPsiElementPointer(arg);
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return JavaBundle.message("navigate.to.duplicate.fix");
}
@Override
public @NotNull ModCommand perform(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiExpression element = myPointer.getElement();
if (element == null) return ModCommand.nop();
return ModCommand.select(element);
}
}
}
@@ -4,6 +4,19 @@ import java.util.*;
public class DuplicateCondition {
void testAndChain(String s) {
if (!s.isEmpty() && <warning descr="Duplicate condition 's.trim().length() == 5'">s.trim().length() == 5</warning> && <warning descr="Duplicate condition 's.trim().length() == 5'">s.trim().length() == 5</warning>) {}
}
void testAndChainNested(String s) {
if (!<warning descr="Duplicate condition 's.isEmpty()'">s.isEmpty()</warning> && <warning descr="Duplicate condition 's.trim().length() == 5'">s.trim().length() == 5</warning>) {
if (<warning descr="Duplicate condition 's.trim().length() == 5'">s.trim().length() == 5</warning>) {
if (!<warning descr="Duplicate condition 's.isEmpty()'">s.isEmpty()</warning>) {}
}
System.out.println("Hello");
}
}
void x(boolean b) {
if (<warning descr="Duplicate condition 'b'">b</warning> || <warning descr="Duplicate condition 'b'">b</warning> || <warning descr="Duplicate condition 'b'">b</warning> ) {
@@ -0,0 +1,9 @@
class X {
void foo(int x) {
if (<warning descr="Duplicate condition 'Math.abs(x) > 0'">Math.abs(x) > 0</warning>) {
if (<warning descr="Duplicate condition 'Math.abs(x) > 0'">Math<caret>.abs(x) > 0</warning>) {
}
}
}
}
@@ -1,5 +1,6 @@
package com.siyeh.ig.controlflow;
import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.codeInspection.InspectionProfileEntry;
import com.intellij.testFramework.LightProjectDescriptor;
import com.siyeh.ig.LightJavaInspectionTestCase;
@@ -24,6 +25,11 @@ public class DuplicateConditionInspectionTest extends LightJavaInspectionTestCas
public void testDuplicateWithNegation() {
doTest();
}
public void testFix() {
doTest();
IntentionAction action = myFixture.findSingleIntention("Navigate to duplicate");
myFixture.checkIntentionPreviewHtml(action, "<p>&rarr; <icon src=\"icon\"/>&nbsp;Fix.java, line #3</p>");
}
@Nullable
@Override