From 47816dcc2109e3368b5affec572be31e8f2a18d2 Mon Sep 17 00:00:00 2001 From: peter Date: Sun, 28 Oct 2012 18:11:46 +0100 Subject: [PATCH] suppress constant condition reporting for any expression involving getters (IDEA-93244) --- .../dataFlow/DataFlowInspection.java | 2 +- .../dataFlow/StandardInstructionVisitor.java | 15 ++++++++---- .../dataFlow/fixture/ThisFieldGetters.java | 23 +++++++++++++++++++ .../DataFlowInspectionFixtureTest.java | 1 + 4 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ThisFieldGetters.java diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 4d91e400d137..48ad2cc6a58a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -283,7 +283,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool { createSimplifyToAssignmentFix() ); } - else if (shouldReportConditionAlwaysTrueOrFalse(psiAnchor, evaluatesToTrue) && !visitor.silenceConstantCondition(instruction)) { + else if (shouldReportConditionAlwaysTrueOrFalse(psiAnchor, evaluatesToTrue) && !visitor.silenceConstantCondition(psiAnchor)) { final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue); String message = InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.when.reached" : diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f2ecef1f8f4d..6080e4ebf058 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -20,8 +20,10 @@ import com.intellij.codeInspection.dataFlow.instructions.*; import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.ArrayUtil; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.FactoryMap; import gnu.trove.THashSet; import org.jetbrains.annotations.NotNull; @@ -36,7 +38,7 @@ import java.util.Set; public class StandardInstructionVisitor extends InstructionVisitor { private final Set myReachable = new THashSet(); private final Set myCanBeNullInInstanceof = new THashSet(); - private final Set myNotToReportReachability = new THashSet(); + private final Set myNotToReportReachability = new THashSet(); private final Set myUsefulInstanceofs = new THashSet(); private final FactoryMap myParametersNotNull = new FactoryMap() { @Override @@ -328,7 +330,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { } if (isViaMethods(dfaLeft) || isViaMethods(dfaRight)) { - myNotToReportReachability.add(instruction); + ContainerUtil.addIfNotNull(myNotToReportReachability, instruction.getPsiAnchor()); } myCanBeNullInInstanceof.add(instruction); @@ -435,7 +437,12 @@ public class StandardInstructionVisitor extends InstructionVisitor { return myCanBeNullInInstanceof.contains(instruction); } - public boolean silenceConstantCondition(BranchingInstruction instruction) { - return instruction instanceof BinopInstruction && myNotToReportReachability.contains(instruction); + public boolean silenceConstantCondition(@Nullable PsiElement element) { + for (PsiElement skipped : myNotToReportReachability) { + if (PsiTreeUtil.isAncestor(element, skipped, false)) { + return true; + } + } + return false; } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ThisFieldGetters.java b/java/java-tests/testData/inspection/dataFlow/fixture/ThisFieldGetters.java new file mode 100644 index 000000000000..c732b18f823b --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ThisFieldGetters.java @@ -0,0 +1,23 @@ +class Test { + + private int count1; + private int count2; + + public void test() { + int oldCount1 = getCount1(); + int oldCount2 = getCount2(); + count1++; + if (oldCount1 != getCount1() || oldCount2 != getCount2()) { + System.out.println("changed"); + } + } + + private int getCount1() { + return count1; + } + + private int getCount2() { + return count2; + } + +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index e32605c17a37..e4f0751aac17 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -81,6 +81,7 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testChainedFinalFieldsDfa() throws Throwable { doTest(); } public void testFinalFieldsDifferentInstances() throws Throwable { doTest(); } + public void testThisFieldGetters() throws Throwable { doTest(); } public void testChainedFinalFieldAccessorsDfa() throws Throwable { doTest(); } public void testAssigningUnknownToNullable() throws Throwable { doTest(); }