IG: add way to simplify "x == y ? y : x" to "x" to "Redundant conditional expression...
authorBas Leijdekkers <basleijdekkers@gmail.com>
Tue, 25 Oct 2016 13:02:48 +0000 (15:02 +0200)
committerBas Leijdekkers <basleijdekkers@gmail.com>
Wed, 26 Oct 2016 10:41:08 +0000 (12:41 +0200)
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspection.java
plugins/InspectionGadgets/src/inspectionDescriptions/UnnecessaryConditionalExpression.html
plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/unnecessary_conditional_expression/UnnecessaryConditionalExpression.java [new file with mode: 0644]
plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspectionTest.java [new file with mode: 0644]

index 846946029562c7c3f6e8f6cfb920daa96ccf2431..630cbaef3dc02e32450377bb9ba7a418f5efdfd8 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2015 Dave Griffith, Bas Leijdekkers
+ * Copyright 2003-2016 Dave Griffith, Bas Leijdekkers
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,15 +18,19 @@ package com.siyeh.ig.controlflow;
 import com.intellij.codeInspection.CleanupLocalInspectionTool;
 import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.openapi.project.Project;
+import com.intellij.psi.JavaTokenType;
+import com.intellij.psi.PsiBinaryExpression;
 import com.intellij.psi.PsiConditionalExpression;
 import com.intellij.psi.PsiExpression;
-import com.intellij.util.IncorrectOperationException;
+import com.intellij.psi.tree.IElementType;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
 import com.siyeh.ig.InspectionGadgetsFix;
 import com.siyeh.ig.PsiReplacementUtil;
 import com.siyeh.ig.psiutils.BoolUtils;
+import com.siyeh.ig.psiutils.EquivalenceChecker;
+import com.siyeh.ig.psiutils.ParenthesesUtils;
 import org.jetbrains.annotations.NotNull;
 
 public class UnnecessaryConditionalExpressionInspection extends BaseInspection implements CleanupLocalInspectionTool {
@@ -40,8 +44,7 @@ public class UnnecessaryConditionalExpressionInspection extends BaseInspection i
   @Override
   @NotNull
   public String getDisplayName() {
-    return InspectionGadgetsBundle.message(
-      "unnecessary.conditional.expression.display.name");
+    return InspectionGadgetsBundle.message("unnecessary.conditional.expression.display.name");
   }
 
   @Override
@@ -57,35 +60,23 @@ public class UnnecessaryConditionalExpressionInspection extends BaseInspection i
   @Override
   @NotNull
   public String buildErrorString(Object... infos) {
-    final PsiConditionalExpression expression =
-      (PsiConditionalExpression)infos[0];
-    return InspectionGadgetsBundle.message(
-      "simplifiable.conditional.expression.problem.descriptor",
-      calculateReplacementExpression(expression));
-  }
-
-  static String calculateReplacementExpression(
-    PsiConditionalExpression exp) {
-    final PsiExpression thenExpression = exp.getThenExpression();
-    final PsiExpression elseExpression = exp.getElseExpression();
-    final PsiExpression condition = exp.getCondition();
-
-    if (BoolUtils.isFalse(thenExpression) &&
-        BoolUtils.isTrue(elseExpression)) {
-      return BoolUtils.getNegatedExpressionText(condition);
-    }
-    else {
-      return condition.getText();
-    }
+    final String replacement = (String)infos[0];
+    return InspectionGadgetsBundle.message("simplifiable.conditional.expression.problem.descriptor", replacement);
   }
 
   @Override
   public InspectionGadgetsFix buildFix(Object... infos) {
-    return new UnnecessaryConditionalFix();
+    final String replacement = (String)infos[0];
+    return new UnnecessaryConditionalFix(replacement);
   }
 
-  private static class UnnecessaryConditionalFix
-    extends InspectionGadgetsFix {
+  private static class UnnecessaryConditionalFix extends InspectionGadgetsFix {
+
+    private final String myReplacement;
+
+    public UnnecessaryConditionalFix(String replacement) {
+      myReplacement = replacement;
+    }
 
     @Override
     @NotNull
@@ -95,22 +86,16 @@ public class UnnecessaryConditionalExpressionInspection extends BaseInspection i
     }
 
     @Override
-    public void doFix(Project project, ProblemDescriptor descriptor)
-      throws IncorrectOperationException {
-      final PsiConditionalExpression expression =
-        (PsiConditionalExpression)descriptor.getPsiElement();
-      final String newExpression =
-        calculateReplacementExpression(expression);
-      PsiReplacementUtil.replaceExpression(expression, newExpression);
+    public void doFix(Project project, ProblemDescriptor descriptor) {
+      final PsiConditionalExpression expression = (PsiConditionalExpression)descriptor.getPsiElement();
+      PsiReplacementUtil.replaceExpression(expression, myReplacement);
     }
   }
 
-  private static class UnnecessaryConditionalExpressionVisitor
-    extends BaseInspectionVisitor {
+  private static class UnnecessaryConditionalExpressionVisitor extends BaseInspectionVisitor {
 
     @Override
-    public void visitConditionalExpression(
-      PsiConditionalExpression expression) {
+    public void visitConditionalExpression(PsiConditionalExpression expression) {
       super.visitConditionalExpression(expression);
       final PsiExpression thenExpression = expression.getThenExpression();
       if (thenExpression == null) {
@@ -120,12 +105,38 @@ public class UnnecessaryConditionalExpressionInspection extends BaseInspection i
       if (elseExpression == null) {
         return;
       }
-      if (BoolUtils.isFalse(thenExpression) &&
-          BoolUtils.isTrue(elseExpression) ||
-          BoolUtils.isTrue(thenExpression) &&
-          BoolUtils.isFalse(elseExpression)) {
-        registerError(expression, expression);
+      final PsiExpression condition = ParenthesesUtils.stripParentheses(expression.getCondition());
+      if (condition == null) {
+        return;
+      }
+      if (BoolUtils.isFalse(thenExpression) && BoolUtils.isTrue(elseExpression)) {
+        registerError(expression, BoolUtils.getNegatedExpressionText(condition));
+      }
+      else if (BoolUtils.isTrue(thenExpression) && BoolUtils.isFalse(elseExpression)) {
+        registerError(expression, condition.getText());
+      }
+      else if (isUnnecessary(condition, thenExpression, elseExpression, JavaTokenType.EQEQ)) {
+        registerError(expression, elseExpression.getText());
+      }
+      else if (isUnnecessary(condition, elseExpression, thenExpression, JavaTokenType.NE)) {
+        registerError(expression, thenExpression.getText());
+      }
+    }
+
+    boolean isUnnecessary(PsiExpression condition, PsiExpression thenExpression, PsiExpression elseExpression, IElementType expectedToken) {
+      if (!(condition instanceof PsiBinaryExpression)) {
+        return false;
+      }
+      final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)condition;
+      final IElementType token = binaryExpression.getOperationTokenType();
+      if (token != expectedToken) {
+        return false;
       }
+      final EquivalenceChecker equivalence = EquivalenceChecker.getCanonicalPsiEquivalence();
+      final PsiExpression lhs = binaryExpression.getLOperand();
+      final PsiExpression rhs = binaryExpression.getROperand();
+      return equivalence.expressionsAreEquivalent(thenExpression, lhs) && equivalence.expressionsAreEquivalent(elseExpression, rhs) ||
+             equivalence.expressionsAreEquivalent(thenExpression, rhs) && equivalence.expressionsAreEquivalent(elseExpression, lhs);
     }
   }
 }
\ No newline at end of file
index f839cfb974f040e73dac72f67618f8d23f4d25a4..d109005e5e99123772cecac24cb5237d76309f23 100644 (file)
@@ -1,8 +1,39 @@
 <html>
 <body>
-Reports conditional expressions of the form
-<b><i>condition</i>?true:false</b> or <b><i>condition</i>?false:true</b>. These expressions may be safely simplified
-to <b><i>condition</i></b> or <b>!<i>condition</i></b>, respectively.
+Reports conditional expressions which can be replaced by simpler but equivalent expressions.
+<table>
+  <tr><th>Example</th><th>&rarr;</th><th>Replacement</th></tr>
+
+  <tr>
+    <td>condition ? true : false</td>
+    <td></td>
+    <td>condition</td>
+  </tr>
+
+  <tr>
+    <td>condition ? false : true</td>
+    <td></td>
+    <td>!condition</td>
+  </tr>
+
+  <tr>
+    <td>value == null ? null : value</td>
+    <td></td>
+    <td>value</td>
+  </tr>
+
+  <tr>
+    <td>result != 0 ? result : 0</td>
+    <td></td>
+    <td>result</td>
+  </tr>
+
+  <tr>
+    <td>a == b ? a : b</td>
+    <td></td>
+    <td>b</td>
+  </tr>
+</table>
 <!-- tooltip end -->
 <p>
 
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/unnecessary_conditional_expression/UnnecessaryConditionalExpression.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/unnecessary_conditional_expression/UnnecessaryConditionalExpression.java
new file mode 100644 (file)
index 0000000..4ea7a85
--- /dev/null
@@ -0,0 +1,19 @@
+class UnnecessaryConditionalExpression {
+
+  void one(boolean condition) {
+    final boolean a = <warning descr="'condition ? true : false' can be simplified to 'condition'">condition ? true : false</warning>;
+    final boolean b = <warning descr="'condition ? false : true' can be simplified to '!condition'">condition ? false : true</warning>;
+  }
+
+  int two(int i) {
+    return <warning descr="'i == 0 ? 0 : i' can be simplified to 'i'">i == 0 ? 0 : i</warning>;
+  }
+
+  Object three(Object o) {
+    return <warning descr="'o != null ? o : null' can be simplified to 'o'">o != null ? o : null</warning>;
+  }
+
+  int four(int a, int b) {
+    return <warning descr="'a == b ? a : b' can be simplified to 'b'">a == b ? a : b</warning>;
+  }
+}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspectionTest.java
new file mode 100644 (file)
index 0000000..5ddac13
--- /dev/null
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2000-2016 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 UnnecessaryConditionalExpressionInspectionTest extends LightInspectionTestCase {
+
+  public void testUnnecessaryConditionalExpression() {
+    doTest();
+  }
+
+  @Nullable
+  @Override
+  protected InspectionProfileEntry getInspection() {
+    return new UnnecessaryConditionalExpressionInspection();
+  }
+}