diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/NavigateToDuplicateExpressionFix.java b/java/java-analysis-impl/src/com/intellij/codeInspection/NavigateToDuplicateExpressionFix.java new file mode 100644 index 000000000000..8bdd404a6dbe --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/NavigateToDuplicateExpressionFix.java @@ -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 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); + } +} diff --git a/java/java-analysis-impl/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java b/java/java-analysis-impl/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java index 400c25175174..b217c4600c4e 100644 --- a/java/java-analysis-impl/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java +++ b/java/java-analysis-impl/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java @@ -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 myAnalyzedStatements = new HashSet<>(); + private final Set myAnalyzedAndStatements = new HashSet<>(); + private final Set 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 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 conditions, int depth) { - if (depth > LIMIT_DEPTH || !myAnalyzedStatements.add(statement)) return; + private void collectConditionsForIfStatementAndChain(PsiIfStatement statement, Set 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 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); diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/OverwrittenKeyInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/OverwrittenKeyInspection.java index 8b08f25b0059..a33df1076706 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/OverwrittenKeyInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/OverwrittenKeyInspection.java @@ -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 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); - } - } } diff --git a/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java index cc14a4b8eda4..7a8e052b5c9f 100644 --- a/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java +++ b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java @@ -4,6 +4,19 @@ import java.util.*; public class DuplicateCondition { + void testAndChain(String s) { + if (!s.isEmpty() && s.trim().length() == 5 && s.trim().length() == 5) {} + } + + void testAndChainNested(String s) { + if (!s.isEmpty() && s.trim().length() == 5) { + if (s.trim().length() == 5) { + if (!s.isEmpty()) {} + } + System.out.println("Hello"); + } + } + void x(boolean b) { if (b || b || b ) { diff --git a/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/Fix.java b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/Fix.java new file mode 100644 index 000000000000..2a5d7aa70ee4 --- /dev/null +++ b/java/java-tests/testData/ig/com/siyeh/igtest/controlflow/duplicate_condition/Fix.java @@ -0,0 +1,9 @@ +class X { + void foo(int x) { + if (Math.abs(x) > 0) { + if (Math.abs(x) > 0) { + + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java b/java/java-tests/testSrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java index 342b0c971aa1..a6a9bc0f299d 100644 --- a/java/java-tests/testSrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java +++ b/java/java-tests/testSrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java @@ -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, "

→  Fix.java, line #3

"); + } @Nullable @Override