From bd8362a64785c5e7e666a40daf4c0b17a9d9f70f Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 5 Dec 2017 09:47:57 +0700 Subject: [PATCH] DFA: track primitive getters (including isXyz) Fixes mostly IDEA-146061 Invalid 'may produce NullPointerException' warning: list.isEmpty result doesn't change when invoked twice in a row --- .../codeInspection/dataFlow/DfaFactType.java | 2 +- .../dataFlow/StandardInstructionVisitor.java | 17 +---- .../dataFlow/rangeSet/LongRangeSet.java | 24 +++++-- .../dataFlow/value/DfaExpressionFactory.java | 2 +- .../dataFlow/fixture/PrimitiveGetters.java | 69 +++++++++++++++++++ .../DataFlowInspection8Test.java | 2 + 6 files changed, 94 insertions(+), 22 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java index fe0e2db262c0..0fb847de5ebf 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java @@ -141,7 +141,7 @@ public abstract class DfaFactType extends Key { } PsiModifierListOwner psiVariable = var.getPsiVariable(); LongRangeSet fromType = LongRangeSet.fromType(var.getVariableType()); - return fromType == null ? null : LongRangeSet.fromAnnotation(psiVariable).intersect(fromType); + return fromType == null ? null : LongRangeSet.fromPsiElement(psiVariable).intersect(fromType); } @Nullable diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index b9b47ec5e85a..cc9238c2d5ac 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -27,8 +27,6 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; -import com.siyeh.ig.callMatcher.CallMapper; -import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.MethodUtils; import gnu.trove.THashSet; import one.util.streamex.StreamEx; @@ -44,15 +42,6 @@ import java.util.stream.Stream; public class StandardInstructionVisitor extends InstructionVisitor { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.StandardInstructionVisitor"); - private static final CallMapper KNOWN_METHOD_RANGES = new CallMapper() - .register(CallMatcher.instanceCall("java.time.LocalDateTime", "getHour"), LongRangeSet.range(0, 23)) - .register(CallMatcher.instanceCall("java.time.LocalDateTime", "getMinute", "getSecond"), LongRangeSet.range(0, 59)) - .register(CallMatcher.staticCall(CommonClassNames.JAVA_LANG_LONG, "numberOfLeadingZeros", "numberOfTrailingZeros", "bitCount"), - LongRangeSet.range(0, Long.SIZE)) - .register(CallMatcher.staticCall(CommonClassNames.JAVA_LANG_INTEGER, "numberOfLeadingZeros", "numberOfTrailingZeros", "bitCount"), - LongRangeSet.range(0, Integer.SIZE)) - .register(CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_ENUM, "ordinal").parameterCount(0), LongRangeSet.indexRange()); - private final Set myReachable = new THashSet<>(); private final Set myCanBeNullInInstanceof = new THashSet<>(); private final Set myUsefulInstanceofs = new THashSet<>(); @@ -546,11 +535,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (range != null) { PsiCall call = instruction.getCallExpression(); if (call instanceof PsiMethodCallExpression) { - LongRangeSet inferredRange = KNOWN_METHOD_RANGES.mapFirst((PsiMethodCallExpression)call); - if (inferredRange == null) { - inferredRange = LongRangeSet.fromAnnotation(call.resolveMethod()); - } - range = range.intersect(inferredRange); + range = range.intersect(LongRangeSet.fromPsiElement(call.resolveMethod())); } return factory.getFactValue(DfaFactType.RANGE, range); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java index 0f9fea57dad3..57a08651c633 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java @@ -4,10 +4,10 @@ package com.intellij.codeInspection.dataFlow.rangeSet; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInspection.dataFlow.DfaFactType; import com.intellij.codeInspection.dataFlow.value.*; -import com.intellij.psi.PsiModifierListOwner; -import com.intellij.psi.PsiPrimitiveType; -import com.intellij.psi.PsiType; +import com.intellij.psi.*; import com.intellij.util.ThreeState; +import com.siyeh.ig.callMatcher.CallMapper; +import com.siyeh.ig.callMatcher.CallMatcher; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -24,6 +24,16 @@ import static com.intellij.codeInsight.AnnotationUtil.CHECK_TYPE; * @author Tagir Valeev */ public abstract class LongRangeSet { + // TODO: create an external annotation and use it + private static final CallMapper KNOWN_METHOD_RANGES = new CallMapper() + .register(CallMatcher.instanceCall("java.time.LocalDateTime", "getHour"), range(0, 23)) + .register(CallMatcher.instanceCall("java.time.LocalDateTime", "getMinute", "getSecond"), range(0, 59)) + .register(CallMatcher.staticCall(CommonClassNames.JAVA_LANG_LONG, "numberOfLeadingZeros", "numberOfTrailingZeros", "bitCount"), + range(0, Long.SIZE)) + .register(CallMatcher.staticCall(CommonClassNames.JAVA_LANG_INTEGER, "numberOfLeadingZeros", "numberOfTrailingZeros", "bitCount"), + range(0, Integer.SIZE)) + .register(CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_ENUM, "ordinal").parameterCount(0), indexRange()); + LongRangeSet() {} /** @@ -394,8 +404,14 @@ public abstract class LongRangeSet { } @NotNull - public static LongRangeSet fromAnnotation(PsiModifierListOwner owner) { + public static LongRangeSet fromPsiElement(PsiModifierListOwner owner) { if (owner == null) return all(); + if (owner instanceof PsiMethod) { + LongRangeSet rangeSet = KNOWN_METHOD_RANGES.mapFirst((PsiMethod)owner); + if (rangeSet != null) { + return rangeSet; + } + } if (AnnotationUtil.isAnnotated(owner, "javax.annotation.Nonnegative", CHECK_TYPE)) { return range(0, Long.MAX_VALUE); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java index 870f3310fcfd..ddc34727916c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java @@ -191,7 +191,7 @@ public class DfaExpressionFactory { } if (target instanceof PsiMethod) { PsiMethod method = (PsiMethod)target; - if (PropertyUtilBase.isSimplePropertyGetter(method) && !(method.getReturnType() instanceof PsiPrimitiveType)) { + if (PropertyUtilBase.isSimplePropertyGetter(method)) { String qName = PsiUtil.getMemberQualifiedName(method); if (qName == null || !FALSE_GETTERS.value(qName)) { return method; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java b/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java new file mode 100644 index 000000000000..652905a9dde9 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java @@ -0,0 +1,69 @@ +import org.jetbrains.annotations.*; +import java.util.*; + +public class PrimitiveGetters { + interface Xyz { + boolean isFoo(); + } + + boolean test(Object[] locals, Object[] remotes) { + for (int j = 0; j < locals.length; j++) { + Object local = locals[j]; + if (local instanceof Xyz && remotes[j] instanceof Xyz) { + Xyz localXyz = (Xyz)local; + Xyz remoteXyz = (Xyz)local; + if (localXyz.isFoo() != remoteXyz.isFoo()) { + return false; + } + } + else { + return false; + } + } + return true; + } +} + +// IDEA-146061 +class SuggestionListFail { + class Suggestion { + + } + + final protected @NotNull ArrayList suggestions = new ArrayList(); + final protected @NotNull HashSet suggestionSet = new HashSet(); + + public SuggestionListFail() { + } + + @Contract(pure = true) + public boolean isEmpty() { + return suggestions.isEmpty(); + } + + @NotNull + public SuggestionListFail wrap(@Nullable SuggestionListFail prefixes, @Nullable SuggestionListFail suffixes) { + SuggestionListFail wrappedList = new SuggestionListFail(); + + if ((prefixes == null || prefixes.isEmpty()) && (suffixes == null || suffixes.isEmpty())) { + } else if (prefixes == null || prefixes.isEmpty()) { + for (Suggestion suffix : suffixes.suggestions) { + for (Suggestion suggestion : suggestions) { + } + } + } else if (suffixes == null || suffixes.isEmpty()) { + for (Suggestion prefix : prefixes.suggestions) { + for (Suggestion suggestion : suggestions) { + } + } + } else { + for (Suggestion prefix : prefixes.suggestions) { + for (Suggestion suffix : suffixes.suggestions) { + for (Suggestion suggestion : suggestions) { + } + } + } + } + return wrappedList; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index 7e2f87b0d4ae..28390c1ae223 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -216,4 +216,6 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testMutabilityJdk() { doTest(); } + + public void testPrimitiveGetters() { doTest(); } } \ No newline at end of file