TrivialIfInspection: ignoreChainedIf option (IDEA-227395)

GitOrigin-RevId: 8764972719450901862996220cc6d97b2c5c24ff

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 <code>System.#ref()</code> 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

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) {

plugins/InspectionGadgets/src/inspectionDescriptions/TrivialIf.html

@@ -16,7 +16,14 @@ can be simplified to
     <b>return</b> foo();
 </pre></code>
 <!-- tooltip end -->
-<p>
+<p>Use an option below to not display warning in case of chaining if statement. E.g.:
+  <code><pre>
+    if (condition1) return true;
+    if (condition2) return false;
+    return true;
+  </pre></code>
+  The fix action will still be available in this case.
+</p>
 
 </body>
 </html>

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;
   }
 }