IDEA-122414 Boolean expression simplification changes behavior of the code

This commit is contained in:
peter
2014-03-28 19:30:28 +01:00
parent e24e934c7f
commit 6b91f55eb7
9 changed files with 60 additions and 23 deletions
@@ -173,21 +173,21 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
return super.shouldCheckTimeLimit();
}
};
analyzeDfaWithNestedClosures(scope, holder, dfaRunner, Arrays.asList(dfaRunner.createMemoryState()));
analyzeDfaWithNestedClosures(scope, holder, dfaRunner, Arrays.asList(dfaRunner.createMemoryState()), onTheFly);
}
private void analyzeDfaWithNestedClosures(PsiElement scope,
ProblemsHolder holder,
StandardDataFlowRunner dfaRunner,
Collection<DfaMemoryState> initialStates) {
Collection<DfaMemoryState> initialStates, final boolean onTheFly) {
final DataFlowInstructionVisitor visitor = new DataFlowInstructionVisitor(dfaRunner);
final RunnerResult rc = dfaRunner.analyzeMethod(scope, visitor, IGNORE_ASSERT_STATEMENTS, initialStates);
if (rc == RunnerResult.OK) {
createDescription(dfaRunner, holder, visitor);
createDescription(dfaRunner, holder, visitor, onTheFly);
MultiMap<PsiElement,DfaMemoryState> nestedClosures = dfaRunner.getNestedClosures();
for (PsiElement closure : nestedClosures.keySet()) {
analyzeDfaWithNestedClosures(closure, holder, dfaRunner, nestedClosures.get(closure));
analyzeDfaWithNestedClosures(closure, holder, dfaRunner, nestedClosures.get(closure), onTheFly);
}
}
else if (rc == RunnerResult.TOO_COMPLEX) {
@@ -234,7 +234,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
protected void addSurroundWithIfFix(PsiExpression qualifier, List<LocalQuickFix> fixes, boolean onTheFly) {
}
private void createDescription(StandardDataFlowRunner runner, ProblemsHolder holder, DataFlowInstructionVisitor visitor) {
private void createDescription(StandardDataFlowRunner runner, ProblemsHolder holder, DataFlowInstructionVisitor visitor, final boolean onTheFly) {
Pair<Set<Instruction>, Set<Instruction>> constConditions = runner.getConstConditionalExpressions();
Set<Instruction> trueSet = constConditions.getFirst();
Set<Instruction> falseSet = constConditions.getSecond();
@@ -265,7 +265,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
reportCastMayFail(holder, (TypeCastInstruction)instruction);
}
else if (instruction instanceof BranchingInstruction) {
handleBranchingInstruction(holder, visitor, trueSet, falseSet, reportedAnchors, (BranchingInstruction)instruction);
handleBranchingInstruction(holder, visitor, trueSet, falseSet, reportedAnchors, (BranchingInstruction)instruction, onTheFly);
}
}
@@ -410,7 +410,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
private void handleBranchingInstruction(ProblemsHolder holder,
StandardInstructionVisitor visitor,
Set<Instruction> trueSet,
Set<Instruction> falseSet, HashSet<PsiElement> reportedAnchors, BranchingInstruction instruction) {
Set<Instruction> falseSet, HashSet<PsiElement> reportedAnchors, BranchingInstruction instruction, final boolean onTheFly) {
PsiElement psiAnchor = instruction.getPsiAnchor();
boolean underBinary = isAtRHSOfBooleanAnd(psiAnchor);
if (instruction instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction)) {
@@ -434,11 +434,12 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
else if (psiAnchor != null && !reportedAnchors.contains(psiAnchor) && !isCompileConstantInIfCondition(psiAnchor)) {
boolean evaluatesToTrue = trueSet.contains(instruction);
if (onTheLeftSideOfConditionalAssignment(psiAnchor)) {
final PsiElement parent = psiAnchor.getParent();
if (parent instanceof PsiAssignmentExpression && ((PsiAssignmentExpression)parent).getLExpression() == psiAnchor) {
holder.registerProblem(
psiAnchor,
InspectionsBundle.message("dataflow.message.pointless.assignment.expression", Boolean.toString(evaluatesToTrue)),
createSimplifyToAssignmentFix()
createConditionalAssignmentFixes(evaluatesToTrue, (PsiAssignmentExpression)parent, onTheFly)
);
}
else if (!skipReportingConstantCondition(visitor, psiAnchor, evaluatesToTrue)) {
@@ -452,6 +453,10 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
}
protected LocalQuickFix[] createConditionalAssignmentFixes(boolean evaluatesToTrue, PsiAssignmentExpression parent, final boolean onTheFly) {
return LocalQuickFix.EMPTY_ARRAY;
}
private boolean skipReportingConstantCondition(StandardInstructionVisitor visitor, PsiElement psiAnchor, boolean evaluatesToTrue) {
return DONT_REPORT_TRUE_ASSERT_STATEMENTS && isAssertionEffectively(psiAnchor, evaluatesToTrue) ||
visitor.silenceConstantCondition(psiAnchor);
@@ -581,15 +586,6 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
return false;
}
private static boolean onTheLeftSideOfConditionalAssignment(final PsiElement psiAnchor) {
final PsiElement parent = psiAnchor.getParent();
if (parent instanceof PsiAssignmentExpression) {
final PsiAssignmentExpression expression = (PsiAssignmentExpression)parent;
if (expression.getLExpression() == psiAnchor) return true;
}
return false;
}
@Nullable
private static LocalQuickFix createSimplifyBooleanExpressionFix(PsiElement element, final boolean value) {
SimplifyBooleanExpressionFix fix = createIntention(element, value);
@@ -626,7 +622,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
@NotNull
private static LocalQuickFix createSimplifyToAssignmentFix() {
protected static LocalQuickFix createSimplifyToAssignmentFix() {
return new LocalQuickFix() {
@NotNull
@Override
@@ -19,6 +19,7 @@ import com.intellij.codeInsight.daemon.GroupNames;
import com.intellij.codeInspection.*;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.DefUseUtil;
import com.intellij.util.containers.ContainerUtil;
import gnu.trove.THashSet;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
@@ -99,10 +100,12 @@ public class DefUseInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
else if (context instanceof PsiAssignmentExpression) {
final PsiAssignmentExpression assignment = (PsiAssignmentExpression)context;
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(isOnTheFly ? createRemoveAssignmentFix() : null);
holder.registerProblem(assignment.getLExpression(),
InspectionsBundle.message("inspection.unused.assignment.problem.descriptor3",
assignment.getRExpression().getText(), "<code>#ref</code>" + " #loc"),
ProblemHighlightType.LIKE_UNUSED_SYMBOL, createRemoveAssignmentFix());
ProblemHighlightType.LIKE_UNUSED_SYMBOL, fixes.toArray(new LocalQuickFix[fixes.size()])
);
}
else {
if (context instanceof PsiPrefixExpression && REPORT_PREFIX_EXPRESSIONS ||
@@ -19,11 +19,15 @@ import com.intellij.codeInsight.NullableNotNullDialog;
import com.intellij.codeInspection.InspectionsBundle;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.SurroundWithIfFix;
import com.intellij.codeInspection.defUse.DefUseInspection;
import com.intellij.ide.DataManager;
import com.intellij.openapi.actionSystem.CommonDataKeys;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.project.ProjectManager;
import com.intellij.psi.JavaTokenType;
import com.intellij.psi.PsiAssignmentExpression;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.tree.IElementType;
import javax.swing.*;
import javax.swing.event.ChangeEvent;
@@ -40,6 +44,17 @@ public class DataFlowInspection extends DataFlowInspectionBase {
fixes.add(new SurroundWithIfFix(qualifier));
}
}
@Override
protected LocalQuickFix[] createConditionalAssignmentFixes(boolean evaluatesToTrue, PsiAssignmentExpression assignment, final boolean onTheFly) {
IElementType op = assignment.getOperationTokenType();
boolean toRemove = op == JavaTokenType.ANDEQ && !evaluatesToTrue || op == JavaTokenType.OREQ && evaluatesToTrue;
if (toRemove && !onTheFly) {
return LocalQuickFix.EMPTY_ARRAY;
}
return new LocalQuickFix[]{toRemove ? new DefUseInspection.RemoveAssignmentFix() : createSimplifyToAssignmentFix()};
}
@Override
public JComponent createOptionsPanel() {
return new OptionsPanel();
@@ -55,7 +55,7 @@ public class DefUseInspection extends DefUseInspectionBase {
return new RemoveAssignmentFix();
}
private static class RemoveAssignmentFix extends RemoveInitializerFix {
public static class RemoveAssignmentFix extends RemoveInitializerFix {
@NotNull
@Override
public String getName() {
@@ -33,7 +33,7 @@
<problem>
<file>Test.java</file>
<line>51</line>
<description>Condition &lt;code&gt;o&lt;/code&gt; at the left side of assignment expression is always &lt;code&gt;false&lt;/code&gt;. Can be simplified to normal assignment.</description>
<description>Condition &lt;code&gt;o&lt;/code&gt; at the left side of assignment expression is always &lt;code&gt;false&lt;/code&gt;. Can be simplified.</description>
</problem>
</problems>
@@ -0,0 +1,9 @@
class Contracts {
private boolean method(boolean a) {
boolean b = true;
<warning descr="Condition 'b' at the left side of assignment expression is always 'true'. Can be simplified"><caret>b</warning> |= a;
return b;
}
}
@@ -0,0 +1,8 @@
class Contracts {
private boolean method(boolean a) {
boolean b = true;
return b;
}
}
@@ -323,6 +323,12 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase {
doTest();
}
public void testTrueOrEqualsSomething() {
doTest();
myFixture.launchAction(myFixture.findSingleIntention("Remove redundant assignment"));
myFixture.checkResultByFile(getTestName(false) + "_after.java");
}
public void _testNullCheckBeforeInstanceof() { doTest(); } // http://youtrack.jetbrains.com/issue/IDEA-113220
}