expressions, String token, boolean negate, StringBuilder out) {
+ if (expressions.size() == 1) {
+ final PsiExpression expression = expressions.get(0);
+ if (!negate) {
+ out.append(expression.getText());
+ return;
+ }
+ if (ComparisonUtils.isComparison(expression)) {
+ final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)expression;
+ final String negatedComparison = ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
+ final PsiExpression lhs = binaryExpression.getLOperand();
+ final PsiExpression rhs = binaryExpression.getROperand();
+ assert rhs != null;
+ out.append(lhs.getText()).append(negatedComparison).append(rhs.getText());
+ }
+ else {
+ if (ParenthesesUtils.getPrecedence(expression) > ParenthesesUtils.PREFIX_PRECEDENCE) {
+ out.append("!(").append(expression.getText()).append(')');
+ }
+ else {
+ out.append('!').append(expression.getText());
+ }
}
- return InspectionGadgetsBundle.message("boolean.expression.can.be.simplified.polyadic.problem.descriptor");
}
else {
- final PsiPrefixExpression expression = (PsiPrefixExpression)infos[0];
- final PsiExpression e = removeRedundantNots(expression);
-
- if (e == null) {
- return InspectionGadgetsBundle.message("boolean.expression.can.be.simplified.problem.descriptor", "");
+ if (negate) {
+ out.append("!(");
}
+ boolean useToken = false;
+ for (PsiExpression expression : expressions) {
+ if (useToken) {
+ out.append(token);
+ }
+ else {
+ useToken = true;
+ }
+ buildSimplifiedExpression(expression, out);
+ }
+ if (negate) {
+ out.append(')');
+ }
+ }
+ }
- return InspectionGadgetsBundle.message(
- "boolean.expression.can.be.simplified.problem.descriptor",
- e instanceof PsiPrefixExpression ? calculateSimplifiedPrefixExpression((PsiPrefixExpression)e) : e.getText());
+ private void buildSimplifiedPrefixExpression(PsiPrefixExpression expression, StringBuilder out) {
+ final PsiJavaToken sign = expression.getOperationSign();
+ final IElementType tokenType = sign.getTokenType();
+ final PsiExpression operand = expression.getOperand();
+ if (JavaTokenType.EXCL.equals(tokenType)) {
+ final Boolean value = evaluate(operand);
+ if (value == Boolean.TRUE) {
+ out.append(PsiKeyword.FALSE);
+ return;
+ }
+ else if (value == Boolean.FALSE) {
+ out.append(PsiKeyword.TRUE);
+ return;
+ }
+ }
+ buildSimplifiedExpression(operand, out.append(sign.getText()));
+ }
+
+ @Override
+ public InspectionGadgetsFix buildFix(Object... infos) {
+ return new PointlessBooleanExpressionFix();
+ }
+
+ private class PointlessBooleanExpressionFix extends InspectionGadgetsFix {
+
+ @NotNull
+ public String getName() {
+ return InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix");
+ }
+
+ @Override
+ public void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException {
+ final PsiElement element = descriptor.getPsiElement();
+ if (!(element instanceof PsiExpression)) {
+ return;
+ }
+ final PsiExpression expression = (PsiExpression)element;
+ replaceExpression(expression, buildSimplifiedExpression(expression, new StringBuilder()).toString());
}
}
@@ -105,297 +275,142 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
return new PointlessBooleanExpressionVisitor();
}
- @Nullable
- private String calculateSimplifiedBinaryExpression(
- PsiBinaryExpression expression) {
- final PsiExpression lhs = expression.getLOperand();
-
- final PsiExpression rhs = expression.getROperand();
- if (rhs == null) {
- return null;
- }
- final IElementType tokenType = expression.getOperationTokenType();
- final String rhsText = rhs.getText();
- final String lhsText = lhs.getText();
- if (tokenType.equals(JavaTokenType.ANDAND) ||
- tokenType.equals(JavaTokenType.AND)) {
- if (isAlwaysTrue(lhs)) {
- return rhsText;
- }
- else if (isAlwaysFalse(lhs) || isAlwaysFalse(rhs)) {
- return "false";
- }
- else {
- return lhsText;
- }
- }
- else if (tokenType.equals(JavaTokenType.OROR) ||
- tokenType.equals(JavaTokenType.OR)) {
- if (isAlwaysFalse(lhs)) {
- return rhsText;
- }
- else {
- return lhsText;
- }
- }
- else if (tokenType.equals(JavaTokenType.XOR) ||
- tokenType.equals(JavaTokenType.NE)) {
- if (isAlwaysFalse(lhs)) {
- return rhsText;
- }
- else if (isAlwaysFalse(rhs)) {
- return lhsText;
- }
- else if (isAlwaysTrue(lhs)) {
- return createStringForNegatedExpression(rhs);
- }
- else {
- return createStringForNegatedExpression(lhs);
- }
- }
- else if (tokenType.equals(JavaTokenType.EQEQ)) {
- if (isAlwaysTrue(lhs)) {
- return rhsText;
- }
- else if (isAlwaysTrue(rhs)) {
- return lhsText;
- }
- else if (isAlwaysFalse(lhs)) {
- return createStringForNegatedExpression(rhs);
- }
- else {
- return createStringForNegatedExpression(lhs);
- }
- }
- else {
- return "";
- }
- }
-
- private static String createStringForNegatedExpression(PsiExpression exp) {
- if (ComparisonUtils.isComparison(exp)) {
- final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)exp;
- final String negatedComparison = ComparisonUtils.getNegatedComparison(binaryExpression.getOperationTokenType());
- final PsiExpression lhs = binaryExpression.getLOperand();
- final PsiExpression rhs = binaryExpression.getROperand();
- assert rhs != null;
- return lhs.getText() + negatedComparison + rhs.getText();
- }
- else {
- if (ParenthesesUtils.getPrecedence(exp) >
- ParenthesesUtils.PREFIX_PRECEDENCE) {
- return "!(" + exp.getText() + ')';
- }
- else {
- return '!' + exp.getText();
- }
- }
- }
-
- @NotNull
- private String calculateSimplifiedPrefixExpression(PsiPrefixExpression e) {
- final PsiExpression expression = e.getOperand();
-
- final Boolean value = evaluate(expression);
-
- if (value == Boolean.TRUE) {
- return PsiKeyword.FALSE;
- }
-
- if (value == Boolean.FALSE) {
- return PsiKeyword.TRUE;
- }
-
- return expression != null ? expression.getText() : "";
- }
-
- @Nullable
- private static PsiExpression removeRedundantNots(@NotNull PsiPrefixExpression expression) {
- if (!JavaTokenType.EXCL.equals(expression.getOperationTokenType())) {
- return expression;
- }
-
- final PsiExpression operand = expression.getOperand();
- if (operand == null || !(operand instanceof PsiPrefixExpression)) {
- return expression;
- }
-
- final PsiPrefixExpression prefixOperand = (PsiPrefixExpression)operand;
- if (!JavaTokenType.EXCL.equals(prefixOperand.getOperationTokenType())) {
- return expression;
- }
-
- final PsiExpression op = prefixOperand.getOperand();
- return op != null && op instanceof PsiPrefixExpression ? removeRedundantNots((PsiPrefixExpression)op) : op;
- }
-
- @Override
- public InspectionGadgetsFix buildFix(Object... infos) {
- return new BooleanLiteralComparisonFix();
- }
-
- private class BooleanLiteralComparisonFix extends InspectionGadgetsFix {
- @NotNull
- public String getName() {
- return InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix");
- }
-
- @Override
- public void doFix(Project project, ProblemDescriptor descriptor)
- throws IncorrectOperationException {
- final PsiElement element = descriptor.getPsiElement();
- processSubExpressions(project, (PsiExpression)element);
- }
-
- private boolean processSubExpressions(Project project, PsiExpression element) {
- if (element instanceof PsiPrefixExpression) {
- return processPrefixExpression(project, (PsiPrefixExpression)element);
- }
-
- if (element instanceof PsiPolyadicExpression) {
- final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)element;
- for (PsiExpression operand : polyadicExpression.getOperands()) {
- final Boolean bool = evaluate(operand);
- if (bool != null) {
- SimplifyBooleanExpressionFix.simplifyExpression(project, operand, bool);
- return true;
- }
- if (processSubExpressions(project, operand)) {
- return true;
- }
- }
- }
- return false;
- }
-
- private boolean processPrefixExpression(Project project, PsiPrefixExpression expression) {
- final Boolean value = evaluate(expression);
-
- if (value == null && JavaTokenType.EXCL.equals(expression.getOperationTokenType())) {
- final PsiExpression fixed = removeRedundantNots(expression);
- if (fixed != null && fixed != expression) {
- expression.replace(fixed);
- }
- }
- else {
- SimplifyBooleanExpressionFix.simplifyExpression(project, expression, value);
- }
- return false;
- }
- }
-
private class PointlessBooleanExpressionVisitor extends BaseInspectionVisitor {
- @Override
- public void visitClass(@NotNull PsiClass aClass) {
- //to avoid drilldown
- }
-
@Override
public void visitPolyadicExpression(PsiPolyadicExpression expression) {
super.visitPolyadicExpression(expression);
- final IElementType sign = expression.getOperationTokenType();
- if (!booleanTokens.contains(sign)) {
+ checkExpression(expression);
+ }
+
+ @Override
+ public void visitPrefixExpression(PsiPrefixExpression expression) {
+ super.visitPrefixExpression(expression);
+ checkExpression(expression);
+ }
+
+ private void checkExpression(PsiExpression expression) {
+ if (!isPointlessBooleanExpression(expression)) {
return;
}
- final PsiExpression[] operands = expression.getOperands();
- for (PsiExpression operand : operands) {
- if (operand == null) {
- return;
- }
- final PsiType opType = operand.getType();
- if (opType == null) {
- return;
- }
- if (!opType.equals(PsiType.BOOLEAN) &&
- !opType.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) {
- return;
- }
- }
-
- final boolean isPointless;
- if (sign.equals(JavaTokenType.EQEQ) || sign.equals(JavaTokenType.NE)) {
- isPointless = equalityExpressionIsPointless(operands);
- }
- else if (sign.equals(JavaTokenType.ANDAND) || sign.equals(JavaTokenType.AND)) {
- isPointless = andExpressionIsPointless(operands);
- }
- else if (sign.equals(JavaTokenType.OROR) || sign.equals(JavaTokenType.OR)) {
- isPointless = orExpressionIsPointless(operands);
- }
- else if (sign.equals(JavaTokenType.XOR)) {
- isPointless = xorExpressionIsPointless(operands);
- }
- else {
- isPointless = false;
- }
- if (!isPointless) {
+ final PsiElement parent = ParenthesesUtils.getParentSkipParentheses(expression);
+ if (parent instanceof PsiExpression && isPointlessBooleanExpression((PsiExpression)parent)) {
return;
}
registerError(expression, expression);
}
- @Override
- public void visitPrefixExpression(@NotNull PsiPrefixExpression expression) {
- super.visitPrefixExpression(expression);
- final PsiExpression operand = expression.getOperand();
- final IElementType tokenType = expression.getOperationTokenType();
- if (JavaTokenType.EXCL.equals(tokenType) && notExpressionIsPointless(operand)) {
- registerError(expression, expression);
+ private boolean isPointlessBooleanExpression(PsiExpression expression) {
+ if (expression instanceof PsiPrefixExpression) {
+ return evaluate(expression) != null;
}
- }
-
- private boolean equalityExpressionIsPointless(PsiExpression... lhs) {
- for (PsiExpression expression : lhs) {
- if (evaluate(expression) != null) return true;
- }
- return false;
- }
-
- private boolean andExpressionIsPointless(PsiExpression... lhs) {
- return equalityExpressionIsPointless(lhs);
- }
-
- private boolean orExpressionIsPointless(PsiExpression... lhs) {
- for (PsiExpression expression : lhs) {
- if (isAlwaysFalse(expression)) return true;
- }
- return false;
- }
-
- private boolean xorExpressionIsPointless(PsiExpression... lhs) {
- return equalityExpressionIsPointless(lhs);
- }
-
- private boolean notExpressionIsPointless(PsiExpression arg) {
- if (arg instanceof PsiPrefixExpression) {
- final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)arg;
- if (JavaTokenType.EXCL.equals(prefixExpression.getOperationTokenType())) {
- return true;
+ else if (expression instanceof PsiPolyadicExpression) {
+ final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression;
+ final IElementType sign = polyadicExpression.getOperationTokenType();
+ if (!booleanTokens.contains(sign)) {
+ return false;
}
+ final PsiExpression[] operands = polyadicExpression.getOperands();
+ boolean containsConstant = false;
+ for (PsiExpression operand : operands) {
+ if (operand == null) {
+ return false;
+ }
+ final PsiType type = operand.getType();
+ if (type == null || !type.equals(PsiType.BOOLEAN) && !type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) {
+ return false;
+ }
+ containsConstant |= (evaluate(operand) != null);
+ }
+ if (!containsConstant) {
+ return false;
+ }
+ return true;
}
- return equalityExpressionIsPointless(arg);
+ return false;
}
}
@Nullable
private Boolean evaluate(@Nullable PsiExpression expression) {
- if (m_ignoreExpressionsContainingConstants && !(expression instanceof PsiLiteralExpression)) {
+ if (expression == null || m_ignoreExpressionsContainingConstants && containsReference(expression)) {
return null;
}
-
- if (expression == null) {
- return null;
+ if (expression instanceof PsiParenthesizedExpression) {
+ final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)expression;
+ return evaluate(parenthesizedExpression.getExpression());
+ }
+ else if (expression instanceof PsiPolyadicExpression) {
+ final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression;
+ final IElementType tokenType = polyadicExpression.getOperationTokenType();
+ if (tokenType.equals(JavaTokenType.OROR)) {
+ final PsiExpression[] operands = polyadicExpression.getOperands();
+ for (PsiExpression operand : operands) {
+ if (evaluate(operand) == Boolean.TRUE) {
+ return Boolean.TRUE;
+ }
+ }
+ }
+ else if (tokenType.equals(JavaTokenType.ANDAND)) {
+ final PsiExpression[] operands = polyadicExpression.getOperands();
+ for (PsiExpression operand : operands) {
+ if (evaluate(operand) == Boolean.FALSE) {
+ return Boolean.FALSE;
+ }
+ }
+ }
+ }
+ else if (expression instanceof PsiPrefixExpression) {
+ final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression;
+ final IElementType tokenType = prefixExpression.getOperationTokenType();
+ if (JavaTokenType.EXCL.equals(tokenType)) {
+ final PsiExpression operand = prefixExpression.getOperand();
+ final Boolean b = evaluate(operand);
+ if (b == Boolean.FALSE) {
+ return Boolean.TRUE;
+ } else if (b == Boolean.TRUE) {
+ return Boolean.FALSE;
+ }
+ }
}
final Boolean value = (Boolean)ConstantExpressionUtil.computeCastTo(expression, PsiType.BOOLEAN);
return value != null ? value.booleanValue() : null;
}
- private boolean isAlwaysTrue(@Nullable PsiExpression expression) {
- return evaluate(expression) == Boolean.TRUE;
+ private static boolean containsReference(@Nullable PsiExpression expression) {
+ if (expression == null) {
+ return false;
+ }
+ final ReferenceVisitor visitor = new ReferenceVisitor();
+ expression.accept(visitor);
+ return visitor.containsReference();
}
- private boolean isAlwaysFalse(@Nullable PsiExpression expression) {
- return evaluate(expression) == Boolean.FALSE;
+ private static class ReferenceVisitor extends JavaRecursiveElementVisitor {
+
+ private boolean referenceFound = false;
+
+ @Override
+ public void visitElement(PsiElement element) {
+ if (referenceFound) {
+ return;
+ }
+ super.visitElement(element);
+ }
+
+ @Override
+ public void visitReferenceExpression(PsiReferenceExpression expression) {
+ final PsiElement target = expression.resolve();
+ if (target instanceof PsiField && ExpressionUtils.isConstant((PsiField)target)) {
+ referenceFound = true;
+ }
+ else {
+ super.visitReferenceExpression(expression);
+ }
+ }
+
+ public boolean containsReference() {
+ return referenceFound;
+ }
}
}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/PointlessBooleanExpression.html b/plugins/InspectionGadgets/src/inspectionDescriptions/PointlessBooleanExpression.html
index 0443701fa5da..ff6834573950 100644
--- a/plugins/InspectionGadgets/src/inspectionDescriptions/PointlessBooleanExpression.html
+++ b/plugins/InspectionGadgets/src/inspectionDescriptions/PointlessBooleanExpression.html
@@ -1,13 +1,15 @@
This inspection reports pointless or pointlessly
-complicated boolean expressions. Such expressions include anding with true,
-oring with false,
+complicated boolean expressions. Such expressions include anding with true,
+oring with false,
equality comparison with a boolean literal, or negation of a boolean literal. Such expressions may be the result of automated refactorings
not completely followed through to completion, and in any case are unlikely to be what the developer
intended to do.
+Use the checkbox below to ignore named constants when determining if an expression is pointless.
+
Powered by InspectionGadgets
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/Negation.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/Negation.after.java
index 84bb80545cd9..372292c4ecb1 100644
--- a/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/Negation.after.java
+++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/Negation.after.java
@@ -1,5 +1,8 @@
class C {
void m() {
final boolean isCxf = true;
+ if (false) {
+ //comment
+ }
}
}
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/PointlessBooleanExpression.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/PointlessBooleanExpression.java
index 8693d1014f07..d39daa783203 100644
--- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/PointlessBooleanExpression.java
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/PointlessBooleanExpression.java
@@ -18,4 +18,14 @@ public class Bug {
}
}
+}
+class PointlessBooleanExpression {
+ void foo(boolean a, boolean b) {
+ boolean c = !(b && false);
+ boolean d = a ^ b ^ true;
+ boolean x = a ^ !true ^ b;
+
+ boolean y = false || c;
+ boolean z = b != true;
+ }
}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/expected.xml
index 11faee0cb5b2..f246db4d2716 100644
--- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/expected.xml
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const/expected.xml
@@ -6,4 +6,40 @@
Pointless boolean expression
'!true' can be simplified 'false'
+
+
+ PointlessBooleanExpression.java
+ 24
+ Pointless boolean expression
+ <code>!(b && false)</code> can be simplified to 'true' #loc
+
+
+
+ PointlessBooleanExpression.java
+ 25
+ Pointless boolean expression
+ <code>a ^ b ^ true</code> can be simplified to '!(a^b)' #loc
+
+
+
+ PointlessBooleanExpression.java
+ 26
+ Pointless boolean expression
+ <code>a ^ !true ^ b</code> can be simplified to 'a^b' #loc
+
+
+
+ PointlessBooleanExpression.java
+ 28
+ Pointless boolean expression
+ <code>false || c</code> can be simplified to 'c' #loc
+
+
+
+ PointlessBooleanExpression.java
+ 29
+ Pointless boolean expression
+ <code>b != true</code> can be simplified to '!b' #loc
+
+
diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspectionTest.java
index b5ad880d16e7..c55dd566d817 100644
--- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspectionTest.java
+++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspectionTest.java
@@ -22,7 +22,7 @@ import com.siyeh.ig.IGInspectionTestCase;
*/
public class PointlessBooleanExpressionInspectionTest extends IGInspectionTestCase {
- public void testIgnoreExpressionsContainingConstants() throws Exception {
+ public void test() throws Exception {
final PointlessBooleanExpressionInspection inspection = new PointlessBooleanExpressionInspection();
inspection.m_ignoreExpressionsContainingConstants = true;
doTest("com/siyeh/igtest/controlflow/pointless_boolean_expression_ignore_cont_const", inspection);