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 6f5ebb2cbf79..27c83dd9ff85 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 @@ -4,14 +4,22 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool; import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.codeInspection.dataFlow.StandardMethodContract.ValueConstraint; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; +import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.Stream; + +import static com.intellij.codeInspection.dataFlow.StandardMethodContract.ParseException; +import static com.intellij.codeInspection.dataFlow.StandardMethodContract.parseContract; /** * @author peter @@ -80,22 +88,109 @@ public class ContractInspection extends AbstractBaseJavaLocalInspectionTool { public static String checkContract(PsiMethod method, String text) { List contracts; try { - contracts = StandardMethodContract.parseContract(text); + contracts = parseContract(text); } - catch (StandardMethodContract.ParseException e) { + catch (ParseException e) { return e.getMessage(); } - int paramCount = method.getParameterList().getParametersCount(); - for (int i = 0; i < contracts.size(); i++) { - StandardMethodContract contract = contracts.get(i); + PsiParameter[] parameters = method.getParameterList().getParameters(); + int paramCount = parameters.length; + List possibleConditions = Collections.singletonList(new Conditions(paramCount)); + for (StandardMethodContract contract : contracts) { if (contract.getParameterCount() != paramCount) { - return "Method takes " + paramCount + " parameters, while contract clause number " + (i + 1) + " expects " + contract.getParameterCount(); + return "Method takes " + paramCount + " parameters, " + + "while contract clause '" + contract + "' expects " + contract.getParameterCount(); + } + for (int i = 0; i < parameters.length; i++) { + ValueConstraint constraint = contract.getParameterConstraint(i); + PsiType type = parameters[i].getType(); + switch (constraint) { + case ANY_VALUE: + break; + case NULL_VALUE: + case NOT_NULL_VALUE: + if (type instanceof PsiPrimitiveType) { + return "Contract clause '"+contract+"': parameter #"+(i+1)+" has primitive type '"+type.getPresentableText()+"'"; + } + break; + case TRUE_VALUE: + case FALSE_VALUE: + if (!PsiType.BOOLEAN.equals(type) && !type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) { + return "Contract clause '"+contract+"': parameter #"+(i+1)+" has '"+type.getPresentableText()+"' type (expected boolean)"; + } + break; + } } String problem = contract.getReturnValue().getMethodCompatibilityProblem(method); if (problem != null) { return problem; } + if (possibleConditions != null) { + if (possibleConditions.isEmpty()) { + return "Contract clause '" + contract + "' is unreachable: previous contracts cover all possible cases"; + } + if (StreamEx.of(possibleConditions).allMatch(c -> c.fitContract(contract) == null)) { + return "Contract clause '" + contract + "' is never satisfied as its conditions are covered by previous contracts"; + } + possibleConditions = StreamEx.of(possibleConditions).flatMap(c -> c.misfitContract(contract)) + .limit(DataFlowRunner.MAX_STATES_PER_BRANCH).toList(); + if (possibleConditions.size() >= DataFlowRunner.MAX_STATES_PER_BRANCH) { + possibleConditions = null; + } + } } return null; } + + private static final class Conditions { + private final List myParameters; + + Conditions(int paramCount) { + myParameters = StreamEx.constant(ValueConstraint.ANY_VALUE, paramCount).toList(); + } + + private Conditions(List parameters) { + myParameters = parameters; + } + + @Nullable + Conditions fitContract(StandardMethodContract contract) { + List result = new ArrayList<>(myParameters); + assert contract.getParameterCount() == result.size(); + for (int i = 0; i < result.size(); i++) { + ValueConstraint condition = result.get(i); + ValueConstraint constraint = contract.getParameterConstraint(i); + if (condition == constraint || condition == ValueConstraint.ANY_VALUE) { + result.set(i, constraint); + } else if (constraint == ValueConstraint.ANY_VALUE) { + result.set(i, condition); + } + else { + return null; + } + } + return new Conditions(result); + } + + @NotNull + Stream misfitContract(StandardMethodContract contract) { + assert contract.getParameterCount() == myParameters.size(); + List constraints = contract.getConstraints(); + List template = StreamEx.constant(ValueConstraint.ANY_VALUE, myParameters.size()).toList(); + List antiContracts = new ArrayList<>(); + for (int i = 0; i < constraints.size(); i++) { + ValueConstraint constraint = constraints.get(i); + if (constraint == ValueConstraint.ANY_VALUE) continue; + template.set(i, constraint.negate()); + antiContracts.add(new StandardMethodContract(template.toArray(new ValueConstraint[0]), ContractReturnValue.returnAny())); + template.set(i, constraint); + } + return StreamEx.of(antiContracts).map(this::fitContract).nonNull(); + } + + @Override + public String toString() { + return myParameters.toString(); + } + } } diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/ConditionsConflict.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/ConditionsConflict.java new file mode 100644 index 000000000000..39de8a0d4e4a --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/ConditionsConflict.java @@ -0,0 +1,22 @@ +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.Nullable; + +class Foo { + @Contract("_ -> !null; null -> null") + public native String nonTrivialAfterTrivial(String x); + + @Contract("!null -> !null; !null -> null") + public native String repeating(String x); + + @Contract("true, false, _ -> !null; true, true, _ -> null; true, _, _ -> fail") + public native String booleanProblem(boolean x, boolean y, String z); + + @Contract("true, false, _ -> !null; true, true, _ -> null; false, _, _ -> fail") + public native String booleanOk(boolean x, boolean y, String z); + + @Contract("true, false, _ -> !null; false, _, _ -> fail; true, true, _ -> null") + public native String booleanOk2(boolean x, boolean y, String z); + + @Contract("null, null, null, null, null -> null; null, null, !null, null, _ -> null; null, null, null, null, !null -> !null; null, null, _, null, !null -> fail") + public native String test(String a, String b, String c, String d, String e); +} diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java index 6e21ab679aa5..85c69503123e 100644 --- a/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java @@ -5,7 +5,7 @@ class Foo { @Contract("a") void malformedContract() {} - @Contract("null -> _") + @Contract("null -> _") void wrongParameterCount(Object a, boolean b) {} @Contract(pure=true) @@ -48,4 +48,10 @@ class Foo { @Contract("->foo") public native void invalidReturn(); + + @Contract("true -> fail") + public native void invalidType(String s); + + @Contract("null -> fail") + public native void invalidType(int s); } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java index 684d3b090db0..2df2e7069b62 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java @@ -52,4 +52,5 @@ public class ContractCheckTest extends LightCodeInsightFixtureTestCase { public void testCallingNotNullMethod() { doTest(); } public void testMutationSignatureProblems() { doTest(); } public void testNewThisParam() { doTest(); } + public void testConditionsConflict() { doTest(); } }