diff --git a/source/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java b/source/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java index 2648d919c918..dcddd44c2502 100644 --- a/source/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java +++ b/source/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java @@ -7,6 +7,7 @@ import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.Comparing; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.util.IncorrectOperationException; @@ -15,11 +16,13 @@ public class SimplifyBooleanExpressionFix implements IntentionAction { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpression"); private final PsiExpression mySubExpression; - private final boolean mySubExpressionValue; + private final Boolean mySubExpressionValue; private PsiExpression trueExpression; private PsiExpression falseExpression; - public SimplifyBooleanExpressionFix(PsiExpression subExpression, boolean subExpressionValue) { + // subExpressionValue == Boolean.TRUE or Boolean.FALSE if subExpression evaluates to boolean constant and needs to be replaced + // otherwise subExpressionValue= null and we starting to simplify expression without any further knowledge + public SimplifyBooleanExpressionFix(PsiExpression subExpression, Boolean subExpressionValue) { mySubExpression = subExpression; mySubExpressionValue = subExpressionValue; } @@ -38,8 +41,15 @@ public class SimplifyBooleanExpressionFix implements IntentionAction { } public void invoke(Project project, Editor editor, PsiFile file) throws IncorrectOperationException { - PsiExpression constExpression = mySubExpression.getManager().getElementFactory().createExpressionFromText(Boolean.toString(mySubExpressionValue), mySubExpression); - PsiExpression expression = (PsiExpression)mySubExpression.replace(constExpression); + PsiExpression expression; + if (mySubExpressionValue == null) { + expression = mySubExpression; + } + else { + PsiExpression constExpression = mySubExpression.getManager().getElementFactory() + .createExpressionFromText(mySubExpressionValue.toString(), mySubExpression); + expression = (PsiExpression)mySubExpression.replace(constExpression); + } while (expression.getParent() instanceof PsiExpression) { expression = (PsiExpression)expression.getParent(); } @@ -51,7 +61,7 @@ public class SimplifyBooleanExpressionFix implements IntentionAction { trueExpression = expression.getManager().getElementFactory().createExpressionFromText(Boolean.toString(true), null); falseExpression = expression.getManager().getElementFactory().createExpressionFromText(Boolean.toString(false), null); final PsiExpression[] copy = new PsiExpression[]{(PsiExpression)expression.copy()}; - copy[0].accept(new PsiRecursiveElementVisitor() { + copy[0].accept(new PsiElementVisitor() { public void visitElement(PsiElement element) { final PsiElement[] children = element.getChildren(); for (int i = 0; i < children.length; i++) { @@ -101,6 +111,14 @@ public class SimplifyBooleanExpressionFix implements IntentionAction { else if (JavaTokenType.OROR == tokenType || JavaTokenType.OR == tokenType) { resultExpression = lConstBoolean.booleanValue() ? trueExpression : rOperand; } + else if (JavaTokenType.EQEQ == tokenType) { + simplifyEquation(lConstBoolean, rOperand); + } + else if (JavaTokenType.NE == tokenType) { + resultExpression = createNegatedExpression(rOperand); + visitPrefixExpression((PsiPrefixExpression)resultExpression); + simplifyEquation(lConstBoolean, resultExpression); + } } else if (rConstBoolean != null) { if (JavaTokenType.ANDAND == tokenType || JavaTokenType.AND == tokenType) { @@ -109,9 +127,37 @@ public class SimplifyBooleanExpressionFix implements IntentionAction { else if (JavaTokenType.OROR == tokenType || JavaTokenType.OR == tokenType) { resultExpression = rConstBoolean.booleanValue() ? trueExpression : lOperand; } + else if (JavaTokenType.EQEQ == tokenType) { + simplifyEquation(rConstBoolean, lOperand); + } + else if (JavaTokenType.NE == tokenType) { + simplifyEquation(rConstBoolean, createNegatedExpression(lOperand)); + } } } + private void simplifyEquation(final Boolean constBoolean, final PsiExpression otherOperand) { + if (constBoolean.booleanValue()) { + resultExpression = otherOperand; + } + else { + final PsiPrefixExpression negated = createNegatedExpression(otherOperand); + resultExpression = negated; + visitPrefixExpression(negated); + } + } + + private PsiPrefixExpression createNegatedExpression(final PsiExpression otherOperand) { + try { + return (PsiPrefixExpression)otherOperand.getManager().getElementFactory() + .createExpressionFromText("!" + otherOperand.getText(), otherOperand); + } + catch (IncorrectOperationException e) { + LOG.error(e); + } + return null; + } + public void visitPrefixExpression(PsiPrefixExpression expression) { final PsiExpression operand = expression.getOperand(); final Boolean constBoolean = getConstBoolean(operand); @@ -138,6 +184,19 @@ public class SimplifyBooleanExpressionFix implements IntentionAction { return "true".equals(text) ? Boolean.TRUE : "false".equals(text) ? Boolean.FALSE : null; } + public static PsiExpression canBeSimplified(PsiExpression expression) { + try { + final SimplifyBooleanExpressionFix fix = new SimplifyBooleanExpressionFix(expression, null); + final PsiExpression newExpression = fix.simplifyExpression(expression); + if (Comparing.strEqual(newExpression.getText(), expression.getText())) return null; + return newExpression; + } + catch (IncorrectOperationException e) { + LOG.error(e); + return null; + } + } + public boolean startInWriteAction() { return true; } diff --git a/source/com/intellij/codeInsight/intention/impl/SimplifyBooleanExpressionAction.java b/source/com/intellij/codeInsight/intention/impl/SimplifyBooleanExpressionAction.java index 36022746902f..61e78cc89b7b 100644 --- a/source/com/intellij/codeInsight/intention/impl/SimplifyBooleanExpressionAction.java +++ b/source/com/intellij/codeInsight/intention/impl/SimplifyBooleanExpressionAction.java @@ -3,15 +3,14 @@ */ package com.intellij.codeInsight.intention.impl; -import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix; +import com.intellij.codeInsight.intention.IntentionAction; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Comparing; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiFile; import com.intellij.psi.PsiExpression; +import com.intellij.psi.PsiFile; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; @@ -23,7 +22,7 @@ public class SimplifyBooleanExpressionAction implements IntentionAction{ } public String getFamilyName() { - return new SimplifyBooleanExpressionFix(null,false).getFamilyName(); + return new SimplifyBooleanExpressionFix(null,null).getFamilyName(); } public boolean isAvailable(Project project, Editor editor, PsiFile file) { @@ -43,17 +42,11 @@ public class SimplifyBooleanExpressionAction implements IntentionAction{ if (element == null) return null; PsiExpression expression = PsiTreeUtil.getParentOfType(element, PsiExpression.class); if (expression == null) return null; - final Boolean constBoolean = SimplifyBooleanExpressionFix.getConstBoolean(expression); - if (constBoolean == null) return null; - PsiExpression topexpression = expression; - while (topexpression.getParent() instanceof PsiExpression) { - topexpression = (PsiExpression)topexpression.getParent(); + while (expression.getParent() instanceof PsiExpression) { + expression = (PsiExpression)expression.getParent(); } - if (topexpression == expression) return null; - final SimplifyBooleanExpressionFix fix = new SimplifyBooleanExpressionFix(topexpression, constBoolean.booleanValue()); - final PsiExpression newExpression = fix.simplifyExpression(topexpression); - if (Comparing.strEqual(newExpression.getText(), topexpression.getText())) return null; - return replace ? (PsiExpression)topexpression.replace(newExpression) : newExpression; + final PsiExpression newExpression = SimplifyBooleanExpressionFix.canBeSimplified(expression); + return replace ? (PsiExpression)expression.replace(newExpression) : newExpression; } public void invoke(Project project, Editor editor, PsiFile file) throws IncorrectOperationException { diff --git a/source/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/source/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 978e86a2f136..705da0669dfa 100644 --- a/source/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/source/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -740,7 +740,8 @@ class ControlFlowAnalyzer extends PsiElementVisitor { public void visitExpression(PsiExpression expression) { startElement(expression); - pushUnknown(); + DfaValue dfaValue = DfaValueFactory.create(expression); + addInstruction(new PushInstruction(dfaValue == null ? DfaUnknownValue.getInstance() : dfaValue)); finishElement(expression); } @@ -782,28 +783,34 @@ class ControlFlowAnalyzer extends PsiElementVisitor { startElement(expression); try { - String op = expression.getOperationSign().getText(); - PsiExpression lExpr = expression.getLOperand(); - PsiExpression rExpr = expression.getROperand(); - - if (lExpr == null || rExpr == null) { - pushUnknown(); - return; - } - - if ("&&".equals(op)) { - generateAndExpression(lExpr, rExpr); - } - else if ("||".equals(op)) { - generateOrExpression(lExpr, rExpr); - } - else if ("^".equals(op) && expression.getType() == PsiType.BOOLEAN) { - generateXorExpression(expression, lExpr, rExpr); + DfaValue dfaValue = DfaValueFactory.create(expression); + if (dfaValue != null) { + addInstruction(new PushInstruction(dfaValue)); } else { - lExpr.accept(this); - rExpr.accept(this); - addInstruction(new BinopInstruction(op, expression.isPhysical() ? expression : null)); + String op = expression.getOperationSign().getText(); + PsiExpression lExpr = expression.getLOperand(); + PsiExpression rExpr = expression.getROperand(); + + if (lExpr == null || rExpr == null) { + pushUnknown(); + return; + } + + if ("&&".equals(op)) { + generateAndExpression(lExpr, rExpr); + } + else if ("||".equals(op)) { + generateOrExpression(lExpr, rExpr); + } + else if ("^".equals(op) && expression.getType() == PsiType.BOOLEAN) { + generateXorExpression(expression, lExpr, rExpr); + } + else { + lExpr.accept(this); + rExpr.accept(this); + addInstruction(new BinopInstruction(op, expression.isPhysical() ? expression : null)); + } } } finally { @@ -855,21 +862,27 @@ class ControlFlowAnalyzer extends PsiElementVisitor { public void visitConditionalExpression(PsiConditionalExpression expression) { startElement(expression); - PsiExpression condition = expression.getCondition(); - - PsiExpression thenExpression = expression.getThenExpression(); - PsiExpression elseExpression = expression.getElseExpression(); - - if (condition != null && thenExpression != null && elseExpression != null) { - condition.accept(this); - addInstruction(new ConditionalGotoInstruction(getStartOffset(elseExpression), true, condition)); - thenExpression.accept(this); - - addInstruction(new GotoInstruction(getEndOffset(expression))); - elseExpression.accept(this); + DfaValue dfaValue = DfaValueFactory.create(expression); + if (dfaValue != null) { + addInstruction(new PushInstruction(dfaValue)); } else { - pushUnknown(); + PsiExpression condition = expression.getCondition(); + + PsiExpression thenExpression = expression.getThenExpression(); + PsiExpression elseExpression = expression.getElseExpression(); + + if (condition != null && thenExpression != null && elseExpression != null) { + condition.accept(this); + addInstruction(new ConditionalGotoInstruction(getStartOffset(elseExpression), true, condition)); + thenExpression.accept(this); + + addInstruction(new GotoInstruction(getEndOffset(expression))); + elseExpression.accept(this); + } + else { + pushUnknown(); + } } finishElement(expression); @@ -970,13 +983,12 @@ class ControlFlowAnalyzer extends PsiElementVisitor { processMethodParameters(expression); addInstruction(new MethodCallInstruction(expression)); - PsiExpression expr = methodExpression; if (myCatchStack.size() > 0) { addMethodThrows((PsiMethod)methodExpression.getReference().resolve()); } - pushTypeOrUnknown(expr); + pushTypeOrUnknown(methodExpression); } finally { finishElement(expression); @@ -1057,29 +1069,35 @@ class ControlFlowAnalyzer extends PsiElementVisitor { public void visitPrefixExpression(PsiPrefixExpression expression) { startElement(expression); - PsiExpression operand = expression.getOperand(); + DfaValue dfaValue = DfaValueFactory.create(expression); + if (dfaValue != null) { + addInstruction(new PushInstruction(dfaValue)); + } + else { + PsiExpression operand = expression.getOperand(); - if (operand != null) { - operand.accept(this); - if (expression.getOperationSign().getTokenType() == JavaTokenType.EXCL) { - addInstruction(new NotInstruction()); + if (operand == null) { + pushUnknown(); } else { - addInstruction(new PopInstruction()); - pushUnknown(); + operand.accept(this); + if (expression.getOperationSign().getTokenType() == JavaTokenType.EXCL) { + addInstruction(new NotInstruction()); + } + else { + addInstruction(new PopInstruction()); + pushUnknown(); - if (operand instanceof PsiReferenceExpression) { - PsiVariable psiVariable = DfaValueFactory.resolveVariable((PsiReferenceExpression)operand); - if (psiVariable != null) { - DfaVariableValue dfaVariable = DfaVariableValue.Factory.getInstance().create(psiVariable, false); - addInstruction(new FlushVariableInstruction(dfaVariable)); + if (operand instanceof PsiReferenceExpression) { + PsiVariable psiVariable = DfaValueFactory.resolveVariable((PsiReferenceExpression)operand); + if (psiVariable != null) { + DfaVariableValue dfaVariable = DfaVariableValue.Factory.getInstance().create(psiVariable, false); + addInstruction(new FlushVariableInstruction(dfaVariable)); + } } } } } - else { - pushUnknown(); - } finishElement(expression); } diff --git a/source/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/source/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 7c9b43c1561b..dc1b255af128 100644 --- a/source/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/source/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -26,7 +26,7 @@ import java.util.*; public class DataFlowInspection extends BaseLocalInspectionTool { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.DataFlowInspection"); - public static final String DISPLAY_NAME = "Constant conditions & exceptions"; + private static final String DISPLAY_NAME = "Constant conditions & exceptions"; public static final String SHORT_NAME = "ConstantConditions"; public DataFlowInspection() { @@ -51,7 +51,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool { return allProblems == null ? null : allProblems.toArray(new ProblemDescriptor[allProblems.size()]); } - private ProblemDescriptor[] analyzeCodeBlock(final PsiCodeBlock body, InspectionManager manager) { + private static ProblemDescriptor[] analyzeCodeBlock(final PsiCodeBlock body, InspectionManager manager) { if (body == null) return null; DataFlowRunner dfaRunner = new DataFlowRunner(); if (dfaRunner.analyzeMethod(body)) { @@ -85,7 +85,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool { return null; } - private ProblemDescriptor[] createDescription(DataFlowRunner runner, InspectionManager manager) { + private static ProblemDescriptor[] createDescription(DataFlowRunner runner, InspectionManager manager) { HashSet[] constConditions = runner.getConstConditionalExpressions(); HashSet trueSet = constConditions[0]; HashSet falseSet = constConditions[1]; @@ -180,7 +180,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool { ProblemHighlightType.GENERIC_ERROR_OR_WARNING)); } else { - final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(true); + final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, true); descriptions.add(manager.createProblemDescriptor(psiAnchor, "Condition #ref #loc is always true", localQuickFix, @@ -196,7 +196,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool { } else if (psiAnchor != null) { if (!reportedAnchors.contains(psiAnchor)) { - final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(trueSet.contains(instruction)); + final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, trueSet.contains(instruction)); descriptions.add(manager.createProblemDescriptor(psiAnchor, "Condition #ref #loc is always " + (trueSet.contains(instruction) ? "true" : "false") + ".", localQuickFix, @@ -210,18 +210,25 @@ public class DataFlowInspection extends BaseLocalInspectionTool { return descriptions.toArray(new ProblemDescriptor[descriptions.size()]); } - private static LocalQuickFix createSimplifyBooleanExpressionFix(final boolean value) { + private static LocalQuickFix createSimplifyBooleanExpressionFix(PsiElement element, final boolean value) { + if (!(element instanceof PsiExpression)) return null; + final PsiExpression expression = (PsiExpression)element; + while (element.getParent() instanceof PsiExpression) { + element = element.getParent(); + } + final SimplifyBooleanExpressionFix fix = new SimplifyBooleanExpressionFix(expression, Boolean.valueOf(value)); + // simplify intention already active + if (SimplifyBooleanExpressionFix.canBeSimplified((PsiExpression)element) != null) return null; return new LocalQuickFix() { public String getName() { - return new SimplifyBooleanExpressionFix(null,false).getText(); + return fix.getText(); } public void applyFix(Project project, ProblemDescriptor descriptor) { final PsiElement psiElement = descriptor.getPsiElement(); try { - final SimplifyBooleanExpressionFix action = new SimplifyBooleanExpressionFix((PsiExpression)psiElement, value); LOG.assertTrue(psiElement.isValid()); - action.invoke(project, null, psiElement.getContainingFile()); + fix.invoke(project, null, psiElement.getContainingFile()); } catch (IncorrectOperationException e) { LOG.error(e); diff --git a/source/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java b/source/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java index da3ee6c9ca71..5a2af403ea2c 100644 --- a/source/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java +++ b/source/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java @@ -54,7 +54,7 @@ public class DfaConstValue extends DfaValue { return createFromValue(value); } - private DfaConstValue createFromValue(Object value) { + public DfaConstValue createFromValue(Object value) { if (value == Boolean.TRUE) return dfaTrue; if (value == Boolean.FALSE) return dfaFalse; diff --git a/source/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java b/source/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java index 0d57687536b1..54bd90f70735 100644 --- a/source/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java +++ b/source/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java @@ -10,6 +10,8 @@ package com.intellij.codeInspection.dataFlow.value; import com.intellij.openapi.diagnostic.Logger; import com.intellij.psi.*; +import com.intellij.psi.impl.ConstantExpressionEvaluator; +import gnu.trove.THashSet; import gnu.trove.TIntObjectHashMap; public class DfaValueFactory { @@ -70,6 +72,12 @@ public class DfaValueFactory { else if (psiExpression instanceof PsiNewExpression) { result = DfaNewValue.Factory.getInstance().create(psiExpression.getType()); } + else { + final Object value = ConstantExpressionEvaluator.computeConstantExpression(psiExpression, new THashSet(), false); + if (value != null) { + result = DfaConstValue.Factory.getInstance().createFromValue(value); + } + } return result; } diff --git a/source/com/intellij/psi/impl/ConstantExpressionEvaluator.java b/source/com/intellij/psi/impl/ConstantExpressionEvaluator.java index 9b37f3170373..53924c38825c 100644 --- a/source/com/intellij/psi/impl/ConstantExpressionEvaluator.java +++ b/source/com/intellij/psi/impl/ConstantExpressionEvaluator.java @@ -9,11 +9,11 @@ import gnu.trove.THashSet; import java.util.Set; public class ConstantExpressionEvaluator extends PsiElementVisitor { - protected Set myVisitedVars; + protected Set myVisitedVars; protected boolean myThrowExceptionOnOverflow; protected Object myValue; - protected ConstantExpressionEvaluator(Set visitedVars, boolean throwExceptionOnOverflow) { + protected ConstantExpressionEvaluator(Set visitedVars, boolean throwExceptionOnOverflow) { myVisitedVars = visitedVars; myThrowExceptionOnOverflow = throwExceptionOnOverflow; } @@ -486,8 +486,8 @@ public class ConstantExpressionEvaluator extends PsiElementVisitor { return; } - Set oldVisitedVars = myVisitedVars; - if (myVisitedVars == null) { myVisitedVars = new THashSet(); } + Set oldVisitedVars = myVisitedVars; + if (myVisitedVars == null) { myVisitedVars = new THashSet(); } myVisitedVars.add(variable); try { @@ -545,7 +545,7 @@ public class ConstantExpressionEvaluator extends PsiElementVisitor { if (result instanceof Double && ((Double) result).isInfinite()) throw new ConstantEvaluationOverflowException(expression); } - public static Object computeConstantExpression(PsiExpression expression, Set visitedVars, boolean throwExceptionOnOverflow) { + public static Object computeConstantExpression(PsiExpression expression, Set visitedVars, boolean throwExceptionOnOverflow) { ConstantExpressionEvaluator evaluator = new ConstantExpressionEvaluator(visitedVars, throwExceptionOnOverflow); return _compute(evaluator, expression); } diff --git a/testData/inspection/dataFlow/constantExpr/expected.xml b/testData/inspection/dataFlow/constantExpr/expected.xml new file mode 100644 index 000000000000..8e169325e53b --- /dev/null +++ b/testData/inspection/dataFlow/constantExpr/expected.xml @@ -0,0 +1,8 @@ + + + + Test.java + 6 + Condition 'i == j' is always true + + diff --git a/testData/inspection/dataFlow/constantExpr/src/Test.java b/testData/inspection/dataFlow/constantExpr/src/Test.java new file mode 100644 index 000000000000..4ec920579cdb --- /dev/null +++ b/testData/inspection/dataFlow/constantExpr/src/Test.java @@ -0,0 +1,9 @@ +public class Test { + private static final int CONST = 3/2 + 0*1; + public void foo() { + int i = 0; + int j = 2 + (CONST) - 6/2; + if (i == j) { + } + } +} \ No newline at end of file diff --git a/testSource/com/intellij/codeInspection/DataFlowTest.java b/testSource/com/intellij/codeInspection/DataFlowTest.java index c959bf8aaa46..3199216b5df4 100644 --- a/testSource/com/intellij/codeInspection/DataFlowTest.java +++ b/testSource/com/intellij/codeInspection/DataFlowTest.java @@ -103,15 +103,14 @@ public class DataFlowTest extends InspectionTestCase { public void testscrIDEA1() throws Exception { doTest(); } - /* public void testSCR18186() throws Exception { doTest(); } - */ -/* - public void testSCR15406() throws Exception { + //public void testSCR15406() throws Exception { + // doTest(); + //} + public void testconstantExpr() throws Exception { doTest(); } -*/ }