IG: split off "Conditional can be pushed inside branch expression" inspection

from "Conditional expression with identical branches" inspection

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml

@@ -603,6 +603,10 @@
                      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"/>
+    <localInspection groupPath="Java" language="JAVA" shortName="ConditionalCanBePushedInsideExpression" bundle="com.siyeh.InspectionGadgetsBundle"
+                     key="conditional.can.be.pushed.inside.expression.display.name" groupBundle="messages.InspectionsBundle"
+                     groupKey="group.names.control.flow.issues" enabledByDefault="true" level="INFORMATION"
+                     implementationClass="com.siyeh.ig.controlflow.ConditionalCanBePushedInsideExpressionInspection"/>
     <localInspection groupPath="Java" language="JAVA" suppressId="ConfusingElseBranch" shortName="ConfusingElse" bundle="com.siyeh.InspectionGadgetsBundle"
                      key="redundant.else.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.control.flow.issues"
                      enabledByDefault="true" level="INFORMATION" implementationClass="com.siyeh.ig.controlflow.ConfusingElseInspection"/>

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties

@@ -816,7 +816,10 @@ enumerated.constant.naming.convention.element.description=Enumerated constant
 final.method.display.name='final' method
 transient.field.in.non.serializable.class.display.name=Transient field in non-serializable class
 bad.exception.thrown.display.name=Prohibited exception thrown
-conditional.expression.with.identical.branches.display.name=Conditional expression with identical or similar branches
+conditional.expression.with.identical.branches.display.name=Conditional expression with identical branches
+conditional.can.be.pushed.inside.expression.display.name=Conditional can be pushed inside branch expression
+conditional.can.be.pushed.inside.expression.option=Ignore when conditional will be only argument of a method call
+conditional.can.be.pushed.inside.expression.quickfix=Push conditional expression inside branch
 raw.use.of.parameterized.type.display.name=Raw use of parameterized class
 standard.variable.names.display.name=Standard variable names
 instance.variable.naming.convention.display.name=Instance field naming convention
@@ -858,7 +861,7 @@ switch.statements.without.default.problem.descriptor=<code>#ref</code> statement
 default.not.last.case.in.switch.problem.descriptor=<code>#ref</code> branch not last case in 'switch' statement #loc
 loop.statements.that.dont.loop.problem.descriptor=<code>#ref</code> statement does not loop #loc
 conditional.expression.with.identical.branches.problem.descriptor=Conditional expression <code>#ref</code> with identical branches #loc
-conditional.expression.with.similar.branches.problem.descriptor=Conditional expression <code>#ref</code> with similar branches #loc
+conditional.can.be.pushed.inside.expression.problem.descriptor=Conditional expression can be pushed inside branch #loc
 if.statement.with.identical.branches.problem.descriptor=<code>#ref</code> statement with identical branches #loc
 duplicate.condition.problem.descriptor=Duplicate condition <code>#ref</code> #loc
 duplicate.condition.ignore.method.calls.option=Ignore method calls in condition
@@ -1123,8 +1126,6 @@ standard.variable.names.ignore.override.option=Ignore for parameter names identi
 static.variable.naming.convention.mutable.option=Check 'static final' fields with a mutable type
 boolean.method.name.must.start.with.question.table.column.name=Boolean method name prefix
 conditional.expression.with.identical.branches.collapse.quickfix=Collapse conditional expression
-conditional.expression.with.identical.branches.push.inside.quickfix=Push conditional inside expression
-conditional.expression.with.identical.branches.collapse.quickfix.family=Conditional expression can be simplified
 redundant.else.unwrap.quickfix=Remove redundant 'else'
 constant.conditional.expression.problem.descriptor=<code>#ref</code> can be simplified to ''{0}'' #loc
 constant.conditional.expression.simplify.quickfix=Simplify

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspection.java

@@ -0,0 +1,141 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.siyeh.ig.controlflow;
+
+import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.codeInspection.ProblemHighlightType;
+import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.*;
+import com.siyeh.InspectionGadgetsBundle;
+import com.siyeh.ig.BaseInspection;
+import com.siyeh.ig.BaseInspectionVisitor;
+import com.siyeh.ig.InspectionGadgetsFix;
+import com.siyeh.ig.psiutils.EquivalenceChecker;
+import com.siyeh.ig.psiutils.ParenthesesUtils;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import javax.swing.*;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class ConditionalCanBePushedInsideExpressionInspection extends BaseInspection {
+
+  public boolean ignoreSingleArgument = true;
+
+  @Nls
+  @NotNull
+  @Override
+  public String getDisplayName() {
+    return InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.display.name");
+  }
+
+  @NotNull
+  @Override
+  protected String buildErrorString(Object... infos) {
+    return InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.problem.descriptor");
+  }
+
+  @Nullable
+  @Override
+  public JComponent createOptionsPanel() {
+    return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.option"),
+                                          this, "ignoreSingleArgument");
+  }
+
+  @Nullable
+  @Override
+  protected InspectionGadgetsFix buildFix(Object... infos) {
+    return new PushConditionalInsideFix();
+  }
+
+  private static class PushConditionalInsideFix extends InspectionGadgetsFix {
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.quickfix");
+    }
+
+    @Override
+    protected void doFix(Project project, ProblemDescriptor descriptor) {
+      final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)descriptor.getPsiElement();
+      final PsiExpression thenExpression = conditionalExpression.getThenExpression();
+      if (thenExpression == null) {
+        return;
+      }
+      final EquivalenceChecker.Match match =
+        EquivalenceChecker.getCanonicalPsiEquivalence().expressionsMatch(thenExpression, conditionalExpression.getElseExpression());
+      if (!match.isPartialMatch()) {
+        return;
+      }
+      final PsiElement leftDiff = match.getLeftDiff();
+      final PsiElement rightDiff = match.getRightDiff();
+      final String expression = "(" + conditionalExpression.getCondition().getText() + " ? " +
+                                leftDiff.getText() + " : " + rightDiff.getText() + ")";
+      final PsiExpression newConditionalExpression =
+        JavaPsiFacade.getElementFactory(project).createExpressionFromText(expression, conditionalExpression);
+      final PsiElement replacedConditionalExpression = leftDiff.replace(newConditionalExpression);
+      ParenthesesUtils.removeParentheses((PsiExpression)replacedConditionalExpression, false);
+      conditionalExpression.replace(thenExpression);
+    }
+  }
+
+  @Override
+  public BaseInspectionVisitor buildVisitor() {
+    return new ConditionalCanBePushedInsideExpressionVisitor();
+  }
+
+  private  class ConditionalCanBePushedInsideExpressionVisitor extends BaseInspectionVisitor {
+
+    @Override
+    public void visitConditionalExpression(PsiConditionalExpression expression) {
+      super.visitConditionalExpression(expression);
+      final PsiExpression thenExpression = expression.getThenExpression();
+      if (thenExpression == null) {
+        return;
+      }
+      final PsiExpression elseExpression = expression.getElseExpression();
+      final EquivalenceChecker.Match match =
+        EquivalenceChecker.getCanonicalPsiEquivalence().expressionsMatch(thenExpression, elseExpression);
+      if (match.isExactMismatch() || match.isExactMatch()) {
+        return;
+      }
+      registerError(expression, ignoreSingleArgument && isOnlyArgumentOfMethodCall(match.getLeftDiff())
+                                ? ProblemHighlightType.INFORMATION
+                                : ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
+    }
+
+    private boolean isOnlyArgumentOfMethodCall(PsiElement element) {
+      if (element == null) {
+        return false;
+      }
+      final PsiElement parent = element.getParent();
+      if (!(parent instanceof PsiExpressionList)) {
+        return false;
+      }
+      final PsiExpressionList expressionList = (PsiExpressionList)parent;
+      if (expressionList.getExpressions().length != 1) {
+        return false;
+      }
+      final PsiElement grandParent = expressionList.getParent();
+      return grandParent instanceof PsiMethodCallExpression;
+    }
+  }
+}

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalExpressionWithIdenticalBranchesInspection.java

@@ -17,7 +17,6 @@ package com.siyeh.ig.controlflow;
 
 import com.intellij.codeInspection.CleanupLocalInspectionTool;
 import com.intellij.codeInspection.ProblemDescriptor;
-import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.siyeh.InspectionGadgetsBundle;
@@ -26,20 +25,9 @@ import com.siyeh.ig.BaseInspectionVisitor;
 import com.siyeh.ig.InspectionGadgetsFix;
 import com.siyeh.ig.PsiReplacementUtil;
 import com.siyeh.ig.psiutils.EquivalenceChecker;
-import com.siyeh.ig.psiutils.ParenthesesUtils;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
-
-import javax.swing.*;
 
 public class ConditionalExpressionWithIdenticalBranchesInspection extends BaseInspection implements CleanupLocalInspectionTool {
-  public boolean myReportOnlyExactlyIdentical;
-
-  @Nullable
-  @Override
-  public JComponent createOptionsPanel() {
-    return new SingleCheckboxOptionsPanel("Report only exactly identical branches", this, "myReportOnlyExactlyIdentical");
-  }
 
   @Override
   @NotNull
@@ -50,70 +38,34 @@ public class ConditionalExpressionWithIdenticalBranchesInspection extends BaseIn
   @Override
   @NotNull
   protected String buildErrorString(Object... infos) {
-    final EquivalenceChecker.Match decision = (EquivalenceChecker.Match)infos[1];
-    return InspectionGadgetsBundle.message(decision.isPartialMatch()
-                                           ? "conditional.expression.with.similar.branches.problem.descriptor"
-                                           : "conditional.expression.with.identical.branches.problem.descriptor");
+    return InspectionGadgetsBundle.message("conditional.expression.with.identical.branches.problem.descriptor");
   }
 
   @Override
   public InspectionGadgetsFix buildFix(Object... infos) {
-    return new CollapseConditional((PsiConditionalExpression)infos[0]);
+    return new CollapseConditionalFix();
   }
 
-  private static class CollapseConditional extends InspectionGadgetsFix {
-    private final SmartPsiElementPointer<PsiConditionalExpression> myConditionalExpression;
-
-    public CollapseConditional(PsiConditionalExpression expression) {
-      myConditionalExpression = SmartPointerManager.getInstance(expression.getProject()).createSmartPsiElementPointer(expression);
-    }
-
-    @Override
-    @NotNull
-    public String getName() {
-      return InspectionGadgetsBundle.message(getEquivalenceDecision().isExactMatch()
-                                             ? "conditional.expression.with.identical.branches.collapse.quickfix"
-                                             : "conditional.expression.with.identical.branches.push.inside.quickfix");
-    }
+  private static class CollapseConditionalFix extends InspectionGadgetsFix {
 
     @Override
     @NotNull
     public String getFamilyName() {
-      return InspectionGadgetsBundle.message("conditional.expression.with.identical.branches.collapse.quickfix.family");
-    }
-
-    public PsiConditionalExpression getConditionalExpression() {
-      return myConditionalExpression.getElement();
-    }
-
-    private EquivalenceChecker.Match getEquivalenceDecision() {
-      return EquivalenceChecker.getCanonicalPsiEquivalence()
-        .expressionsMatch(getConditionalExpression().getThenExpression(), getConditionalExpression().getElseExpression());
+      return InspectionGadgetsBundle.message("conditional.expression.with.identical.branches.collapse.quickfix");
     }
 
     @Override
     public void doFix(Project project, ProblemDescriptor descriptor) {
-      final EquivalenceChecker.Match decision = getEquivalenceDecision();
-      final PsiConditionalExpression conditionalExpression = getConditionalExpression();
+      final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)descriptor.getPsiElement();
       final PsiExpression thenExpression = conditionalExpression.getThenExpression();
-      assert thenExpression != null;
-      if (decision.isExactMatch()) {
-        final PsiConditionalExpression expression = (PsiConditionalExpression)descriptor.getPsiElement();
-        final String bodyText = thenExpression.getText();
-        PsiReplacementUtil.replaceExpression(expression, bodyText);
-      } else if (!decision.isExactMismatch()) {
-        final PsiElement leftDiff = decision.getLeftDiff();
-        final PsiElement rightDiff = decision.getRightDiff();
-
-        final String expression = "(" + conditionalExpression.getCondition().getText() + " ? " + leftDiff.getText() + " : " + rightDiff.getText() + ")";
-        final PsiExpression newConditionalExpression =
-          JavaPsiFacade.getElementFactory(project).createExpressionFromText(expression, conditionalExpression);
-
-        final PsiElement replacedConditionalExpression = leftDiff.replace(newConditionalExpression);
-        ParenthesesUtils.removeParentheses((PsiExpression)replacedConditionalExpression, false);
-        conditionalExpression.replace(thenExpression);
+      if (thenExpression == null) {
+        return;
       }
-     }
+      final PsiExpression elseExpression = conditionalExpression.getElseExpression();
+      if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(thenExpression, elseExpression)) {
+        PsiReplacementUtil.replaceExpression(conditionalExpression, thenExpression.getText());
+      }
+    }
   }
 
   @Override
@@ -121,17 +73,18 @@ public class ConditionalExpressionWithIdenticalBranchesInspection extends BaseIn
     return new ConditionalExpressionWithIdenticalBranchesVisitor();
   }
 
-  private class ConditionalExpressionWithIdenticalBranchesVisitor extends BaseInspectionVisitor {
+  private static class ConditionalExpressionWithIdenticalBranchesVisitor extends BaseInspectionVisitor {
 
     @Override
     public void visitConditionalExpression(PsiConditionalExpression expression) {
       super.visitConditionalExpression(expression);
       final PsiExpression thenExpression = expression.getThenExpression();
+      if (thenExpression == null) {
+        return;
+      }
       final PsiExpression elseExpression = expression.getElseExpression();
-      final EquivalenceChecker.Match decision = EquivalenceChecker.getCanonicalPsiEquivalence()
-        .expressionsMatch(thenExpression, elseExpression);
-      if (thenExpression != null && (myReportOnlyExactlyIdentical ? decision.isExactMatch() : !decision.isExactMismatch())) {
-        registerError(expression, expression, decision);
+      if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(thenExpression, elseExpression)) {
+        registerError(expression);
       }
     }
   }

plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalCanBePushedInsideExpression.html

@@ -0,0 +1,12 @@
+<html>
+<body>
+Reports conditional expressions with <em>then</em> and <em>else</em> branches so similar that the conditional expression can be pushed inside, thereby shortening the code.
+<p>For example the following conditional expression:
+  <pre><code>condition ? message("value: " + 1) : message("value: " + 2)</code></pre>
+Can be pushed inside and transformed into:
+  <pre><code>message("value: " + (condition ? 1 : 2))</code></pre>
+<!-- tooltip end -->
+<p>
+  <small>New in 2017.2</small>
+</body>
+</html>

plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalExpressionWithIdenticalBranches.html

@@ -1,8 +1,7 @@
 <html>
 <body>
-Reports conditional expressions
-with identical or similar "then" and "else" branches. Such expressions are almost certainly
-programmer error.
+Reports conditional expressions with identical <em>then</em> and <em>else</em> branches.
+Such expressions are almost certainly a mistake.
 <!-- tooltip end -->
 <p>
 

plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_can_be_pushed_inside_expression/ConditionalCanBePushedInsideExpression.java

@@ -0,0 +1,56 @@
+import java.util.Random;
+
+class ConditionalCanBePushedInsideExpression {
+
+  void fuzzy() {
+    String someString = <warning descr="Conditional expression can be pushed inside branch">new Random().nextBoolean() ? "2" + "q" + "1" : "2" + "qwe" + "1"</warning>;
+  }
+
+  void fuzzy2() {
+    Object someString = <warning descr="Conditional expression can be pushed inside branch">new Random().nextBoolean() ? (Object) "1" : (Object) "2"</warning>;
+  }
+
+  void fuzzy3() {
+    Object someString = <warning descr="Conditional expression can be pushed inside branch">new Random().nextBoolean() ? "21" + (Object) "1" : "21" + (Object) "2"</warning>;
+  }
+
+  void fuzzy4(int[] ints) {
+    int i = <warning descr="Conditional expression can be pushed inside branch">new Random().nextBoolean() ? ints[3] : ints[4]</warning>;
+  }
+
+  void fuzzy5(String[] strings) {
+    String s = <warning descr="Conditional expression can be pushed inside branch">new Random().nextBoolean()? "asd" + strings[2] : "qwe" + strings[2]</warning>;
+  }
+
+  void fuzzy6() {
+    int j = <warning descr="Conditional expression can be pushed inside branch">new Random().nextBoolean() ? 6 + someMethod("123", "") : 6 + someMethod("321", "")</warning>;
+  }
+
+  void fuzzy7(int k) {
+    int i = k == 10 ? singleParameterMethod("one") : singleParameterMethod("two");
+  }
+
+  int someMethod(String s, String s2) {
+    return s.length();
+  }
+
+  int singleParameterMethod(String s) {
+    return s.length();
+  }
+
+  class Item {
+    Item(String name) {
+    }
+
+    Item(int value) {
+    }
+
+    void v() {
+      int i = 1;
+      Item item = (i == 1 ? new Item("1") : new Item(i)); // warning here
+
+      Item item1 = (<warning descr="Conditional expression can be pushed inside branch">i == 1 ? new Item("1") : new Item("2")</warning>); // warning here
+    }
+  }
+
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_expression_with_identical_branches/ConditionalExpressionWithIdenticalBranches.java

@@ -1,7 +1,5 @@
 package com.siyeh.igtest.controlflow.conditional_expression_with_identical_branches;
 
-import java.util.Random;
-
 class ConditionalExpressionWithIdenticalBranches {
 
   int one(boolean b) {
@@ -20,49 +18,10 @@ class ConditionalExpressionWithIdenticalBranches {
     return b?<EOLError descr="Expression expected"></EOLError><EOLError descr="';' expected"></EOLError>
   }
 
-  void fuzzy() {
-    String someString = <warning descr="Conditional expression 'new Random().nextBoolean() ? \"2\" + \"q\" + \"1\" : \"2\" + \"qwe\" + \"1\"' with similar branches">new Random().nextBoolean() ? "2" + "q" + "1" : "2" + "qwe" + "1"</warning>;
-  }
-
-  void fuzzy2() {
-    Object someString = <warning descr="Conditional expression 'new Random().nextBoolean() ? (Object) \"1\" : (Object) \"2\"' with similar branches">new Random().nextBoolean() ? (Object) "1" : (Object) "2"</warning>;
-  }
-
-  void fuzzy3() {
-    Object someString = <warning descr="Conditional expression 'new Random().nextBoolean() ? \"21\" + (Object) \"1\" : \"21\" + (Object) \"2\"' with similar branches">new Random().nextBoolean() ? "21" + (Object) "1" : "21" + (Object) "2"</warning>;
-  }
-
-  void fuzzy4(int[] ints) {
-    int i = <warning descr="Conditional expression 'new Random().nextBoolean() ? ints[3] : ints[4]' with similar branches">new Random().nextBoolean() ? ints[3] : ints[4]</warning>;
-  }
-
-  void fuzzy5(String[] strings) {
-    String s = <warning descr="Conditional expression 'new Random().nextBoolean()? \"asd\" + strings[2] : \"qwe\" + strings[2]' with similar branches">new Random().nextBoolean()? "asd" + strings[2] : "qwe" + strings[2]</warning>;
-  }
-
-  void fuzzy6() {
-    int j = <warning descr="Conditional expression 'new Random().nextBoolean() ? 6 + someMethod(\"123\", \"\") : 6 + someMethod(\"321\", \"\")' with similar branches">new Random().nextBoolean() ? 6 + someMethod("123", "") : 6 + someMethod("321", "")</warning>;
-  }
-
   int someMethod(String s, String s2) {
     return s.length();
   }
 
-  class Item {
-    Item(String name) {
-    }
-
-    Item(int value) {
-    }
-
-    void v() {
-      int i = 1;
-      Item item = (i == 1 ? new Item("1") : new Item(i)); // warning here
-
-      Item item1 = (<warning descr="Conditional expression 'i == 1 ? new Item(\"1\") : new Item(\"2\")' with similar branches">i == 1 ? new Item("1") : new Item("2")</warning>); // warning here
-    }
-  }
-
   class A {
     private String test(String... s) {
       return "";

plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspectionTest.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.siyeh.ig.controlflow;
+
+import com.intellij.codeInspection.InspectionProfileEntry;
+import com.siyeh.ig.LightInspectionTestCase;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class ConditionalCanBePushedInsideExpressionInspectionTest extends LightInspectionTestCase {
+
+  public void testConditionalCanBePushedInsideExpression() {
+    doTest();
+  }
+
+  @Nullable
+  @Override
+  protected InspectionProfileEntry getInspection() {
+    return new ConditionalCanBePushedInsideExpressionInspection();
+  }
+}