From 5a8e6d64ccb0195a618d11b68fb4c8797cf7bed7 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 27 Jun 2018 17:42:55 +0700 Subject: [PATCH] DFA instruction visitor refactoring wave#7 Merge reporting of constant variables and constant conditions Fix DfaConstValue#createFromValue (for constants like Integer X = new Integer(1)) DfaMemoryState#getConstantValue now accepts DfaValue (and processes unboxed value as well) Report "will cause NPE", etc. messages when dereferenced value is statically known to be null (not only null literal) IDEA-194676 --- .../dataFlow/ContractReturnValue.java | 10 +- .../dataFlow/CustomMethodHandlers.java | 9 +- .../dataFlow/DataFlowInspectionBase.java | 169 ++++++++++++------ .../dataFlow/DataFlowInstructionVisitor.java | 112 +++++------- .../dataFlow/DfaMemoryState.java | 10 +- .../dataFlow/DfaMemoryStateImpl.java | 18 +- .../dataFlow/InstructionVisitor.java | 4 +- .../dataFlow/NullabilityProblemKind.java | 52 ++++-- .../dataFlow/StandardInstructionVisitor.java | 4 +- .../dataFlow/value/DfaConstValue.java | 15 +- .../dataFlow/ArrayAccessNPE/expected.xml | 2 +- .../inspection/dataFlow/SCR15162/expected.xml | 2 +- .../dataFlow/fixture/AdvancedArrayAccess.java | 2 +- .../dataFlow/fixture/BoxingBoolean.java | 8 +- ...paringToNotNullShouldNotAffectNullity.java | 2 +- .../dataFlow/fixture/ComplexInitializer.java | 2 +- .../dataFlow/fixture/ContractAnnotation.java | 2 +- ...ractPreservesUnknownMethodNullability.java | 2 +- .../FieldUsedBeforeInitialization.java | 6 +- .../fixture/ImplicitlyInitializedField.java | 8 +- .../dataFlow/fixture/LessThanRelations.java | 2 +- .../dataFlow/fixture/LongRangeBasics.java | 4 +- .../fixture/LongRangeKnownMethods.java | 4 +- .../MergedInitializerAndConstructor.java | 6 +- .../dataFlow/fixture/MutabilityJdk.java | 2 +- .../MutableNotAnnotatedFieldsTreatment.java | 2 +- .../MutableNullableFieldsTreatment.java | 2 +- .../dataFlow/fixture/OptionalInlining.java | 14 +- .../dataFlow/fixture/OptionalIsPresent.java | 16 +- .../dataFlow/fixture/ReportAlwaysNull.java | 22 +++ .../fixture/ReportConstantReferences.java | 2 +- .../dataFlow/fixture/StreamInlining.java | 10 +- .../dataFlow/fixture/StreamKnownSource.java | 8 +- .../dataFlow/fixture/ThisAsVariable.java | 2 +- .../fixture/TryWithResourcesNullability.java | 2 +- .../dataFlow/fixture/UnknownOnStack.java | 2 +- .../dataFlow/fixture/VariablesDiverge.java | 2 +- .../dataFlow/nullableField/expected.xml | 2 +- .../DataFlowInspectionTest.java | 6 + .../src/messages/InspectionsBundle.properties | 3 + 40 files changed, 318 insertions(+), 234 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ReportAlwaysNull.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractReturnValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractReturnValue.java index 18935cbc12de..346ae32be43c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractReturnValue.java @@ -507,14 +507,8 @@ public abstract class ContractReturnValue { @Override public boolean isValueCompatible(DfaMemoryState state, DfaValue value) { - if (value instanceof DfaVariableValue) { - value = state.getConstantValue((DfaVariableValue)value); - } - if (value instanceof DfaConstValue) { - Object constant = ((DfaConstValue)value).getValue(); - return Boolean.valueOf(myValue).equals(constant); - } - return true; + DfaConstValue dfaConst = state.getConstantValue(value); + return dfaConst == null || Boolean.valueOf(myValue).equals(dfaConst.getValue()); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java index d6914948701e..46d1e85a4ae2 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java @@ -19,7 +19,6 @@ import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; import com.intellij.codeInspection.dataFlow.value.DfaConstValue; import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; -import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.psi.*; import com.intellij.psi.util.CachedValueProvider; import com.intellij.psi.util.CachedValuesManager; @@ -223,11 +222,9 @@ class CustomMethodHandlers { return fact.min(); } } - if (value instanceof DfaVariableValue) { - value = memoryState.getConstantValue((DfaVariableValue)value); - } - if (value instanceof DfaConstValue) { - Object constant = ((DfaConstValue)value).getValue(); + DfaConstValue dfaConst = memoryState.getConstantValue(value); + if (dfaConst != null) { + Object constant = dfaConst.getValue(); if (constant instanceof String && ((String)constant).length() > MAX_STRING_CONSTANT_LENGTH_TO_TRACK) return null; return constant; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 8185f8588c84..f6020be127ce 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -35,13 +35,17 @@ import com.siyeh.ig.psiutils.*; import one.util.streamex.IntStreamEx; import one.util.streamex.StreamEx; import org.jdom.Element; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; +import java.text.MessageFormat; import java.util.*; +import static com.intellij.util.ObjectUtils.tryCast; + @SuppressWarnings("ConditionalExpressionWithIdenticalBranches") public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool { static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.DataFlowInspection"); @@ -272,11 +276,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool reportOptionalOfNullableImprovements(holder, reportedAnchors, visitor.getOfNullableCalls()); - visitor.getBooleanExpressions().forEach((expression, state) -> { - if (state != ThreeState.UNSURE) { - reportConstantBoolean(holder, expression, reportedAnchors, state.toBoolean()); - } - }); + reportConstants(holder, visitor, reportedAnchors); reportMethodReferenceProblems(holder, visitor); @@ -284,10 +284,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool reportArrayStoreProblems(holder, visitor); - if (REPORT_CONSTANT_REFERENCE_VALUES) { - reportConstantReferenceValues(holder, visitor, reportedAnchors); - } - if (REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL && visitor.isAlwaysReturnsNotNull(runner.getInstructions())) { reportAlwaysReturnsNotNull(holder, scope); } @@ -300,6 +296,76 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool reportDuplicateAssignments(holder, reportedAnchors, visitor); } + private void reportConstants(ProblemsHolder holder, DataFlowInstructionVisitor visitor, HashSet reportedAnchors) { + visitor.getConstantExpressions().forEach((expression, result) -> { + if (result == DataFlowInstructionVisitor.ConstantResult.UNKNOWN) return; + if (isCondition(expression)) { + if (result.value() instanceof Boolean) { + reportConstantBoolean(holder, expression, reportedAnchors, (Boolean)result.value()); + } + } + else { + reportConstantReferenceValue(holder, reportedAnchors, expression, result); + } + }); + } + + private static boolean isCondition(@NotNull PsiExpression expression) { + PsiType type = expression.getType(); + if (type == null || !PsiType.BOOLEAN.isAssignableFrom(type)) return false; + if (!(expression instanceof PsiMethodCallExpression) && !(expression instanceof PsiReferenceExpression)) return true; + PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); + if (parent instanceof PsiStatement) return !(parent instanceof PsiReturnStatement); + if (parent instanceof PsiPolyadicExpression) { + IElementType tokenType = ((PsiPolyadicExpression)parent).getOperationTokenType(); + return tokenType.equals(JavaTokenType.ANDAND) || tokenType.equals(JavaTokenType.OROR); + } + if (parent instanceof PsiConditionalExpression) { + return PsiTreeUtil.isAncestor(((PsiConditionalExpression)parent).getCondition(), expression, false); + } + return false; + } + + private void reportConstantReferenceValue(ProblemsHolder holder, Set reportedAnchors, + PsiExpression ref, DataFlowInstructionVisitor.ConstantResult constant) { + if (!REPORT_CONSTANT_REFERENCE_VALUES && ref instanceof PsiReferenceExpression) return; + if (shouldBeSuppressed(ref)) return; + if (constant == DataFlowInstructionVisitor.ConstantResult.UNKNOWN || !reportedAnchors.add(ref)) return; + List fixes = new SmartList<>(); + String presentableName = constant.toString(); + fixes.add(new ReplaceWithConstantValueFix(presentableName, presentableName)); + Object value = constant.value(); + boolean isAssertion = value instanceof Boolean && isAssertionEffectively(ref, (Boolean)value); + if (isAssertion && DONT_REPORT_TRUE_ASSERT_STATEMENTS) return; + if (value instanceof Boolean) { + ContainerUtil.addIfNotNull(fixes, createReplaceWithNullCheckFix(ref, (Boolean)value)); + } + if (holder.isOnTheFly()) { + if (ref instanceof PsiReferenceExpression) { + fixes.add(new SetInspectionOptionFix(this, "REPORT_CONSTANT_REFERENCE_VALUES", + InspectionsBundle.message("inspection.data.flow.turn.off.constant.references.quickfix"), + false)); + } + if (isAssertion) { + fixes.add(new SetInspectionOptionFix(this, "DONT_REPORT_TRUE_ASSERT_STATEMENTS", + InspectionsBundle.message("inspection.data.flow.turn.off.true.asserts.quickfix"), true)); + } + } + + String valueText; + ProblemHighlightType type; + if (ref instanceof PsiMethodCallExpression) { + type = ProblemHighlightType.GENERIC_ERROR_OR_WARNING; + valueText = "Result of"; + } + else { + type = ProblemHighlightType.WEAK_WARNING; + valueText = "Value"; + } + holder.registerProblem(ref, MessageFormat.format("{0} #ref #loc is always ''{1}''", valueText, presentableName), + type, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); + } + private void reportDuplicateAssignments(ProblemsHolder holder, HashSet reportedAnchors, DataFlowInstructionVisitor visitor) { @@ -347,6 +413,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool private void reportNullabilityProblems(ProblemsHolder holder, DataFlowInstructionVisitor visitor, HashSet reportedAnchors) { + Map expressions = visitor.getConstantExpressions(); visitor.problems().forEach(problem -> { if (NullabilityProblemKind.passingNullableArgumentToNonAnnotatedParameter.isMyProblem(problem) || NullabilityProblemKind.assigningNullableValueToNonAnnotatedField.isMyProblem(problem) || @@ -357,33 +424,36 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool if (!reportedAnchors.add(problem.getAnchor())) return; NullabilityProblemKind.innerClassNPE.ifMyProblem(problem, newExpression -> { List fixes = createNPEFixes(newExpression.getQualifier(), newExpression, holder.isOnTheFly()); - holder.registerProblem(getElementToHighlight(newExpression), problem.getMessage(), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); + holder.registerProblem(getElementToHighlight(newExpression), problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); }); NullabilityProblemKind.callMethodRefNPE.ifMyProblem(problem, methodRef -> holder.registerProblem(methodRef, InspectionsBundle.message("dataflow.message.npe.methodref.invocation"), createMethodReferenceNPEFixes(methodRef).toArray(LocalQuickFix.EMPTY_ARRAY))); - NullabilityProblemKind.callNPE.ifMyProblem(problem, call -> reportCallMayProduceNpe(holder, call)); + NullabilityProblemKind.callNPE.ifMyProblem(problem, call -> reportCallMayProduceNpe(holder, problem.getMessage(expressions), call)); NullabilityProblemKind.passingNullableToNotNullParameter.ifMyProblem(problem, expr -> reportNullableArgument(holder, expr)); NullabilityProblemKind.arrayAccessNPE.ifMyProblem(problem, expression -> { LocalQuickFix[] fix = createNPEFixes(expression.getArrayExpression(), expression, holder.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY); - holder.registerProblem(expression, problem.getMessage(), fix); + holder.registerProblem(expression, problem.getMessage(expressions), fix); }); NullabilityProblemKind.fieldAccessNPE.ifMyProblem(problem, element -> { PsiElement parent = element.getParent(); PsiExpression fieldAccess = parent instanceof PsiReferenceExpression ? (PsiExpression)parent : element; LocalQuickFix[] fix = createNPEFixes(element, fieldAccess, holder.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY); - holder.registerProblem(element, problem.getMessage(), fix); + holder.registerProblem(element, problem.getMessage(expressions), fix); }); - NullabilityProblemKind.unboxingNullable.ifMyProblem(problem, element -> holder.registerProblem(element, problem.getMessage())); - NullabilityProblemKind.nullableFunctionReturn.ifMyProblem(problem, expr -> holder.registerProblem(expr, problem.getMessage())); - NullabilityProblemKind.assigningToNotNull.ifMyProblem(problem, expr -> reportNullabilityProblem(holder, problem, expr)); - NullabilityProblemKind.storingToNotNullArray.ifMyProblem(problem, expr -> reportNullabilityProblem(holder, problem, expr)); + NullabilityProblemKind.unboxingNullable.ifMyProblem(problem, element -> holder.registerProblem(element, problem.getMessage(expressions))); + NullabilityProblemKind.nullableFunctionReturn.ifMyProblem(problem, expr -> holder.registerProblem(expr, problem.getMessage(expressions))); + NullabilityProblemKind.assigningToNotNull.ifMyProblem(problem, expr -> reportNullabilityProblem(holder, problem, expr, expressions)); + NullabilityProblemKind.storingToNotNullArray.ifMyProblem(problem, expr -> reportNullabilityProblem(holder, problem, expr, expressions)); }); } - private void reportNullabilityProblem(ProblemsHolder holder, NullabilityProblem problem, PsiExpression expr) { - holder.registerProblem(expr, problem.getMessage(), createNPEFixes(expr, expr, holder.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY)); + private void reportNullabilityProblem(ProblemsHolder holder, + NullabilityProblem problem, + PsiExpression expr, + Map expressions) { + holder.registerProblem(expr, problem.getMessage(expressions), createNPEFixes(expr, expr, holder.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY)); } private static void reportArrayAccessProblems(ProblemsHolder holder, DataFlowInstructionVisitor visitor) { @@ -490,35 +560,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool }); } - private void reportConstantReferenceValues(ProblemsHolder holder, DataFlowInstructionVisitor visitor, Set reportedAnchors) { - visitor.getConstantReferenceValues().forEach((ref, dfaConst) -> { - if (ref.getParent() instanceof PsiReferenceExpression || DfaConstValue.isSentinel(dfaConst)) return; - if (!reportedAnchors.add(ref)) return; - - final Object value = dfaConst.getValue(); - PsiVariable constant = dfaConst.getConstant(); - final String exprText = String.valueOf(value); - final String presentableName = constant != null ? constant.getName() : exprText; - - List fixes = new SmartList<>(); - fixes.add(new ReplaceWithConstantValueFix(presentableName, exprText)); - boolean isAssertion = value instanceof Boolean && isAssertionEffectively(ref, (Boolean)value); - if (isAssertion && DONT_REPORT_TRUE_ASSERT_STATEMENTS) return; - if (holder.isOnTheFly()) { - fixes.add(new SetInspectionOptionFix(this, "REPORT_CONSTANT_REFERENCE_VALUES", - InspectionsBundle.message("inspection.data.flow.turn.off.constant.references.quickfix"), - false)); - if (isAssertion) { - fixes.add(new SetInspectionOptionFix(this, "DONT_REPORT_TRUE_ASSERT_STATEMENTS", - InspectionsBundle.message("inspection.data.flow.turn.off.true.asserts.quickfix"), true)); - } - } - - holder.registerProblem(ref, "Value #ref #loc is always '" + presentableName + "'", - ProblemHighlightType.WEAK_WARNING, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); - }); - } - private void reportNullableArgumentsPassedToNonAnnotated(DataFlowInstructionVisitor visitor, ProblemsHolder holder, Set reportedAnchors) { for (PsiElement expr : visitor.problems() .map(NullabilityProblemKind.passingNullableArgumentToNonAnnotatedParameter::asMyProblem).nonNull() @@ -581,20 +622,20 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool if (parent instanceof PsiAssignmentExpression) { PsiExpression lExpression = ((PsiAssignmentExpression)parent).getLExpression(); PsiElement target = lExpression instanceof PsiReferenceExpression ? ((PsiReferenceExpression)lExpression).resolve() : null; - return ObjectUtils.tryCast(target, PsiField.class); + return tryCast(target, PsiField.class); } return null; } - private void reportCallMayProduceNpe(ProblemsHolder holder, PsiMethodCallExpression callExpression) { + private void reportCallMayProduceNpe(ProblemsHolder holder, + String message, + PsiMethodCallExpression callExpression) { PsiReferenceExpression methodExpression = callExpression.getMethodExpression(); List fixes = createNPEFixes(methodExpression.getQualifierExpression(), callExpression, holder.isOnTheFly()); ContainerUtil.addIfNotNull(fixes, ReplaceWithObjectsEqualsFix.createFix(callExpression, methodExpression)); PsiElement toHighlight = getElementToHighlight(callExpression); - holder.registerProblem(toHighlight, - InspectionsBundle.message("dataflow.message.npe.method.invocation"), - fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); + holder.registerProblem(toHighlight, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); } private static void reportCastMayFail(ProblemsHolder holder, TypeCastInstruction instruction) { @@ -699,17 +740,33 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool holder.registerProblem(psiAnchor, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); } + @Contract("null -> false") private static boolean shouldBeSuppressed(PsiElement anchor) { if (!(anchor instanceof PsiExpression)) return false; + // Don't report System.out.println(b = false) + if (anchor instanceof PsiAssignmentExpression) return true; PsiExpression expression = (PsiExpression)anchor; + // Dereference of null will be covered by other warning + if (ExpressionUtils.isVoidContext(expression) || isDereferenceContext(expression)) return true; + if (isFlagCheck(anchor)) return true; + if (expression instanceof PsiReferenceExpression) { + PsiField field = tryCast(((PsiReferenceExpression)expression).resolve(), PsiField.class); + return field != null && field.hasModifierProperty(PsiModifier.STATIC) && ExpressionUtils.isNullLiteral(field.getInitializer()); + } while (expression != null && BoolUtils.isNegation(expression)) { expression = BoolUtils.getNegated(expression); } - PsiMethodCallExpression call = ObjectUtils.tryCast(expression, PsiMethodCallExpression.class); + PsiMethodCallExpression call = tryCast(expression, PsiMethodCallExpression.class); // Reported by "Equals with itself" inspection; avoid double reporting return call != null && EqualsWithItselfInspection.isEqualsWithItself(call); } + private static boolean isDereferenceContext(PsiExpression ref) { + PsiElement parent = PsiUtil.skipParenthesizedExprUp(ref.getParent()); + return parent instanceof PsiReferenceExpression || parent instanceof PsiArrayAccessExpression + || parent instanceof PsiSwitchStatement || parent instanceof PsiSynchronizedStatement; + } + private static LocalQuickFix createReplaceWithNullCheckFix(PsiElement psiAnchor, boolean evaluatesToTrue) { if (evaluatesToTrue) return null; if (!(psiAnchor instanceof PsiMethodCallExpression)) return null; @@ -843,7 +900,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool int index = ArrayUtil.indexOf(((PsiExpressionList)parent).getExpressions(), anchor); if (index >= 0) { ValueConstraint wantedConstraint = evaluatesToTrue ? ValueConstraint.FALSE_VALUE : ValueConstraint.TRUE_VALUE; - PsiMethodCallExpression call = ObjectUtils.tryCast(parent.getParent(), PsiMethodCallExpression.class); + PsiMethodCallExpression call = tryCast(parent.getParent(), PsiMethodCallExpression.class); if (call != null) { PsiMethod method = call.resolveMethod(); if (method != null) { @@ -875,7 +932,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool return false; } - static boolean isFlagCheck(PsiElement element) { + private static boolean isFlagCheck(PsiElement element) { PsiElement scope = PsiTreeUtil.getParentOfType(element, PsiStatement.class, PsiVariable.class); PsiExpression topExpression = scope instanceof PsiIfStatement ? ((PsiIfStatement)scope).getCondition() : scope instanceof PsiVariable ? ((PsiVariable)scope).getInitializer() : diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java index 55a929705418..ca188eb3d05d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -9,10 +9,8 @@ import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; -import com.intellij.psi.util.PsiUtil; import com.intellij.util.ThreeState; import com.intellij.util.containers.ContainerUtil; -import com.siyeh.ig.psiutils.ExpressionUtils; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -27,12 +25,11 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { private final Map, StateInfo> myStateInfos = new LinkedHashMap<>(); private final Set myCCEInstructions = ContainerUtil.newHashSet(); private final Map myFailingCalls = new HashMap<>(); - private final Map myBooleanExpressions = new HashMap<>(); + private final Map myConstantExpressions = new HashMap<>(); private final Map myOfNullableCalls = new HashMap<>(); private final Map> myArrayStoreProblems = new HashMap<>(); private final Map myMethodReferenceResults = new HashMap<>(); private final Map myOutOfBoundsArrayAccesses = new HashMap<>(); - private final Map myValues = new HashMap<>(); private final Set myReceiverMutabilityViolation = new HashSet<>(); private final Set myArgumentMutabilityViolation = new HashSet<>(); private final Map mySameValueAssigned = new HashMap<>(); @@ -128,8 +125,8 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { return myOfNullableCalls; } - Map getBooleanExpressions() { - return myBooleanExpressions; + Map getConstantExpressions() { + return myConstantExpressions; } Map getMethodReferenceResults() { @@ -167,7 +164,9 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { @Nullable TextRange range, @NotNull DfaMemoryState memState) { expression.accept(new ExpressionVisitor(value, memState)); - handleBooleanResults(value, memState, expression); + if (range == null) { + handleBooleanResults(value, memState, expression); + } } @Override @@ -213,10 +212,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { return super.visitEndOfInitializer(instruction, runner, state); } - public Map getConstantReferenceValues() { - return myValues; - } - private static boolean hasNonTrivialFailingContracts(PsiCallExpression call) { List contracts = JavaMethodContractUtil.getMethodCallContracts(call); return !contracts.isEmpty() && @@ -224,47 +219,18 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { } private void handleBooleanResults(DfaValue value, DfaMemoryState memState, PsiExpression expression) { - ThreeState curState = myBooleanExpressions.get(expression); - if (curState == ThreeState.UNSURE) return; - ThreeState nextState = ThreeState.UNSURE; - value = value instanceof DfaVariableValue ? memState.getConstantValue((DfaVariableValue)value) : value; - if (value instanceof DfaConstValue) { - Object val = ((DfaConstValue)value).getValue(); - if (val instanceof Boolean) { - nextState = ThreeState.fromBoolean((Boolean)val); - if (curState != null && curState != nextState) { - nextState = ThreeState.UNSURE; - } + if (expression instanceof PsiLiteralExpression) return; + ConstantResult curState = myConstantExpressions.get(expression); + if (curState == ConstantResult.UNKNOWN) return; + ConstantResult nextState = ConstantResult.UNKNOWN; + DfaConstValue dfaConst = memState.getConstantValue(value); + if (dfaConst != null) { + nextState = ConstantResult.fromConstValue(dfaConst); + if (curState != null && curState != nextState) { + nextState = ConstantResult.UNKNOWN; } } - if (curState != null || shouldCollectBooleanResult(expression)) { - myBooleanExpressions.put(expression, nextState); - } - } - - private static boolean shouldCollectBooleanResult(PsiExpression expression) { - if (expression instanceof PsiLiteralExpression) return false; - PsiType type = expression.getType(); - if (type == null || !PsiType.BOOLEAN.isAssignableFrom(type)) return false; - if (expression instanceof PsiPrefixExpression || expression instanceof PsiPolyadicExpression) { - return !DataFlowInspectionBase.isFlagCheck(expression); - } - PsiPolyadicExpression polyadic = tryCast(PsiUtil.skipParenthesizedExprUp(expression.getParent()), PsiPolyadicExpression.class); - if (polyadic != null) { - if ((polyadic.getOperationTokenType().equals(JavaTokenType.ANDAND) || polyadic.getOperationTokenType().equals(JavaTokenType.OROR)) && - !DataFlowInspectionBase.isFlagCheck(expression)) return true; - } - if (expression instanceof PsiMethodCallExpression) { - PsiMethodCallExpression call = (PsiMethodCallExpression)expression; - if (ExpressionUtils.isVoidContext(call)) return false; - PsiMethod method = call.resolveMethod(); - if (method == null || !JavaMethodContractUtil.isPure(method)) return false; - List contracts = JavaMethodContractUtil.getMethodCallContracts(method, call); - return CustomMethodHandlers.find(method) != null || - !contracts.isEmpty() && - contracts.stream().anyMatch(contract -> contract.getReturnValue().isBoolean() && !contract.isTrivial()); - } - return false; + myConstantExpressions.put(expression, nextState); } @Override @@ -302,10 +268,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { } } - private static boolean shouldReportConstValue(Object value) { - return value == null || value instanceof Boolean; - } - private static class StateInfo { boolean ephemeralNpe; boolean normalNpe; @@ -337,23 +299,37 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { myFailingCalls.put(call, DfaConstValue.isContractFail(myValue) && !Boolean.FALSE.equals(isFailing)); } } + } + enum ConstantResult { + TRUE, FALSE, NULL, UNKNOWN; + + @NotNull @Override - public void visitReferenceExpression(PsiReferenceExpression expression) { - super.visitReferenceExpression(expression); - DfaConstValue oldValue = myValues.get(expression); - if (DfaConstValue.isSentinel(oldValue)) return; - if (myValue instanceof DfaVariableValue) { - DfaConstValue constValue = myMemState.getConstantValue((DfaVariableValue)myValue); - boolean report = constValue != null && shouldReportConstValue(constValue.getValue()); - if (!report) { - constValue = null; - } - DfaConstValue newValue = constValue != null && (oldValue == null || oldValue == constValue) - ? constValue - : myValue.getFactory().getConstFactory().getSentinel(); - myValues.put(expression, newValue); + public String toString() { + return name().toLowerCase(Locale.ENGLISH); + } + + public Object value() { + switch (this) { + case TRUE: + return Boolean.TRUE; + case FALSE: + return Boolean.FALSE; + case NULL: + return null; + default: + throw new UnsupportedOperationException(); } } + + @NotNull + static ConstantResult fromConstValue(@NotNull DfaConstValue constant) { + Object value = constant.getValue(); + if (value == null) return NULL; + if (Boolean.TRUE.equals(value)) return TRUE; + if (Boolean.FALSE.equals(value)) return FALSE; + return UNKNOWN; + } } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java index 8875b8f39ae2..80a8b126067d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java @@ -19,6 +19,7 @@ import com.intellij.codeInspection.dataFlow.value.DfaConstValue; import com.intellij.codeInspection.dataFlow.value.DfaPsiType; import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -123,8 +124,15 @@ public interface DfaMemoryState { boolean isNotNull(DfaValue dfaVar); + /** + * Returns a constant value which equals to given value, if such. + * + * @param value a value to find a corresponding constant + * @return found constant or null + */ @Nullable - DfaConstValue getConstantValue(@NotNull DfaVariableValue value); + @Contract("null -> null") + DfaConstValue getConstantValue(@Nullable DfaValue value); /** * Ephemeral means a state that was created when considering a method contract and checking if one of its arguments is null. diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 8ce9787354aa..23cb372d132b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -36,6 +36,7 @@ import com.intellij.util.containers.Stack; import gnu.trove.TIntObjectHashMap; import gnu.trove.TIntObjectProcedure; import one.util.streamex.StreamEx; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -581,7 +582,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaVar instanceof DfaVariableValue) { if (getVariableState((DfaVariableValue)dfaVar).isNotNull()) return true; - DfaConstValue constantValue = getConstantValue((DfaVariableValue)dfaVar); + DfaConstValue constantValue = getConstantValue(dfaVar); if (constantValue != null && constantValue.getValue() != null) return true; } @@ -597,10 +598,17 @@ public class DfaMemoryStateImpl implements DfaMemoryState { @Override @Nullable - public DfaConstValue getConstantValue(@NotNull DfaVariableValue value) { - int index = getEqClassIndex(value); - EqClass ec = index == -1 ? null : myEqClasses.get(index); - return ec == null ? null : (DfaConstValue)unwrap(ec.findConstant(true)); + @Contract("null -> null") + public DfaConstValue getConstantValue(@Nullable DfaValue value) { + if (value instanceof DfaConstValue) { + return (DfaConstValue)value; + } + if (value instanceof DfaUnboxedValue || value instanceof DfaVariableValue) { + int index = getEqClassIndex(value); + EqClass ec = index == -1 ? null : myEqClasses.get(index); + return ec == null ? null : (DfaConstValue)unwrap(ec.findConstant(true)); + } + return null; } @Override diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java index d92307efd124..ba3df68997ec 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java @@ -196,9 +196,7 @@ public abstract class InstructionVisitor { public DfaInstructionState[] visitObjectOfInstruction(ObjectOfInstruction instruction, DataFlowRunner runner, DfaMemoryState state) { DfaValue value = state.pop(); - DfaConstValue constant = value instanceof DfaConstValue ? (DfaConstValue)value : - value instanceof DfaVariableValue ? state.getConstantValue((DfaVariableValue)value) : - null; + DfaConstValue constant = state.getConstantValue(value); PsiType type = constant == null ? null : ObjectUtils.tryCast(constant.getValue(), PsiType.class); state.push(runner.getFactory().createTypeValue(type, Nullability.NOT_NULL)); return nextInstruction(instruction, runner, state); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java index 8aabd487a768..0c0bc17330be 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java @@ -2,14 +2,18 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInspection.InspectionsBundle; import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ObjectUtils; import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.PropertyKey; +import java.util.Map; import java.util.Objects; import java.util.function.Consumer; +import java.util.function.Function; import static com.intellij.codeInspection.InspectionsBundle.BUNDLE; @@ -18,14 +22,17 @@ import static com.intellij.codeInspection.InspectionsBundle.BUNDLE; * @param a type of anchor element which could be associated with given nullability problem kind */ public class NullabilityProblemKind { + private static final Function DEFAULT_DEREFERENCED_ELEMENT_FN = e -> ObjectUtils.tryCast(e, PsiExpression.class); private final String myName; - private final String myNullLiteralMessage; + private final String myAlwaysNullMessage; private final String myNormalMessage; + private final Function myDereferencedElementFunction; private NullabilityProblemKind(@NotNull String name) { myName = name; - myNullLiteralMessage = null; + myAlwaysNullMessage = null; myNormalMessage = null; + myDereferencedElementFunction = DEFAULT_DEREFERENCED_ELEMENT_FN; } private NullabilityProblemKind(@NotNull String name, @NotNull @PropertyKey(resourceBundle = BUNDLE) String message) { @@ -33,22 +40,34 @@ public class NullabilityProblemKind { } private NullabilityProblemKind(@NotNull String name, - @NotNull @PropertyKey(resourceBundle = BUNDLE) String nullLiteralMessage, + @NotNull @PropertyKey(resourceBundle = BUNDLE) String alwaysNullMessage, @NotNull @PropertyKey(resourceBundle = BUNDLE) String normalMessage) { - myName = name; - myNullLiteralMessage = InspectionsBundle.message(nullLiteralMessage); - myNormalMessage = InspectionsBundle.message(normalMessage); + this(name, alwaysNullMessage, normalMessage, DEFAULT_DEREFERENCED_ELEMENT_FN); } - public static final NullabilityProblemKind callNPE = new NullabilityProblemKind<>("callNPE"); + private NullabilityProblemKind(@NotNull String name, + @NotNull @PropertyKey(resourceBundle = BUNDLE) String alwaysNullMessage, + @NotNull @PropertyKey(resourceBundle = BUNDLE) String normalMessage, + Function dereferencedElementFunction) { + myName = name; + myAlwaysNullMessage = InspectionsBundle.message(alwaysNullMessage); + myNormalMessage = InspectionsBundle.message(normalMessage); + myDereferencedElementFunction = dereferencedElementFunction; + } + + public static final NullabilityProblemKind callNPE = + new NullabilityProblemKind<>("callNPE", "dataflow.message.npe.method.invocation.sure", "dataflow.message.npe.method.invocation", + call -> call.getMethodExpression().getQualifierExpression()); public static final NullabilityProblemKind callMethodRefNPE = new NullabilityProblemKind<>("callMethodRefNPE", "dataflow.message.npe.methodref.invocation"); public static final NullabilityProblemKind innerClassNPE = - new NullabilityProblemKind<>("innerClassNPE", "dataflow.message.npe.inner.class.construction"); + new NullabilityProblemKind<>("innerClassNPE", "dataflow.message.npe.inner.class.construction.sure", + "dataflow.message.npe.inner.class.construction", PsiNewExpression::getQualifier); public static final NullabilityProblemKind fieldAccessNPE = new NullabilityProblemKind<>("fieldAccessNPE", "dataflow.message.npe.field.access.sure", "dataflow.message.npe.field.access"); public static final NullabilityProblemKind arrayAccessNPE = - new NullabilityProblemKind<>("arrayAccessNPE", "dataflow.message.npe.array.access"); + new NullabilityProblemKind<>("arrayAccessNPE", "dataflow.message.npe.array.access.sure", "dataflow.message.npe.array.access", + PsiArrayAccessExpression::getArrayExpression); public static final NullabilityProblemKind unboxingNullable = new NullabilityProblemKind<>("unboxingNullable", "dataflow.message.unboxing"); public static final NullabilityProblemKind assigningToNotNull = @@ -138,13 +157,18 @@ public class NullabilityProblemKind { } @NotNull - public String getMessage() { - if (myKind.myNullLiteralMessage == null || myKind.myNormalMessage == null) { + public String getMessage(Map expressions) { + if (myKind.myAlwaysNullMessage == null || myKind.myNormalMessage == null) { throw new IllegalStateException("This problem kind has no message associated: " + myKind); } - return myAnchor instanceof PsiExpression && ExpressionUtils.isNullLiteral((PsiExpression)myAnchor) - ? myKind.myNullLiteralMessage - : myKind.myNormalMessage; + PsiExpression expression = + PsiUtil.skipParenthesizedExprDown(ObjectUtils.tryCast(myKind.myDereferencedElementFunction.apply(myAnchor), PsiExpression.class)); + if (expression != null) { + if (ExpressionUtils.isNullLiteral(expression) || expressions.get(expression) == DataFlowInstructionVisitor.ConstantResult.NULL) { + return myKind.myAlwaysNullMessage; + } + } + return myKind.myNormalMessage; } @Override 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 f28eb5b764cd..b10190ed8263 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 @@ -702,9 +702,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { boolean unknownTargetType = false; DfaValue condition = null; if (instruction.isClassObjectCheck()) { - DfaConstValue constant = dfaRight instanceof DfaConstValue ? (DfaConstValue)dfaRight : - dfaRight instanceof DfaVariableValue ? memState.getConstantValue((DfaVariableValue)dfaRight) : - null; + DfaConstValue constant = memState.getConstantValue(dfaRight); PsiType type = constant == null ? null : ObjectUtils.tryCast(constant.getValue(), PsiType.class); if (type == null || type instanceof PsiPrimitiveType) { // Unknown/primitive class: just execute contract "null -> false" diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java index 193ad45ba12d..eba338266eda 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java @@ -113,7 +113,10 @@ public class DfaConstValue extends DfaValue { if (TypeConversionUtil.isNumericType(type) && !TypeConversionUtil.isFloatOrDoubleType(type)) { type = PsiType.LONG; - value = TypeConversionUtil.computeCastTo(value, type); + Object numeric = TypeConversionUtil.computeCastTo(value, type); + if (numeric != null) { + value = numeric; + } } if (value instanceof Double || value instanceof Float) { double doubleValue = ((Number)value).doubleValue(); @@ -202,14 +205,4 @@ public class DfaConstValue extends DfaValue { public static boolean isContractFail(DfaValue value) { return value instanceof DfaConstValue && ((DfaConstValue)value).getValue() == ourThrowable; } - - /** - * Checks whether given value is a special internal sentinel value returned by {@link Factory#getSentinel()}. - * - * @param value value to check - * @return true if specified value is a sentinel value - */ - public static boolean isSentinel(DfaValue value) { - return value instanceof DfaConstValue && ((DfaConstValue)value).getValue() == SENTINEL; - } } diff --git a/java/java-tests/testData/inspection/dataFlow/ArrayAccessNPE/expected.xml b/java/java-tests/testData/inspection/dataFlow/ArrayAccessNPE/expected.xml index 217863aecadc..ffec26eaa389 100644 --- a/java/java-tests/testData/inspection/dataFlow/ArrayAccessNPE/expected.xml +++ b/java/java-tests/testData/inspection/dataFlow/ArrayAccessNPE/expected.xml @@ -3,6 +3,6 @@ Test.java 5 - Array access <code>path[0]</code> may produce <code>java.lang.NullPointerException</code> + Array access <code>path[0]</code> will produce <code>java.lang.NullPointerException</code> diff --git a/java/java-tests/testData/inspection/dataFlow/SCR15162/expected.xml b/java/java-tests/testData/inspection/dataFlow/SCR15162/expected.xml index 5f2ee3514ef6..ff17aac7396a 100644 --- a/java/java-tests/testData/inspection/dataFlow/SCR15162/expected.xml +++ b/java/java-tests/testData/inspection/dataFlow/SCR15162/expected.xml @@ -3,7 +3,7 @@ NullTest.java 6 - Dereference of 't' may produce java.lang.NullPointerException + Dereference of 't' will produce java.lang.NullPointerException diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java b/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java index e8856ceaa36f..3b7cf01eb810 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java @@ -108,7 +108,7 @@ class AdvancedArrayAccess { void testMethodQualifier() { if(getData()[0] == null) { - System.out.println(getData()[0].trim()); + System.out.println(getData()[0].trim()); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java b/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java index 2b02403f0de9..7aca1a5952f7 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java @@ -29,7 +29,7 @@ class S { public void te3(boolean b){ Boolean c = Boolean.FALSE; - boolean o = !c; + boolean o = !c; if (o) { } } @@ -41,13 +41,13 @@ class S { } public void te5(boolean b){ Boolean c = Boolean.TRUE; - boolean o = b||c; + boolean o = b||c; if (o) { } } public void te6(boolean b){ Boolean c = Boolean.TRUE; - boolean o = !c; + boolean o = !c; o |= c&b; if (o) { } @@ -57,7 +57,7 @@ class S { boolean o; { Boolean c = Boolean.FALSE; - o = !c; + o = !c; } if (o) { } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ComparingToNotNullShouldNotAffectNullity.java b/java/java-tests/testData/inspection/dataFlow/fixture/ComparingToNotNullShouldNotAffectNullity.java index 47fef5d8203b..32e4e199cb3c 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ComparingToNotNullShouldNotAffectNullity.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ComparingToNotNullShouldNotAffectNullity.java @@ -40,7 +40,7 @@ class Bar3 { System.out.println(first.hashCode()); } if (first == null) { - System.out.println(first.hashCode()); + System.out.println(first.hashCode()); } } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java b/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java index a59c40cc197d..deb220921573 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java @@ -13,7 +13,7 @@ class InitializerTest { z = "foo"; } - boolean b = z.startsWith("bar"); + boolean b = z.startsWith("bar"); static final String ABC; static { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java b/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java index a3b17758cb2b..c6e0f9622307 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java @@ -7,7 +7,7 @@ import java.lang.IllegalArgumentException; class AssertIsNotNull { void bar(String s, String s1) { - if (s == null && trimIfNotNull(s) != null) { + if (s == null && trimIfNotNull(s) != null) { throw new AssertionError(); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ContractPreservesUnknownMethodNullability.java b/java/java-tests/testData/inspection/dataFlow/fixture/ContractPreservesUnknownMethodNullability.java index 847ba565398b..8c168d8f63c5 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ContractPreservesUnknownMethodNullability.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ContractPreservesUnknownMethodNullability.java @@ -8,7 +8,7 @@ class TestCase { String s = normalizeSpace(unknown()).trim(); String s3 = normalizeSpaceInverted(unknown()).trim(); - String s4 = normalizeSpace(null).trim(); + String s4 = normalizeSpace(null).trim(); } public static native String unknown(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/FieldUsedBeforeInitialization.java b/java/java-tests/testData/inspection/dataFlow/fixture/FieldUsedBeforeInitialization.java index bb5539b072d0..e2c9a4f648ed 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/FieldUsedBeforeInitialization.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/FieldUsedBeforeInitialization.java @@ -1,6 +1,6 @@ class Foo { String field; - String field2 = field.substring(1); + String field2 = field.substring(1); int field3 = field2.length(); Runnable r = new Runnable() { public void run() { @@ -74,7 +74,7 @@ class FieldInitNoLoop { class NonFinalNotInitialized { String x; - String y = x.trim(); + String y = x.trim(); } class NonFinalInitialized { @@ -87,7 +87,7 @@ class NonFinalInitialized { class NonFinalAssignedInside { String x; - String y = x.trim()+(x = " foo ").trim(); + String y = x.trim()+(x = " foo ").trim(); String z = x.trim(); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ImplicitlyInitializedField.java b/java/java-tests/testData/inspection/dataFlow/fixture/ImplicitlyInitializedField.java index 8c319081f174..02a4a5c9af1b 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ImplicitlyInitializedField.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ImplicitlyInitializedField.java @@ -1,7 +1,7 @@ class Foo { String field; String field2; - int hash = field.hashCode(); + int hash = field.hashCode(); Foo(String f2) { field2 = f2; @@ -23,7 +23,7 @@ class Foo { Instrumented() { System.out.println(s1.length() - +s2.length()); + +s2.length()); } } @@ -32,8 +32,8 @@ class Foo { String s2 = null; NotInstrumented() { - System.out.println(s1.length() - +s2.length()); + System.out.println(s1.length() + +s2.length()); } } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java b/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java index d04701779166..c179507fd44c 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java @@ -37,7 +37,7 @@ class LessThanRelations { void list(List list, int index) { if(index >= list.size()) { System.out.println("Big index"); - } else if(index > 0 && !list.isEmpty()) { + } else if(index > 0 && !list.isEmpty()) { System.out.println("ok"); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeBasics.java b/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeBasics.java index ff8ccb0543eb..dd647af23b17 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeBasics.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeBasics.java @@ -198,8 +198,8 @@ public class LongRangeBasics { System.out.println(s2.trim()); } if(code == 0) { - System.out.println(s1.trim()); - System.out.println(s2.trim()); + System.out.println(s1.trim()); + System.out.println(s2.trim()); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java b/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java index 29fa417f2f7a..1dda25d92e8a 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java @@ -162,8 +162,8 @@ public class LongRangeKnownMethods { void testStringComparison(String name) { // Parentheses misplaced -- found in AndroidStudio - if (!(name.equals("layout_width") && !(name.equals("layout_height")) && - !(name.equals("id")))) { + if (!(name.equals("layout_width") && !(name.equals("layout_height")) && + !(name.equals("id")))) { System.out.println("ok"); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MergedInitializerAndConstructor.java b/java/java-tests/testData/inspection/dataFlow/fixture/MergedInitializerAndConstructor.java index 25512a8b4788..dd630c13e9d7 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/MergedInitializerAndConstructor.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MergedInitializerAndConstructor.java @@ -6,7 +6,7 @@ class MergedInitializerAndConstructor { private Collection collection2 = null; public Test1() { - collection2.add(""); + collection2.add(""); } } @@ -18,7 +18,7 @@ class MergedInitializerAndConstructor { } { - collection2.add(""); //<- warning here + collection2.add(""); //<- warning here } } @@ -27,7 +27,7 @@ class MergedInitializerAndConstructor { public Test3(String s) { super(); - collection2.add(s); + collection2.add(s); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java index 007e4b3efc89..247af4a3f5db 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java @@ -71,7 +71,7 @@ public class MutabilityJdk { List list3 = Collections.unmodifiableList(list2); if(list1.isEmpty()) System.out.println("ok"); if(!list3.isEmpty()) return; - if(!list3.isEmpty()) return; + if(!list3.isEmpty()) return; list2.add("foo"); // list1 size is not flushed (UNMODIFIABLE) if(list1.isEmpty()) System.out.println("ok"); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutableNotAnnotatedFieldsTreatment.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutableNotAnnotatedFieldsTreatment.java index abb8cb973d8f..6da35aad7f63 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/MutableNotAnnotatedFieldsTreatment.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutableNotAnnotatedFieldsTreatment.java @@ -20,7 +20,7 @@ class Foo { if (data != null) { return; } - System.out.println(data.hashCode()); + System.out.println(data.hashCode()); System.out.println(data.hashCode()); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutableNullableFieldsTreatment.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutableNullableFieldsTreatment.java index b9c18b463e23..555cb395d936 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/MutableNullableFieldsTreatment.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutableNullableFieldsTreatment.java @@ -20,7 +20,7 @@ class Foo { if (data != null) { return; } - System.out.println(data.hashCode()); + System.out.println(data.hashCode()); System.out.println(data.hashCode()); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java index 6fce0240cc6a..ab1926e1497f 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java @@ -56,7 +56,7 @@ public class OptionalInlining { void testDeref(Optional opt) { if (opt == null) { - System.out.println(opt.orElse("qq")); + System.out.println(opt.orElse("qq")); } } @@ -68,7 +68,7 @@ public class OptionalInlining { } return "baz"; }); - if (s.equals("bar") && !opt.isPresent()) { + if (s.equals("bar") && !opt.isPresent()) { System.out.println("Impossible"); } } @@ -87,7 +87,7 @@ public class OptionalInlining { if (abc.equals("xyz") && opt.isPresent()) { System.out.println("always"); } - opt.filter(x -> x.length() > 5).filter(x -> x.isEmpty()).ifPresent(x -> System.out.println(x)); + opt.filter(x -> x.length() > 5).filter(x -> x.isEmpty()).ifPresent(x -> System.out.println(x)); } @Nullable @@ -111,7 +111,7 @@ public class OptionalInlining { void testMap(Optional opt) { opt.map(null); String res = opt.map(s -> null).orElse("abc"); - if (!res.equals("abc")) { + if (!res.equals("abc")) { System.out.println("Never"); } String trimmed = Optional.ofNullable(nullableMethod()).map(xx -> xx.trim()).orElse(""); @@ -170,7 +170,7 @@ public class OptionalInlining { if (s.equals("qux")) { System.out.println("Never"); } - boolean res = opt.filter(x -> x.isEmpty()).flatMap(x -> x.length() <= 2 ? Optional.empty() : Optional.of("foo")) + boolean res = opt.filter(x -> x.isEmpty()).flatMap(x -> x.length() <= 2 ? Optional.empty() : Optional.of("foo")) .isPresent(); } @@ -214,14 +214,14 @@ public class OptionalInlining { } void testFilterChain(Optional opt) { - boolean present = opt + boolean present = opt .filter(h -> h.x < 5) .filter(h -> h.x > 6) .map(h -> h.x).isPresent(); } void testFilterMap(Optional opt) { - boolean present = opt + boolean present = opt .filter(h -> h.s == null) .map(h -> h.s) .isPresent(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java index 4ad91a73ef07..f2ee506a223b 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java @@ -7,7 +7,7 @@ class Test { test = Optional.of("x"); } else { test = Optional.empty(); - if(!test.isPresent()) { + if(!test.isPresent()) { System.out.println("Always"); } } @@ -30,8 +30,8 @@ class Test { System.out.println(maybe.get()); } boolean b = ((maybe.isPresent())) && maybe.get() == 1; - boolean c = (!maybe.isPresent()) || maybe.get() == 1; - Integer value = !maybe.isPresent() ? 0 : maybe.get(); + boolean c = (!maybe.isPresent()) || maybe.get() == 1; + Integer value = !maybe.isPresent() ? 0 : maybe.get(); } Optional getIntegerOptional() { @@ -40,7 +40,7 @@ class Test { private static void a() { Optional optional = Optional.empty(); - final boolean present = optional.isPresent(); + final boolean present = optional.isPresent(); // optional = Optional.empty(); if (present) { final String string = optional.get(); @@ -50,7 +50,7 @@ class Test { private static void b() { Optional optional = Optional.empty(); - final boolean present = optional.isPresent(); + final boolean present = optional.isPresent(); optional = Optional.empty(); if (present) { final String string = optional.get(); @@ -81,7 +81,7 @@ class Test { private void checkAsserts2() { Optional o3 = Optional.empty(); - org.testng.Assert.assertTrue(o3.isPresent()); + org.testng.Assert.assertTrue(o3.isPresent()); System.out.println(o3.get()); } @@ -100,9 +100,9 @@ class Test { public static String demo() { Optional holder = Optional.empty(); - if (! holder.isPresent()) { + if (! holder.isPresent()) { holder = Optional.of("hello world"); - if (!holder.isPresent()) { + if (!holder.isPresent()) { return null; } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ReportAlwaysNull.java b/java/java-tests/testData/inspection/dataFlow/fixture/ReportAlwaysNull.java new file mode 100644 index 000000000000..9731e24619e3 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ReportAlwaysNull.java @@ -0,0 +1,22 @@ +import org.jetbrains.annotations.*; +import java.util.List; + +class Test { + static final Object RES = null; + final Object xyz = null; + static final Integer TEST = new Integer(0); + + void test() { + doSmth(RES); + doSmth(xyz); + System.out.println(process(xyz).hashCode()); + Integer x = TEST; + System.out.println(TEST); + System.out.println(x); + } + + @Contract("null -> null") + native Object process(@Nullable Object obj); + + native void doSmth(@Nullable Object obj); +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences.java b/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences.java index 836b7fd74095..05aed07f99b1 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences.java @@ -20,7 +20,7 @@ class Test { public void testDontReplaceQualifierWithNull(Object bar) { if (bar == null) { - bar.hashCode(); + bar.hashCode(); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java index 5fa6afb5f8d8..b9edaae340ca 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java @@ -12,11 +12,11 @@ public class StreamInlining { list.stream().flatMap(null).forEach(System.out::println); list.stream().filter(x -> x != null).forEach(null); List l = null; - l.stream().count(); + l.stream().count(); int[] arr = null; Arrays.stream(arr).count(); Stream stream = null; - stream.filter(x -> x != null).forEach(System.out::println); + stream.filter(x -> x != null).forEach(System.out::println); } void testMethodRef(List list, int[] data) { @@ -125,10 +125,10 @@ public class StreamInlining { boolean flatMap(List list, List> ll) { System.out.println(ll.stream().flatMap(l -> l.stream()).count()); - return list.stream().map(s -> s.isEmpty() ? null : s) + return list.stream().map(s -> s.isEmpty() ? null : s) .flatMap(s -> Stream.of(s, s.trim()) .filter(r -> r != null)) - .anyMatch(x -> x == null); + .anyMatch(x -> x == null); } String blockLambda(List list) { @@ -169,7 +169,7 @@ public class StreamInlining { void testGenerate() { List list1 = Stream.generate(() -> Math.random() > 0.5 ? "foo" : "baz") - .limit(10).filter((xyz -> "bar".equals(xyz))).collect(Collectors.toList()); + .limit(10).filter((xyz -> "bar".equals(xyz))).collect(Collectors.toList()); List list2 = Stream.generate(() -> "xyz").limit(20).filter("bar"::equals).collect(Collectors.toList()); Stream.generate(() -> Optional.of("xyz")).filter(Optional::isPresent).forEach(System.out::println); LongStream.generate(() -> 5).limit(10).filter(x -> x > 6).forEach(s -> System.out.println(s)); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamKnownSource.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamKnownSource.java index 61f823b85af5..9c66fbc21612 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/StreamKnownSource.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamKnownSource.java @@ -38,15 +38,15 @@ public class StreamKnownSource { System.out.println("Probably"); } - boolean emptyAll = Stream.empty().allMatch(Objects::nonNull); + boolean emptyAll = Stream.empty().allMatch(Objects::nonNull); if (emptyAll) { System.out.println("True"); } - boolean emptyAny = Stream.empty().anyMatch(Objects::nonNull); + boolean emptyAny = Stream.empty().anyMatch(Objects::nonNull); if (emptyAny) { System.out.println("False"); } - boolean emptyNone = Stream.empty().noneMatch(Objects::nonNull); + boolean emptyNone = Stream.empty().noneMatch(Objects::nonNull); if (emptyNone) { System.out.println("True"); } @@ -119,7 +119,7 @@ public class StreamKnownSource { return; } - boolean hasNoNulls = list.stream().allMatch(Objects::nonNull); + boolean hasNoNulls = list.stream().allMatch(Objects::nonNull); if(hasNoNulls) { System.out.println("Always"); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ThisAsVariable.java b/java/java-tests/testData/inspection/dataFlow/fixture/ThisAsVariable.java index 8c87d0b3e955..7ac37a4d6bd0 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ThisAsVariable.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ThisAsVariable.java @@ -93,7 +93,7 @@ class ThisAsVariable { } else { Runnable r = new Runnable() { public void run() { - System.out.println(s.trim()); + System.out.println(s.trim()); } }; r.run(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/TryWithResourcesNullability.java b/java/java-tests/testData/inspection/dataFlow/fixture/TryWithResourcesNullability.java index bfe69d78926f..24ff22961ea7 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/TryWithResourcesNullability.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/TryWithResourcesNullability.java @@ -9,7 +9,7 @@ class Test { void m1() throws Exception { MyResourceProvider provider = null; - try (MyResource r = provider.getResource()) { + try (MyResource r = provider.getResource()) { System.out.println(r); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/UnknownOnStack.java b/java/java-tests/testData/inspection/dataFlow/fixture/UnknownOnStack.java index cf13344c5bea..23b6dbeecca3 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/UnknownOnStack.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/UnknownOnStack.java @@ -7,7 +7,7 @@ class Test { private void method() { if (timeStamp == null) { - timeStamp.set(Calendar.getInstance().getTimeInMillis()); // not reported + timeStamp.set(Calendar.getInstance().getTimeInMillis()); // not reported } } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/VariablesDiverge.java b/java/java-tests/testData/inspection/dataFlow/fixture/VariablesDiverge.java index e7b1d0bbd277..40ba44428b8e 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/VariablesDiverge.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/VariablesDiverge.java @@ -8,7 +8,7 @@ class Some { while (parent != null) { parent = parent.getParentFile(); } - System.out.println(parent.getName()); + System.out.println(parent.getName()); } System.out.println(file.getName()); } diff --git a/java/java-tests/testData/inspection/dataFlow/nullableField/expected.xml b/java/java-tests/testData/inspection/dataFlow/nullableField/expected.xml index 0d2f512661b3..834a81ab6230 100644 --- a/java/java-tests/testData/inspection/dataFlow/nullableField/expected.xml +++ b/java/java-tests/testData/inspection/dataFlow/nullableField/expected.xml @@ -3,7 +3,7 @@ Test.java 6 - 'equals' may produce NullPointerException + 'equals' will produce NullPointerException diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 244b5a0c2f29..aa3bb4f6d79d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -625,6 +625,12 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testBoxingBoolean() { doTest(); } public void testOrWithAssignment() { doTest(); } public void testAndAndLastOperand() { doTest(); } + public void testReportAlwaysNull() { + DataFlowInspection inspection = new DataFlowInspection(); + inspection.REPORT_CONSTANT_REFERENCE_VALUES = true; + myFixture.enableInspections(inspection); + myFixture.testHighlighting(true, false, true, getTestName(false) + ".java"); + } public void testAndAndWithOr() { doTest(); } public void testBoxUnboxArrayElement() { doTest(); } public void testExactInstanceOf() { doTest(); } diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 42c42e0eb71e..48ee14c8024d 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -56,9 +56,12 @@ configure.annotations.option=Configure annotations #messages from dataflow inspection dataflow.message.npe.method.invocation=Method invocation #ref #loc may produce java.lang.NullPointerException +dataflow.message.npe.method.invocation.sure=Method invocation #ref #loc will produce java.lang.NullPointerException dataflow.message.npe.inner.class.construction=Inner class construction may produce java.lang.NullPointerException +dataflow.message.npe.inner.class.construction.sure=Inner class construction will produce java.lang.NullPointerException dataflow.message.npe.methodref.invocation=Method reference invocation #ref #loc may produce java.lang.NullPointerException dataflow.message.npe.array.access=Array access #ref #loc may produce java.lang.NullPointerException +dataflow.message.npe.array.access.sure=Array access #ref #loc will produce java.lang.NullPointerException dataflow.message.npe.field.access.sure=Dereference of #ref #loc will produce java.lang.NullPointerException dataflow.message.npe.field.access=Dereference of #ref #loc may produce java.lang.NullPointerException dataflow.message.cce=Casting {0} to #ref #loc may produce java.lang.ClassCastException