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 3ef6d6aa6285..2ac602d69d54 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 @@ -270,7 +270,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { reportNullableArgumentsPassedToNonAnnotated(visitor, holder, reportedAnchors); } - reportOptionalOfNullableImprovements(holder, visitor, reportedAnchors); + reportOptionalOfNullableImprovements(holder, reportedAnchors, runner.getInstructions()); if (REPORT_CONSTANT_REFERENCE_VALUES) { @@ -278,18 +278,26 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { } } - private static void reportOptionalOfNullableImprovements(ProblemsHolder holder, - DataFlowInstructionVisitor visitor, - HashSet reportedAnchors) { - for (PsiElement expr : visitor.getProblems(NullabilityProblem.passingNullToOptional)) { - if (!reportedAnchors.add(expr)) continue; - holder.registerProblem(expr, "Passing null argument to Optional", - DfaOptionalSupport.createReplaceOptionalOfNullableWithEmptyFix(expr)); - } - for (PsiElement expr : visitor.getProblems(NullabilityProblem.passingNotNullToOptional)) { - if (!reportedAnchors.add(expr)) continue; - holder.registerProblem(expr, "Passing a non-null argument to Optional", - DfaOptionalSupport.createReplaceOptionalOfNullableWithOfFix()); + private static void reportOptionalOfNullableImprovements(ProblemsHolder holder, Set reportedAnchors, Instruction[] instructions) { + for (Instruction instruction : instructions) { + if (instruction instanceof MethodCallInstruction) { + final PsiExpression[] args = ((MethodCallInstruction)instruction).getArgs(); + if (args.length != 1) continue; + + final PsiExpression expr = args[0]; + + if (((MethodCallInstruction)instruction).isOptionalAlwaysNullProblem()) { + if (!reportedAnchors.add(expr)) continue; + holder.registerProblem(expr, "Passing null argument to Optional", + DfaOptionalSupport.createReplaceOptionalOfNullableWithEmptyFix(expr)); + } + else if (((MethodCallInstruction)instruction).isOptionalAlwaysNotNullProblem()) { + if (!reportedAnchors.add(expr)) continue; + holder.registerProblem(expr, "Passing a non-null argument to Optional", + DfaOptionalSupport.createReplaceOptionalOfNullableWithOfFix()); + } + + } } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java index 12c0d3ca6f3f..d880c023bf73 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java @@ -29,7 +29,7 @@ import org.jetbrains.annotations.Nullable; /** * @author anet, peter */ -class DfaOptionalSupport { +public class DfaOptionalSupport { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.DfaOptionalSupport"); private static final String GUAVA_OPTIONAL = "com.google.common.base.Optional"; @@ -77,8 +77,8 @@ class DfaOptionalSupport { } @Nullable - static PsiMethod resolveOfNullable(PsiCallExpression expression) { - String name = ((PsiMethodCallExpression)expression).getMethodExpression().getReferenceName(); + public static PsiMethod resolveOfNullable(@NotNull PsiMethodCallExpression expression) { + String name = expression.getMethodExpression().getReferenceName(); if ("ofNullable".equals(name) || "fromNullable".equals(name)) { PsiMethod method = expression.resolveMethod(); PsiClass psiClass = method == null ? null : method.getContainingClass(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblem.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblem.java index 2810ffdb2e34..9240f5d3383e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblem.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblem.java @@ -10,7 +10,5 @@ public enum NullabilityProblem { assigningToNotNull, nullableReturn, passingNullableToNotNullParameter, - passingNullableArgumentToNonAnnotatedParameter, - passingNullToOptional, - passingNotNullToOptional + passingNullableArgumentToNonAnnotatedParameter } 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 66893c0c0c1b..9f2c106b576a 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 @@ -57,15 +57,6 @@ public class StandardInstructionVisitor extends InstructionVisitor { return callExpression != null ? DfaPsiUtil.getElementNullability(key.getResultType(), callExpression.resolveMethod()) : null; } }; - @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") - private final FactoryMap myOptionOfNullable = new FactoryMap() { - @Nullable - @Override - protected Boolean create(MethodCallInstruction key) { - PsiCallExpression expression = key.getCallExpression(); - return expression instanceof PsiMethodCallExpression && DfaOptionalSupport.resolveOfNullable(expression) != null; - } - }; @Override public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { @@ -240,11 +231,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { forceNotNull(runner, memState, arg); } } - else if (myOptionOfNullable.get(instruction)) { - checkNotNullable(memState, arg, NullabilityProblem.passingNotNullToOptional, expr); - checkNotNullable(memState, arg, NullabilityProblem.passingNullToOptional, expr); - } - else if (requiredNullability == Nullness.UNKNOWN) { + else if (!instruction.updateOfNullable(memState, arg) && requiredNullability == Nullness.UNKNOWN) { checkNotNullable(memState, arg, NullabilityProblem.passingNullableArgumentToNonAnnotatedParameter, expr); } } @@ -384,14 +371,9 @@ public class StandardInstructionVisitor extends InstructionVisitor { protected boolean checkNotNullable(DfaMemoryState state, DfaValue value, NullabilityProblem problem, PsiElement anchor) { - if (problem == NullabilityProblem.passingNotNullToOptional) { - return !state.isNotNull(value); - } - boolean notNullable = state.checkNotNullable(value); if (notNullable && - problem != NullabilityProblem.passingNullableArgumentToNonAnnotatedParameter && - problem != NullabilityProblem.passingNullToOptional) { + problem != NullabilityProblem.passingNullableArgumentToNonAnnotatedParameter) { DfaValueFactory factory = ((DfaMemoryStateImpl)state).getFactory(); state.applyCondition(factory.getRelationFactory().createRelation(value, factory.getConstFactory().getNull(), NE, false)); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java index f7aa1da08598..3b38cefb99ca 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java @@ -46,8 +46,11 @@ public class MethodCallInstruction extends Instruction { private final List myContracts; private final MethodType myMethodType; @Nullable private final DfaValue myPrecalculatedReturnValue; + private final boolean myOfNullable; private final boolean myVarArgCall; private final Map myArgRequiredNullability; + private boolean myOnlyNullArgs = true; + private boolean myOnlyNotNullArgs = true; public enum MethodType { BOXING, UNBOXING, REGULAR_METHOD_CALL, CAST @@ -64,6 +67,7 @@ public class MethodCallInstruction extends Instruction { myPrecalculatedReturnValue = null; myTargetMethod = null; myVarArgCall = false; + myOfNullable = false; myArgRequiredNullability = Collections.emptyMap(); } @@ -91,6 +95,7 @@ public class MethodCallInstruction extends Instruction { myShouldFlushFields = !(call instanceof PsiNewExpression && myType != null && myType.getArrayDimensions() > 0) && !isPureCall(); myPrecalculatedReturnValue = precalculatedReturnValue; + myOfNullable = call instanceof PsiMethodCallExpression && DfaOptionalSupport.resolveOfNullable((PsiMethodCallExpression)call) != null; } private Map calcArgRequiredNullability(PsiSubstitutor substitutor, PsiParameter[] parameters) { @@ -191,4 +196,25 @@ public class MethodCallInstruction extends Instruction { ? "BOX" : "CALL_METHOD: " + (myCall == null ? "null" : myCall.getText()); } + + public boolean updateOfNullable(DfaMemoryState memState, DfaValue arg) { + if (!myOfNullable) return false; + + if (!memState.isNotNull(arg)) { + myOnlyNotNullArgs = false; + } + if (!memState.isNull(arg)) { + myOnlyNullArgs = false; + } + return true; + } + + public boolean isOptionalAlwaysNullProblem() { + return myOfNullable && myOnlyNullArgs; + } + + public boolean isOptionalAlwaysNotNullProblem() { + return myOfNullable && myOnlyNotNullArgs; + } + } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalOfNullable.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalOfNullable.java new file mode 100644 index 000000000000..bd5756d4a6e2 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalOfNullable.java @@ -0,0 +1,24 @@ +import java.util.List; +import java.util.Optional; + +class Test { + Optional getName(List numbers) { + return Optional.ofNullable( + numbers.isEmpty() ? + null : + numbers.get(0)); + } + + Optional getName2(List numbers) { + return Optional.ofNullable( + numbers.isEmpty() ? + "2" : + numbers.get(0)); + } + + Optional getName3() { + return Optional.ofNullable(null); + } + +} + diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java index dcffac411bfc..90f864e137d2 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java @@ -18,14 +18,20 @@ package com.intellij.codeInspection; import com.intellij.JavaTestUtil; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.DataFlowInspection; -import com.intellij.openapi.Disposable; import com.intellij.openapi.util.Disposer; +import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; /** * @author peter */ public class DataFlowInspection8Test extends LightCodeInsightFixtureTestCase { + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } @Override protected String getTestDataPath() { @@ -67,12 +73,9 @@ public class DataFlowInspection8Test extends LightCodeInsightFixtureTestCase { final NullableNotNullManager nnnManager = NullableNotNullManager.getInstance(getProject()); nnnManager.setNotNulls("foo.NotNull"); nnnManager.setNullables("foo.Nullable"); - Disposer.register(myTestRootDisposable, new Disposable() { - @Override - public void dispose() { - nnnManager.setNotNulls(); - nnnManager.setNullables(); - } + Disposer.register(myTestRootDisposable, () -> { + nnnManager.setNotNulls(); + nnnManager.setNullables(); }); } @@ -91,4 +94,8 @@ public class DataFlowInspection8Test extends LightCodeInsightFixtureTestCase { myFixture.enableInspections(inspection); myFixture.testHighlighting(true, false, true, getTestName(false) + ".java"); } + + + public void testOptionalOfNullable() { doTest(); } + } diff --git a/java/mockJDK-1.8/jre/lib/annotations.jar b/java/mockJDK-1.8/jre/lib/annotations.jar new file mode 100644 index 000000000000..b78e2de180d4 Binary files /dev/null and b/java/mockJDK-1.8/jre/lib/annotations.jar differ