Contract inspection improved:

1. IDEA-236142 warn about inconsistent Contract/NotNull annotations
2. Localization of messages
3. Better messages

GitOrigin-RevId: 71add12d928cbc7234536c3746e9a97ec8c4c53f

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
+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

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<PsiElement, String> getErrors() {
-      HashMap<PsiElement, String> errors = new HashMap<>();
+    private Map<PsiElement, @InspectionMessage String> getErrors() {
+      HashMap<PsiElement, @InspectionMessage String> 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<PsiElement, String> checkContractClause(PsiMethod method, StandardMethodContract contract, boolean ownContract) {
+  static Map<PsiElement, @InspectionMessage String> checkContractClause(PsiMethod method, StandardMethodContract contract, boolean ownContract) {
     PsiCodeBlock body = method.getBody();
     if (body == null) return Collections.emptyMap();
 

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<PsiElement, String> errors = ContractChecker.checkContractClause(method, contract, ownContract);
-          for (Map.Entry<PsiElement, String> 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();

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);
     }

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("<warning descr="Parameter 'f' is annotated as not-null, so '!null' is not applicable">!null</warning>,true->!null")
   String delegationToInstance(@NotNull Foo f, boolean createIfNeeded) { return f.getString(createIfNeeded); }
 
   @Contract("true->!null")

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("<warning descr="Parameter 'f' is annotated as not-null, so '!null' is not applicable">!null</warning>,true->!null")
   String delegationToInstance(@NotNull Foo f, boolean createIfNeeded) {
     return <warning descr="Return value of clause '!null, true -> !null' could be replaced with 'fail' as method always fails in this case">f.getString(createIfNeeded)</warning>;
   }
 
-  @Contract("!null,false->!null")
+  @Contract("<warning descr="Parameter 'f' is annotated as not-null, so '!null' is not applicable">!null</warning>,false->!null")
   String delegationToInstanceOk(@NotNull Foo f, boolean createIfNeeded) {
     return f.getString(createIfNeeded);
   }

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("<warning descr="Parameter 'foo' is inferred to be not-null, so 'null' is not applicable">null</warning> -> true")
+  public boolean test(Boolean foo) {
+    return foo.booleanValue();
+  }
+}

java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java

@@ -49,9 +49,9 @@ class Foo {
   @Contract("-><warning descr="Return value should be one of: null, !null, true, false, this, new, paramN, fail, _. Found: foo">foo</warning>")
   public native void invalidReturn();
 
-  @Contract("<warning descr="Contract clause 'true -> fail': parameter #1 has 'String' type (expected boolean)">true</warning> -> fail")
+  @Contract("<warning descr="Parameter 's' has 'String' type (expected boolean)">true</warning> -> fail")
   public native void invalidType(String s);
 
-  @Contract("<warning descr="Contract clause 'null -> fail': parameter #1 has primitive type 'int'">null</warning> -> fail")
+  @Contract("<warning descr="Parameter 's' has primitive type 'int', so 'null' is not applicable">null</warning> -> fail")
   public native void invalidType(int s);
 }

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(); }
 }