diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index d68a3e4addea..99dc29ecbb96 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -390,4 +390,20 @@ inspection.string.tokenizer.delimiter.display.name=Duplicated delimiters in java inspection.anonymous.has.lambda.alternative.display.name=Anonymous type has shorter lambda alternative inspection.java.8.list.sort.display.name=Collections.sort() can be replaced with List.sort() inspection.class.has.no.to.string.method.display.name=Class does not override 'toString()' method -inspection.field.not.used.in.to.string.display.name=Field not used in 'toString()' method \ No newline at end of file +inspection.field.not.used.in.to.string.display.name=Field not used in 'toString()' method +inspection.contract.checker.clause.syntax=A contract clause must be in form arg1, ..., argN -> return-value +inspection.contract.checker.unknown.return.value=Return value should be one of: {0}. Found: {1} +inspection.contract.checker.unknown.constraint=Constraint should be one of: {0}. Found: {1} +inspection.contract.checker.empty.constraint=Constraint should not be empty +inspection.contract.checker.unreachable.contract.clause=Contract clause ''{0}'' is unreachable: previous contracts cover all possible cases +inspection.contract.checker.contract.clause.never.satisfied=Contract clause ''{0}'' is never satisfied as its conditions are covered by previous contracts +inspection.contract.checker.pure.method.mutation.contract=Pure method cannot have mutation contract +inspection.contract.checker.parameter.count.mismatch=Method takes {0} parameters, while contract clause ''{1}'' expects {2} +inspection.contract.checker.primitive.parameter.nullability=Parameter ''{0}'' has primitive type ''{1}'', so ''{2}'' is not applicable +inspection.contract.checker.inferred.notnull.parameter.nullability=Parameter ''{0}'' is inferred to be not-null, so ''{1}'' is not applicable +inspection.contract.checker.notnull.parameter.nullability=Parameter ''{0}'' is annotated as not-null, so ''{1}'' is not applicable +inspection.contract.checker.boolean.condition.for.nonboolean.parameter=Parameter ''{0}'' has ''{1}'' type (expected boolean) +inspection.contract.checker.contract.violated=Contract clause ''{0}'' is violated +inspection.contract.checker.no.exception.thrown=Contract clause ''{0}'' is violated: no exception is thrown +inspection.contract.checker.method.always.fails.trivial=Return value of clause ''{0}'' could be replaced with ''fail'' as method always fails +inspection.contract.checker.method.always.fails.nontrivial=Return value of clause ''{0}'' could be replaced with ''fail'' as method always fails in this case \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java index 0bcf56eaf5a1..5c6700573148 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java @@ -10,6 +10,8 @@ import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.codeInspection.dataFlow.value.RelationType; +import com.intellij.codeInspection.util.InspectionMessage; +import com.intellij.java.analysis.JavaAnalysisBundle; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; import com.intellij.util.containers.ContainerUtil; @@ -85,11 +87,11 @@ class ContractChecker { return super.visitControlTransfer(instruction, runner, state); } - private Map getErrors() { - HashMap errors = new HashMap<>(); + private Map getErrors() { + HashMap errors = new HashMap<>(); for (PsiElement element : myViolations) { if (!myNonViolations.contains(element)) { - errors.put(element, "Contract clause '" + myContract + "' is violated"); + errors.put(element, JavaAnalysisBundle.message("inspection.contract.checker.contract.violated", myContract)); } } @@ -97,14 +99,18 @@ class ContractChecker { if (myOwnContract && !myMayReturnNormally && !(PsiUtil.canBeOverridden(myMethod) && ControlFlowUtils.methodAlwaysThrowsException(myMethod))) { for (PsiElement element : myFailures) { - errors.put(element, "Return value of clause '" + myContract + "' could be replaced with 'fail' as method always fails"+ - (myContract.isTrivial() ? "" : " in this case")); + if (myContract.isTrivial()) { + errors.put(element, JavaAnalysisBundle.message("inspection.contract.checker.method.always.fails.trivial", myContract)); + } + else { + errors.put(element, JavaAnalysisBundle.message("inspection.contract.checker.method.always.fails.nontrivial", myContract)); + } } } } else if (myFailures.isEmpty() && errors.isEmpty() && myMayReturnNormally) { PsiIdentifier nameIdentifier = myMethod.getNameIdentifier(); errors.put(nameIdentifier != null ? nameIdentifier : myMethod, - "Contract clause '" + myContract + "' is violated: no exception is thrown"); + JavaAnalysisBundle.message("inspection.contract.checker.no.exception.thrown", myContract)); } return errors; @@ -116,7 +122,7 @@ class ContractChecker { } } - static Map checkContractClause(PsiMethod method, StandardMethodContract contract, boolean ownContract) { + static Map checkContractClause(PsiMethod method, StandardMethodContract contract, boolean ownContract) { PsiCodeBlock body = method.getBody(); if (body == null) return Collections.emptyMap(); 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 a9c655974514..c99845bbb6b9 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 @@ -2,9 +2,13 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.codeInsight.Nullability; +import com.intellij.codeInsight.NullabilityAnnotationInfo; +import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool; import com.intellij.codeInspection.ProblemsHolder; import com.intellij.codeInspection.dataFlow.StandardMethodContract.ValueConstraint; +import com.intellij.java.analysis.JavaAnalysisBundle; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; @@ -41,10 +45,7 @@ public class ContractInspection extends AbstractBaseJavaLocalInspectionTool { boolean ownContract = annotation.getOwner() == method.getModifierList(); for (StandardMethodContract contract : JavaMethodContractUtil.getMethodContracts(method)) { Map errors = ContractChecker.checkContractClause(method, contract, ownContract); - for (Map.Entry entry : errors.entrySet()) { - PsiElement element = entry.getKey(); - holder.registerProblem(element, entry.getValue()); - } + errors.forEach(holder::registerProblem); } } @@ -78,7 +79,7 @@ public class ContractInspection extends AbstractBaseJavaLocalInspectionTool { boolean pure = Boolean.TRUE.equals(AnnotationUtil.getBooleanAttributeValue(annotation, "pure")); String error; if (pure) { - error = "Pure method cannot have mutation contract"; + error = JavaAnalysisBundle.message("inspection.contract.checker.pure.method.mutation.contract"); } else { error = MutationSignature.checkSignature(mutationContract, method); } @@ -108,29 +109,48 @@ public class ContractInspection extends AbstractBaseJavaLocalInspectionTool { for (int clauseIndex = 0; clauseIndex < contracts.size(); clauseIndex++) { StandardMethodContract contract = contracts.get(clauseIndex); if (contract.getParameterCount() != paramCount) { - return ParseException.forClause("Method takes " + paramCount + " parameters, " + - "while contract clause '" + contract + "' expects " + contract.getParameterCount(), text, - clauseIndex); + String message = JavaAnalysisBundle + .message("inspection.contract.checker.parameter.count.mismatch", paramCount, contract, contract.getParameterCount()); + return ParseException.forClause(message, text, clauseIndex); } for (int i = 0; i < parameters.length; i++) { ValueConstraint constraint = contract.getParameterConstraint(i); - PsiType type = parameters[i].getType(); + PsiParameter parameter = parameters[i]; + PsiType type = parameter.getType(); switch (constraint) { case ANY_VALUE: break; case NULL_VALUE: case NOT_NULL_VALUE: if (type instanceof PsiPrimitiveType) { - String message = - "Contract clause '" + contract + "': parameter #" + (i + 1) + " has primitive type '" + type.getPresentableText() + "'"; + String message = JavaAnalysisBundle.message("inspection.contract.checker.primitive.parameter.nullability", + parameter.getName(), type.getPresentableText(), constraint); return ParseException.forConstraint(message, text, clauseIndex, i); + } else { + NullabilityAnnotationInfo info = + NullableNotNullManager.getInstance(method.getProject()).findEffectiveNullabilityInfo(parameter); + if (info != null && info.getNullability() == Nullability.NOT_NULL) { + String message; + if (info.isInferred()) { + if (constraint == ValueConstraint.NULL_VALUE && contract.getReturnValue().isFail()) { + // null -> fail is ok if not-null was inferred + break; + } + message = JavaAnalysisBundle.message("inspection.contract.checker.inferred.notnull.parameter.nullability", + parameter.getName(), constraint); + } else { + message = JavaAnalysisBundle.message("inspection.contract.checker.notnull.parameter.nullability", + parameter.getName(), constraint); + } + return ParseException.forConstraint(message, text, clauseIndex, i); + } } break; case TRUE_VALUE: case FALSE_VALUE: if (!PsiType.BOOLEAN.equals(type) && !type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) { - String message = "Contract clause '" + contract + "': parameter #" + (i + 1) + " has '" + - type.getPresentableText() + "' type (expected boolean)"; + String message = JavaAnalysisBundle.message("inspection.contract.checker.boolean.condition.for.nonboolean.parameter", + parameter.getName(), type.getPresentableText()); return ParseException.forConstraint(message, text, clauseIndex, i); } break; @@ -143,11 +163,11 @@ public class ContractInspection extends AbstractBaseJavaLocalInspectionTool { if (possibleContracts != null) { if (possibleContracts.isEmpty()) { return ParseException - .forClause("Contract clause '" + contract + "' is unreachable: previous contracts cover all possible cases", text, clauseIndex); + .forClause(JavaAnalysisBundle.message("inspection.contract.checker.unreachable.contract.clause", contract), text, clauseIndex); } if (StreamEx.of(possibleContracts).allMatch(c -> c.intersect(contract) == null)) { return ParseException.forClause( - "Contract clause '" + contract + "' is never satisfied as its conditions are covered by previous contracts", text, clauseIndex); + JavaAnalysisBundle.message("inspection.contract.checker.contract.clause.never.satisfied", contract), text, clauseIndex); } possibleContracts = StreamEx.of(possibleContracts).flatMap(c -> c.excludeContract(contract)) .limit(DataFlowRunner.MAX_STATES_PER_BRANCH).toList(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardMethodContract.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardMethodContract.java index 15e04efa93f1..e9ea8558af54 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardMethodContract.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardMethodContract.java @@ -4,6 +4,8 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.codeInspection.dataFlow.value.RelationType; +import com.intellij.codeInspection.util.InspectionMessage; +import com.intellij.java.analysis.JavaAnalysisBundle; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; import com.intellij.util.containers.ContainerUtil; @@ -227,7 +229,7 @@ public final class StandardMethodContract extends MethodContract { String arrow = "->"; int arrowIndex = clause.indexOf(arrow); if (arrowIndex < 0) { - throw ParseException.forClause("A contract clause must be in form arg1, ..., argN -> return-value", text, clauseIndex); + throw ParseException.forClause(JavaAnalysisBundle.message("inspection.contract.checker.clause.syntax"), text, clauseIndex); } String beforeArrow = clause.substring(0, arrowIndex); @@ -245,20 +247,21 @@ public final class StandardMethodContract extends MethodContract { String returnValueString = clause.substring(arrowIndex + arrow.length()); ContractReturnValue returnValue = ContractReturnValue.valueOf(returnValueString); if (returnValue == null) { - throw ParseException.forReturnValue( - "Return value should be one of: null, !null, true, false, this, new, paramN, fail, _. Found: " + returnValueString, - text, clauseIndex); + String possibleValues = "null, !null, true, false, this, new, paramN, fail, _"; + String message = JavaAnalysisBundle.message("inspection.contract.checker.unknown.return.value", possibleValues, returnValueString); + throw ParseException.forReturnValue(message, text, clauseIndex); } return new StandardMethodContract(args, returnValue); } private static ValueConstraint parseConstraint(String name, String text, int clauseIndex, int constraintIndex) throws ParseException { - if (StringUtil.isEmpty(name)) throw new ParseException("Constraint should not be empty"); + if (StringUtil.isEmpty(name)) throw new ParseException(JavaAnalysisBundle.message("inspection.contract.checker.empty.constraint")); for (ValueConstraint constraint : ValueConstraint.values()) { if (constraint.toString().equals(name)) return constraint; } - throw ParseException - .forConstraint("Constraint should be one of: null, !null, true, false, _. Found: " + name, text, clauseIndex, constraintIndex); + String allowedClause = StreamEx.of(ValueConstraint.values()).joining(", "); + String message = JavaAnalysisBundle.message("inspection.contract.checker.unknown.constraint", allowedClause, name); + throw ParseException.forConstraint(message, text, clauseIndex, constraintIndex); } public enum ValueConstraint { @@ -345,11 +348,11 @@ public final class StandardMethodContract extends MethodContract { public static class ParseException extends Exception { private final @Nullable TextRange myRange; - ParseException(String message) { + ParseException(@InspectionMessage String message) { this(message, null); } - ParseException(String message, @Nullable TextRange range) { + ParseException(@InspectionMessage String message, @Nullable TextRange range) { super(message); myRange = range != null && range.isEmpty() ? null : range; } @@ -358,7 +361,7 @@ public final class StandardMethodContract extends MethodContract { return myRange; } - static ParseException forConstraint(String message, String text, int clauseNumber, int constraintNumber) { + static ParseException forConstraint(@InspectionMessage String message, String text, int clauseNumber, int constraintNumber) { TextRange range = findClauseRange(text, clauseNumber); if (range == null) { return new ParseException(message); @@ -384,7 +387,7 @@ public final class StandardMethodContract extends MethodContract { return new ParseException(message, new TextRange(start, end)); } - static ParseException forReturnValue(String message, String text, int clauseNumber) { + static ParseException forReturnValue(@InspectionMessage String message, String text, int clauseNumber) { TextRange range = findClauseRange(text, clauseNumber); if (range == null) { return new ParseException(message); @@ -401,7 +404,7 @@ public final class StandardMethodContract extends MethodContract { return new ParseException(message, new TextRange(index, range.getEndOffset())); } - static ParseException forClause(String message, String text, int clauseNumber) { + static ParseException forClause(@InspectionMessage String message, String text, int clauseNumber) { TextRange range = findClauseRange(text, clauseNumber); return range == null ? new ParseException(message) : new ParseException(message, range); } diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/DelegationToInstanceMethod.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/DelegationToInstanceMethod.java index af1679dd3599..00037a823519 100644 --- a/java/java-tests/testData/inspection/dataFlow/contractCheck/DelegationToInstanceMethod.java +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/DelegationToInstanceMethod.java @@ -3,7 +3,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; class Foo { - @Contract("!null,true->!null") + @Contract("!null,true->!null") String delegationToInstance(@NotNull Foo f, boolean createIfNeeded) { return f.getString(createIfNeeded); } @Contract("true->!null") diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/FailDelegation.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/FailDelegation.java index 703cb0eb3215..651c7d733da4 100644 --- a/java/java-tests/testData/inspection/dataFlow/contractCheck/FailDelegation.java +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/FailDelegation.java @@ -3,12 +3,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; class Foo { - @Contract("!null,true->!null") + @Contract("!null,true->!null") String delegationToInstance(@NotNull Foo f, boolean createIfNeeded) { return f.getString(createIfNeeded); } - @Contract("!null,false->!null") + @Contract("!null,false->!null") String delegationToInstanceOk(@NotNull Foo f, boolean createIfNeeded) { return f.getString(createIfNeeded); } diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/InferredNotNull.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/InferredNotNull.java new file mode 100644 index 000000000000..67c206944d04 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/InferredNotNull.java @@ -0,0 +1,9 @@ +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; + +class Foo { + @Contract("null -> true") + public boolean test(Boolean foo) { + return foo.booleanValue(); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java index 47b341cb9273..c97c81d2d1fd 100644 --- a/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java @@ -49,9 +49,9 @@ class Foo { @Contract("->foo") public native void invalidReturn(); - @Contract("true -> fail") + @Contract("true -> fail") public native void invalidType(String s); - @Contract("null -> fail") + @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 6a5390adc56a..b380b13335c9 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java @@ -58,4 +58,5 @@ public class ContractCheckTest extends LightJavaCodeInsightFixtureTestCase { public void testIsInstance() { doTest(); } public void testObjectBoolean() { doTest(); } public void testParamUncheckedCast() { doTest(); } + public void testInferredNotNull() { doTest(); } }