diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseFixTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseFixTest.java index 7df98747312d..cd04fec87509 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseFixTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseFixTest.java @@ -15,7 +15,7 @@ */ package com.intellij.codeInsight.daemon.quickFix; -import com.siyeh.ig.controlflow.RedundantElseInspection; +import com.siyeh.ig.controlflow.ConfusingElseInspection; /** * User: anna @@ -25,7 +25,7 @@ public class RemoveRedundantElseFixTest extends LightQuickFixParameterizedTestCa @Override protected void setUp() throws Exception { super.setUp(); - enableInspectionTool(new RedundantElseInspection()); + enableInspectionTool(new ConfusingElseInspection()); } public void test() throws Exception { doAllTests(); } diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/inspection-black-list.txt b/platform/analysis-impl/src/com/intellij/codeInspection/inspection-black-list.txt index 39ef433b1729..95c2c0cf8932 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/inspection-black-list.txt +++ b/platform/analysis-impl/src/com/intellij/codeInspection/inspection-black-list.txt @@ -89,7 +89,7 @@ com.siyeh.ig.classmetrics.FieldCountInspection com.siyeh.ig.classmetrics.MethodCountInspection com.siyeh.ig.cloneable.CloneableImplementsCloneInspection com.siyeh.ig.controlflow.ConditionalExpressionInspection -com.siyeh.ig.controlflow.RedundantElseInspection +com.siyeh.ig.controlflow.ConfusingElseInspection com.siyeh.ig.controlflow.DuplicateConditionInspection com.siyeh.ig.controlflow.EnumSwitchStatementWhichMissesCasesInspection com.siyeh.ig.controlflow.ForLoopReplaceableByWhileInspection diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml index d35b9ba9086d..750fa6c9561e 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml @@ -606,9 +606,9 @@ key="conditional.expression.with.identical.branches.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.control.flow.issues" enabledByDefault="false" level="WARNING" implementationClass="com.siyeh.ig.controlflow.ConditionalExpressionWithIdenticalBranchesInspection" cleanupTool="true"/> - + enabledByDefault="true" level="INFORMATION" implementationClass="com.siyeh.ig.controlflow.ConfusingElseInspection"/> Also report when there are no more statements after the 'if' statement +confusing.else.option=Report when there are no more statements after the 'if' statement html.tag.can.be.javadoc.tag.display.name=... can be replaced with {@code ...} html.tag.can.be.javadoc.tag.problem.descriptor=#ref...\\</code\\> can be replaced with '{@code ...}' #loc html.tag.can.be.javadoc.tag.quickfix=Replace with '{@code ...}' diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/RedundantElseInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConfusingElseInspection.java similarity index 72% rename from plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/RedundantElseInspection.java rename to plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConfusingElseInspection.java index 232e1fe04987..32f7a383c14f 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/RedundantElseInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConfusingElseInspection.java @@ -16,8 +16,10 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; @@ -28,7 +30,12 @@ import org.intellij.lang.annotations.Pattern; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -public class RedundantElseInspection extends BaseInspection { +import javax.swing.*; + +public class ConfusingElseInspection extends BaseInspection { + + @SuppressWarnings({"PublicField"}) + public boolean reportWhenNoStatementFollow = true; @Pattern(VALID_ID_PATTERN) @Override @@ -49,9 +56,14 @@ public class RedundantElseInspection extends BaseInspection { return InspectionGadgetsBundle.message("redundant.else.problem.descriptor"); } + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("confusing.else.option"), this, "reportWhenNoStatementFollow"); + } + @Override public BaseInspectionVisitor buildVisitor() { - return new RedundantElseVisitor(); + return new ConfusingElseVisitor(); } @Override @@ -100,7 +112,7 @@ public class RedundantElseInspection extends BaseInspection { } } - private static class RedundantElseVisitor extends BaseInspectionVisitor { + private class ConfusingElseVisitor extends BaseInspectionVisitor { @Override public void visitIfStatement(@NotNull PsiIfStatement statement) { @@ -116,6 +128,17 @@ public class RedundantElseInspection extends BaseInspection { if (ControlFlowUtils.statementMayCompleteNormally(thenBranch)) { return; } + if (!reportWhenNoStatementFollow) { + final PsiStatement nextStatement = getNextStatement(statement); + if (nextStatement == null) { + return; + } + if (!ControlFlowUtils.statementMayCompleteNormally(elseBranch)) { + return; + // protecting against an edge case where both branches return + // and are followed by a case label + } + } final PsiElement elseToken = statement.getElseElement(); if (elseToken == null) { return; @@ -126,7 +149,7 @@ public class RedundantElseInspection extends BaseInspection { registerError(elseToken); } - private static boolean parentCompletesNormally(PsiElement element) { + private boolean parentCompletesNormally(PsiElement element) { PsiElement parent = element.getParent(); while (parent instanceof PsiIfStatement) { final PsiIfStatement ifStatement = (PsiIfStatement)parent; @@ -143,5 +166,21 @@ public class RedundantElseInspection extends BaseInspection { } return !(parent instanceof PsiCodeBlock); } + + @Nullable + private PsiStatement getNextStatement(PsiIfStatement statement) { + while (true) { + final PsiElement parent = statement.getParent(); + if (parent instanceof PsiIfStatement) { + final PsiIfStatement parentIfStatement = (PsiIfStatement)parent; + final PsiStatement elseBranch = parentIfStatement.getElseBranch(); + if (elseBranch == statement) { + statement = parentIfStatement; + continue; + } + } + return PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class); + } + } } } diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/RedundantElse.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ConfusingElse.html similarity index 100% rename from plugins/InspectionGadgets/src/inspectionDescriptions/RedundantElse.html rename to plugins/InspectionGadgets/src/inspectionDescriptions/ConfusingElse.html diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/redundant_else/RedundantElse.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/confusing_else/ConfusingElse.java similarity index 87% rename from plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/redundant_else/RedundantElse.java rename to plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/confusing_else/ConfusingElse.java index 4752ff8e0d97..5e3c411766ce 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/redundant_else/RedundantElse.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/confusing_else/ConfusingElse.java @@ -1,12 +1,12 @@ package com.siyeh.igtest.controlflow.confusing_else; -public class RedundantElse { +public class ConfusingElse { public static void main(String[] args) { if (foo()) { return; } else { - System.out.println("RedundantElseInspection.main"); + System.out.println("ConfusingElseInspection.main"); } bar(); } @@ -52,6 +52,14 @@ public class RedundantElse { return o; } + void lastElse(int i) { + if (i == 1) { + return; + } else { + System.out.println("i = " + i); + } + } + void elseIf(int i) { if (i == 1) { return; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/confusing_else/ConfusingLastElse.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/confusing_else/ConfusingLastElse.java new file mode 100644 index 000000000000..5c35f231aba9 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/confusing_else/ConfusingLastElse.java @@ -0,0 +1,93 @@ +package com.siyeh.igtest.controlflow.confusing_else; + +public class ConfusingLastElse { + + public static void main(String[] args) { + if (foo()) { + return; + } else { + System.out.println("ConfusingElseInspection.main"); + } + bar(); + } + + private static void bar() { + } + + private static boolean foo() { + return true; + } + + void two(boolean b) { + if (foo()) { + System.out.println(0); + } else if (b) { + return; + } else { + System.out.println(1); + } + bar(); + } + + void three(boolean b) { + switch (3) { + case 2: + if (foo()) { + return; + } else { + return; + } + case 3: + } + } + + public int foo(int o) { + if (o == 1) { + o = 2; + } else if (o == 2) { + return 1; + } else { + o = 4; + } + return o; + } + + void lastElse(int i) { + if (i == 1) { + return; + } else { + System.out.println("i = " + i); + } + } + + void elseIf(int i) { + if (i == 1) { + return; + } else if (i == 3) { + System.out.println("i = " + i); + } + System.out.println(); + } + + void elseIfChain(int i) { + while (true) { + if (i == 0) { + System.exit(i); + } + else if (i == 1) { + throw new RuntimeException(); + } + else if (i == 2) { + return; + } + else if (i == 3) { + break; + } + else if (i == 4) { + continue; + } else { + System.out.println(i); + } + } + } +} diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/RedundantElseInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConfusingElseInspectionTest.java similarity index 65% rename from plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/RedundantElseInspectionTest.java rename to plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConfusingElseInspectionTest.java index e5da52315f37..adaada1d9ed5 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/RedundantElseInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConfusingElseInspectionTest.java @@ -22,15 +22,28 @@ import org.jetbrains.annotations.Nullable; /** * @author Bas Leijdekkers */ -public class RedundantElseInspectionTest extends LightInspectionTestCase { +public class ConfusingElseInspectionTest extends LightInspectionTestCase { - public void testRedundantElse() { + private ConfusingElseInspection myInspection = new ConfusingElseInspection(); + + public void testConfusingElse() { doTest(); } + public void testConfusingLastElse() { + boolean oldValue = myInspection.reportWhenNoStatementFollow; + myInspection.reportWhenNoStatementFollow = false; + try { + doTest(); + } + finally { + myInspection.reportWhenNoStatementFollow = oldValue; + } + } + @Nullable @Override protected InspectionProfileEntry getInspection() { - return new RedundantElseInspection(); + return myInspection; } } \ No newline at end of file