diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java index 69206a74251a..1bdd1b256bd0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java @@ -95,6 +95,9 @@ public class ContractInspection extends BaseJavaBatchLocalInspectionTool { if (returnType != null && !InferenceFromSourceUtil.isReturnTypeCompatible(returnType, contract.returnValue)) { return "Method returns " + returnType.getPresentableText() + " but the contract specifies " + contract.returnValue; } + if (method.isConstructor() && contract.returnValue != MethodContract.ValueConstraint.THROW_EXCEPTION) { + return "Invalid contract return value for a constructor: " + contract.returnValue; + } } return null; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 4d72212249f3..2a356bd66862 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -1586,7 +1586,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiMethod constructor = pushConstructorArguments(expression); addConditionalRuntimeThrow(); - addInstruction(new MethodCallInstruction(expression, null, Collections.emptyList())); + addInstruction(new MethodCallInstruction(expression, null, constructor == null ? Collections.emptyList() : getMethodContracts(constructor))); if (!myCatchStack.isEmpty()) { addMethodThrows(constructor, expression); @@ -1597,6 +1597,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } + @Nullable private PsiMethod pushConstructorArguments(PsiConstructorCall call) { PsiExpressionList args = call.getArgumentList(); PsiMethod ctr = call.resolveConstructor(); 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 4da776776b0f..548e0dea874a 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 @@ -48,12 +48,10 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.refactoring.extractMethod.ExtractMethodUtil; -import com.intellij.util.ArrayUtil; -import com.intellij.util.ArrayUtilRt; -import com.intellij.util.IncorrectOperationException; -import com.intellij.util.SmartList; +import com.intellij.util.*; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; +import one.util.streamex.StreamEx; import org.jdom.Element; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -319,6 +317,8 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { } } + reportAlwaysFailingCalls(holder, visitor, reportedAnchors); + reportConstantPushes(runner, holder, visitor, reportedAnchors); reportNullableArguments(visitor, holder, reportedAnchors); @@ -337,6 +337,35 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { } } + private static void reportAlwaysFailingCalls(ProblemsHolder holder, + DataFlowInstructionVisitor visitor, + HashSet reportedAnchors) { + for (PsiCall call : visitor.getAlwaysFailingCalls()) { + PsiMethod method = call.resolveMethod(); + if (method != null && reportedAnchors.add(call)) { + holder.registerProblem(getElementToHighlight(call), "The call to #ref always fails, according to its method contracts"); + } + } + } + + @NotNull private static PsiElement getElementToHighlight(@NotNull PsiCall call) { + PsiJavaCodeReferenceElement ref; + if (call instanceof PsiNewExpression) { + ref = ((PsiNewExpression)call).getClassReference(); + } + else if (call instanceof PsiMethodCallExpression) { + ref = ((PsiMethodCallExpression)call).getMethodExpression(); + } + else { + return call; + } + if (ref != null) { + PsiElement name = ref.getReferenceNameElement(); + return name != null ? name : ref; + } + return call; + } + private void reportConstantPushes(StandardDataFlowRunner runner, ProblemsHolder holder, DataFlowInstructionVisitor visitor, @@ -465,8 +494,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { List fixes = createNPEFixes(methodExpression.getQualifierExpression(), callExpression, onTheFly); ContainerUtil.addIfNotNull(fixes, ReplaceWithObjectsEqualsFix.createFix(callExpression, methodExpression)); - PsiElement toHighlight = methodExpression.getReferenceNameElement(); - if (toHighlight == null) toHighlight = methodExpression; + PsiElement toHighlight = getElementToHighlight(callExpression); holder.registerProblem(toHighlight, InspectionsBundle.message("dataflow.message.npe.method.invocation"), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); @@ -855,6 +883,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { private final MultiMap myProblems = new MultiMap<>(); private final Map, StateInfo> myStateInfos = ContainerUtil.newHashMap(); private final Set myCCEInstructions = ContainerUtil.newHashSet(); + private final Map myFailingCalls = new HashMap<>(); @Override protected void onInstructionProducesCCE(TypeCastInstruction instruction) { @@ -871,6 +900,36 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { }); } + Collection getAlwaysFailingCalls() { + return StreamEx.of(myFailingCalls.keySet()).filter(this::isAlwaysFailing).map(MethodCallInstruction::getCallExpression).toList(); + } + + @Override + public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, + DataFlowRunner runner, + DfaMemoryState memState) { + DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState); + if (hasNonTrivialFailingContracts(instruction)) { + boolean allFail = Arrays.stream(states).allMatch(s -> s.getMemoryState().peek() == runner.getFactory().getConstFactory().getContractFail()); + myFailingCalls.put(instruction, allFail && isAlwaysFailing(instruction)); + } + return states; + } + + private static boolean hasNonTrivialFailingContracts(MethodCallInstruction instruction) { + List contracts = instruction.getContracts(); + return !contracts.isEmpty() && contracts.stream().allMatch(DataFlowInstructionVisitor::isNonTrivialFailingContract); + } + + private static boolean isNonTrivialFailingContract(MethodContract contract) { + return contract.returnValue == MethodContract.ValueConstraint.THROW_EXCEPTION && + Arrays.stream(contract.arguments).anyMatch(v -> v != MethodContract.ValueConstraint.ANY_VALUE); + } + + private boolean isAlwaysFailing(MethodCallInstruction instruction) { + return !Boolean.FALSE.equals(myFailingCalls.get(instruction)); + } + @Override protected boolean checkNotNullable(DfaMemoryState state, DfaValue value, NullabilityProblem problem, PsiElement anchor) { boolean ok = super.checkNotNullable(state, value, problem, anchor); diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/CheckConstructorContracts.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/CheckConstructorContracts.java new file mode 100644 index 000000000000..a10d634a9553 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/CheckConstructorContracts.java @@ -0,0 +1,15 @@ +import org.jetbrains.annotations.Contract; + +class Zoo { + @Contract("null->null" ) + Zoo(Object o) {} + + @Contract("_->fail" ) + Zoo(int o) {} + + @Contract("true->fail" ) + Zoo(boolean o) { + if (o) throw new RuntimeException(); + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ContractConstructor.java b/java/java-tests/testData/inspection/dataFlow/fixture/ContractConstructor.java new file mode 100644 index 000000000000..c29ed07070d1 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ContractConstructor.java @@ -0,0 +1,17 @@ +import org.jetbrains.annotations.*; + +class CandidateInfo { + @Contract("null, true, _ -> fail") + private CandidateInfo(@Nullable Object candidate, boolean applicable, boolean fullySubstituted) { + assert !applicable || candidate != null; + } + + static void test() { + new CandidateInfo(null, true, false); + new CandidateInfo(null, true, true); + + new CandidateInfo(null, false, true); + new CandidateInfo(new Object(), true, true); + new CandidateInfo(new Object(), false, true); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java b/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java index 43c15c19eb70..e25c89ab650f 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java @@ -8,7 +8,7 @@ class Test { String a = notNull(getNullable()); @NotNull - String b = notNull(null); + String b = notNull(null); } @Nullable diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ContractCheckTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/ContractCheckTest.java index e220967e9a3c..8d5eccab914c 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/ContractCheckTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ContractCheckTest.java @@ -44,6 +44,7 @@ public class ContractCheckTest extends LightCodeInsightFixtureTestCase { public void testVarargInferred() { doTest(); } public void testDoubleParameter() { doTest(); } public void testReturnPrimitiveArray() { doTest(); } + public void testCheckConstructorContracts() { doTest(); } public void testPassingVarargsToDelegate() { doTest(); } } diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index a661955992af..7128b952ae18 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -237,6 +237,7 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testContractPreservesUnknownMethodNullability() { doTest(); } public void testContractSeveralClauses() { doTest(); } public void testContractVarargs() { doTest(); } + public void testContractConstructor() { doTest(); } public void testBoxingImpliesNotNull() { doTest(); } public void testLargeIntegersAreNotEqualWhenBoxed() { doTest(); } diff --git a/java/mockJDK-1.7/jre/lib/annotations.jar b/java/mockJDK-1.7/jre/lib/annotations.jar index b78e2de180d4..cfbf21525bae 100644 Binary files a/java/mockJDK-1.7/jre/lib/annotations.jar and b/java/mockJDK-1.7/jre/lib/annotations.jar differ