From e212f2024d8bb501167dc3f50c222be85efc41b7 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 25 Nov 2019 10:40:42 +0700 Subject: [PATCH] TrivialIfInspection: ignoreChainedIf option (IDEA-227395) GitOrigin-RevId: 8764972719450901862996220cc6d97b2c5c24ff --- .../siyeh/InspectionGadgetsBundle.properties | 1 + .../ig/controlflow/TrivialIfInspection.java | 61 ++++++++++++++----- .../src/inspectionDescriptions/TrivialIf.html | 9 ++- .../controlflow/TrivialIfInspectionTest.java | 41 ++++++++++++- 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index b7698f813cc4..1772ac42a5b8 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -670,6 +670,7 @@ system.set.security.manager.display.name=Call to 'System.setSecurityManager()' system.set.security.manager.problem.descriptor=Call to System.#ref() may pose security concerns #loc control.flow.statement.without.braces.display.name=Control flow statement without braces trivial.if.display.name=Redundant 'if' statement +trivial.if.option.ignore.chained=Ignore chained 'if' statements thread.with.default.run.method.display.name=Instantiating a Thread with default 'run()' method while.loop.spins.on.field.display.name='while' loop spins on field while.loop.spins.on.field.fix.family.name=Fix spin loop diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/TrivialIfInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/TrivialIfInspection.java index 56998bf8b4d8..2e2622e6b2b4 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/TrivialIfInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/TrivialIfInspection.java @@ -17,6 +17,9 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInspection.CleanupLocalInspectionTool; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ProblemHighlightType; +import com.intellij.codeInspection.SetInspectionOptionFix; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; @@ -27,18 +30,22 @@ import com.intellij.psi.util.TypeConversionUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.DelegatingFix; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.psiutils.*; import org.intellij.lang.annotations.Pattern; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import javax.swing.*; import java.util.Objects; import static com.intellij.util.ObjectUtils.tryCast; public class TrivialIfInspection extends BaseInspection implements CleanupLocalInspectionTool { + public boolean ignoreChainedIf = false; + @Pattern(VALID_ID_PATTERN) @Override @NotNull @@ -46,6 +53,12 @@ public class TrivialIfInspection extends BaseInspection implements CleanupLocalI return "RedundantIfStatement"; } + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("trivial.if.option.ignore.chained"), this, "ignoreChainedIf"); + } + @Override @NotNull public String getDisplayName() { @@ -63,6 +76,20 @@ public class TrivialIfInspection extends BaseInspection implements CleanupLocalI return InspectionGadgetsBundle.message("trivial.if.problem.descriptor"); } + @NotNull + @Override + protected InspectionGadgetsFix[] buildFixes(Object... infos) { + boolean chainedIf = (boolean)infos[0]; + if (chainedIf) { + return new InspectionGadgetsFix[]{ + new TrivialIfFix(), + new DelegatingFix(new SetInspectionOptionFix( + this, "ignoreChainedIf", InspectionGadgetsBundle.message("trivial.if.option.ignore.chained"), true)) + }; + } + return new InspectionGadgetsFix[]{new TrivialIfFix()}; + } + @Override public InspectionGadgetsFix buildFix(Object... infos) { return new TrivialIfFix(); @@ -199,22 +226,26 @@ public class TrivialIfInspection extends BaseInspection implements CleanupLocalI @Override public BaseInspectionVisitor buildVisitor() { - return new TrivialIfVisitor(); - } - - private static class TrivialIfVisitor extends BaseInspectionVisitor { - - @Override - public void visitIfStatement(@NotNull PsiIfStatement ifStatement) { - super.visitIfStatement(ifStatement); - final PsiExpression condition = ifStatement.getCondition(); - if (condition == null) { - return; + return new BaseInspectionVisitor() { + @Override + public void visitIfStatement(@NotNull PsiIfStatement ifStatement) { + super.visitIfStatement(ifStatement); + boolean chainedIf = PsiTreeUtil.skipWhitespacesAndCommentsBackward(ifStatement) instanceof PsiIfStatement || + (ifStatement.getParent() instanceof PsiIfStatement && + ((PsiIfStatement)ifStatement.getParent()).getElseBranch() == ifStatement); + if (ignoreChainedIf && chainedIf && !isOnTheFly()) return; + final PsiExpression condition = ifStatement.getCondition(); + if (condition == null) { + return; + } + if (isTrivial(ifStatement)) { + PsiElement anchor = Objects.requireNonNull(ifStatement.getFirstChild()); + ProblemHighlightType level = + ignoreChainedIf && chainedIf ? ProblemHighlightType.INFORMATION : ProblemHighlightType.GENERIC_ERROR_OR_WARNING; + registerError(anchor, level, chainedIf); + } } - if (isTrivial(ifStatement)) { - registerStatementError(ifStatement); - } - } + }; } public static boolean isTrivial(PsiIfStatement ifStatement) { diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/TrivialIf.html b/plugins/InspectionGadgets/src/inspectionDescriptions/TrivialIf.html index b9a54dad162d..47a574395168 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/TrivialIf.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/TrivialIf.html @@ -16,7 +16,14 @@ can be simplified to return foo(); -

+

Use an option below to not display warning in case of chaining if statement. E.g.: + 

+    if (condition1) return true;
+    if (condition2) return false;
+    return true;
+
+ The fix action will still be available in this case. +

\ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/TrivialIfInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/TrivialIfInspectionTest.java index 374859d926ee..21327205b0b6 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/TrivialIfInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/TrivialIfInspectionTest.java @@ -56,9 +56,48 @@ public class TrivialIfInspectionTest extends LightJavaInspectionTestCase { "}"); } + public void testReturn() { + doMemberTest("\n" + + " boolean b(int x) {\n" + + " if (x > 20) return true;\n" + + " /*'if' statement can be simplified*/if/**/ (x > 0) return true;\n" + + " return false;\n" + + "}\n"); + } + + public void testReturnIgnoreChain() { + doMemberTest("\n" + + " boolean b(int x) {\n" + + " if (x > 20) return true;\n" + + " if (x > 0) return true;\n" + + " return false;\n" + + "}\n"); + } + + public void testReturnElseIf() { + doMemberTest("\n" + + " boolean b(int x) {\n" + + " if (x > 20) return true;\n" + + " else /*'if' statement can be simplified*/if/**/ (x > 0) return true;\n" + + " else return false;\n" + + "}\n"); + } + + public void testReturnElseIfIgnoreChain() { + doMemberTest("\n" + + " boolean b(int x) {\n" + + " if (x > 20) return true;\n" + + " else if (x > 0) return true;\n" + + " else return false;\n" + + "}\n"); + } @Override protected InspectionProfileEntry getInspection() { - return new TrivialIfInspection(); + TrivialIfInspection inspection = new TrivialIfInspection(); + if (getTestName(false).endsWith("IgnoreChain")) { + inspection.ignoreChainedIf = true; + } + return inspection; } }