[analyzer] Refine the diagnostics in the nullability checker to differentiate between nil and null

[zaks.anna]

Summary

This is a big deal for ObjC, where nullability annotations are extensively used. I've also changed "Null" -> "null" and removed "is" as this is the pattern that Sema is using.

[dcoughlin]

I think in the 'null' case it might be better to keep it as "Null passed" or even "Null value passed". This is different than the 'nil' case because the diagnostic is not referring to a literal.

[zaks.anna]

Looks like Sema uses "null" not only when referring to literals (see below). Also, if we were referring to literals, we would use single quotes, no?

I suggest keeping as is for consistency with the wording that uses nil. I do not see much difference between the two cases..

TestObject * _Nonnull returnsNilObjCInstanceDirectly() {
   // The first warning is from Sema. The second is from the static analyzer.
   return nil; // expected-warning {{null returned from function that requires a non-null return value}}
               // expected-warning@-1 {{nil returned from a function that is expected to return a non-null value}}
}

[zaks.anna]

Updated "null"-> "Null" as per Devin's suggestion.

[xazax.hun]

LGTM!

[dcoughlin]

Looks good.

While you are here, you might consider changing: the checkBind() diagnostic to match the other diagnostics:

"Null assigned to a pointer which is expected to have non-null value" --> "Null assigned to a pointer which is expected to have *a* non-null value"

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

if (Filter.CheckNullReturnedFromNonnull &&
       C.getLocationContext()->inTopFrame()) {
     static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
     ExplodedNode *N = C.generateErrorNode(State, &Tag);
     if (!N)
       return;
 
     SmallString<256> SBuf;
     llvm::raw_svector_ostream OS(SBuf);
-    OS << "Null is returned from a " << C.getDeclDescription(D) <<
+    OS << (RetExpr->getType()->isObjCObjectPointerType() ? "nil" : "null");
+    OS << " returned from a " << C.getDeclDescription(D) <<
           " that is expected to return a non-null value";
-
     reportBugIfInvariantHolds(OS.str(),
                               ErrorKind::NilReturnedToNonnull, N, nullptr, C,
                               RetExpr);
     return;
   }
 
   // If null was returned from a non-null function, mark the nullability
   // invariant as violated even if the diagnostic was suppressed.
for (const ParmVarDecl *Param : Call.parameters()) {
 
     if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull &&
         ArgExprTypeLevelNullability != Nullability::Nonnull &&
         RequiredNullability == Nullability::Nonnull &&
         isDiagnosableCall(Call)) {
       ExplodedNode *N = C.generateErrorNode(State);
       if (!N)
         return;
+
       SmallString<256> SBuf;
       llvm::raw_svector_ostream OS(SBuf);
-      OS << "Null passed to a callee that requires a non-null " << ParamIdx
+      OS << (Param->getType()->isObjCObjectPointerType() ? "nil" : "null");
+      OS << " passed to a callee that requires a non-null " << ParamIdx
          << llvm::getOrdinalSuffix(ParamIdx) << " parameter";
       reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull, N,
                                 nullptr, C,
                                 ArgExpr, /*SuppressPath=*/false);
       return;
     }
 
     const MemRegion *Region = getTrackRegion(*ArgSVal);
if (Filter.CheckNullPassedToNonnull &&
     if (!N)
       return;
 
 
     const Stmt *ValueStmt = S;
     if (ValueExpr)
       ValueStmt = ValueExpr;
 
-    reportBugIfInvariantHolds("Null is assigned to a pointer which is "
-                              "expected to have non-null value",
+    SmallString<256> SBuf;
+    llvm::raw_svector_ostream OS(SBuf);
+    OS << (LocType->isObjCObjectPointerType() ? "nil" : "null");
+    OS << " assigned to a pointer which is expected to have non-null value";
+    reportBugIfInvariantHolds(OS.str(),
                               ErrorKind::NilAssignedToNonnull, N, nullptr, C,
                               ValueStmt);
     return;
   }
 
   // If null was returned from a non-null function, mark the nullability
   // invariant as violated even if the diagnostic was suppressed.
   if (NullAssignedToNonNull) {

test/Analysis/nullability-no-arc.mm

 __attribute__((objc_root_class))
 @interface
 NSObject<NSObject>
 @end
 
 @interface TestObject : NSObject
 @end
 
-TestObject * _Nonnull returnsNilObjCInstanceIndirectly() {
-  TestObject *local = 0;
-  return local; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+TestObject *_Nonnull returnsNilObjCInstanceIndirectly() {
+  TestObject *local = nil;
+  return local; // expected-warning {{nil returned from a function that is expected to return a non-null value}}
 }
 
 TestObject * _Nonnull returnsNilObjCInstanceIndirectlyWithSupressingCast() {
-  TestObject *local = 0;
+  TestObject *local = nil;
   return (TestObject * _Nonnull)local; // no-warning
 }
 
 TestObject * _Nonnull returnsNilObjCInstanceDirectly() {
   // The first warning is from Sema. The second is from the static analyzer.
   return nil; // expected-warning {{null returned from function that requires a non-null return value}}
-              // expected-warning@-1 {{Null is returned from a function that is expected to return a non-null value}}
+              // expected-warning@-1 {{nil returned from a function that is expected to return a non-null value}}
 }
 
 TestObject * _Nonnull returnsNilObjCInstanceDirectlyWithSuppressingCast() {
   return (TestObject * _Nonnull)nil; // no-warning
 }
 
 void testObjCNonARCNoInitialization(TestObject * _Nonnull p) {
   TestObject * _Nonnull implicitlyZeroInitialized; // no-warning
   implicitlyZeroInitialized = p;
 }
 
 void testObjCNonARCExplicitZeroInitialization() {
-  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}}
 }
 
 @interface ClassWithInitializers : NSObject
 @end
 
 @implementation ClassWithInitializers
 - (instancetype _Nonnull)initWithNonnullReturnAndSelfCheckingIdiom {
   // This defensive check is a common-enough idiom that we don't want

test/Analysis/nullability.mm

if (getRandom()) {
       Dummy &r = *p;
     } else {
       int b = p->val;
     }
   }
   Dummy *q = 0;
   if (getRandom()) {
     takesNullable(q);
-    takesNonnull(q); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
+    takesNonnull(q); // expected-warning {{null passed to a callee that requires a non-null 1st parameter}}
   }
   Dummy a;
   Dummy *_Nonnull nonnull = &a;
-  nonnull = q; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+  nonnull = q; // expected-warning {{null assigned to a pointer which is expected to have non-null value}}
   q = &a;
   takesNullable(q);
   takesNonnull(q);
 }
 
 void testMultiParamChecking(Dummy *_Nonnull a, Dummy *_Nullable b,
                             Dummy *_Nonnull c);
 

 
 Dummy *_Nonnull testNullableReturn(Dummy *_Nullable a) {
   Dummy *p = a;
   return p; // expected-warning {{Nullable pointer is returned from a function that is expected to return a non-null value}}
 }
 
 Dummy *_Nonnull testNullReturn() {
   Dummy *p = 0;
-  return p; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+  return p; // expected-warning {{null returned from a function that is expected to return a non-null value}}
 }
 
 void testObjCMessageResultNullability() {
   // The expected result: the most nullable of self and method return type.
   TestObject *o = getUnspecifiedTestObject();
   int *shouldBeNullable = [eraseNullab(getNullableTestObject()) returnsNonnull];
   switch (getRandom()) {
   case 0:

 Dummy * _Nonnull testDirectCastNilToNonnull() {
   takesNonnull((Dummy * _Nonnull)0);  // no-warning
   return (Dummy * _Nonnull)0;         // no-warning
 }
 
 void testIndirectCastNilToNonnullAndPass() {
   Dummy *p = (Dummy * _Nonnull)0;
   // FIXME: Ideally the cast above would suppress this warning.
-  takesNonnull(p);  // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
+  takesNonnull(p);  // expected-warning {{null passed to a callee that requires a non-null 1st parameter}}
 }
 
 void testDirectCastNilToNonnullAndAssignToLocalInInitializer() {
   Dummy * _Nonnull nonnullLocalWithAssignmentInInitializer = (Dummy * _Nonnull)0; // no-warning
   (void)nonnullLocalWithAssignmentInInitializer;
 
   // Since we've already had an invariant violation along this path,
   // we shouldn't warn here.
-(void)testDirectCastNilToNonnullAndAssignToIvar {
   // Since we've already had an invariant violation along this path,
   // we shouldn't warn here.
   _nonnullIvar = 0;
 }
 @end
 
 void testIndirectNilPassToNonnull() {
   Dummy *p = 0;
-  takesNonnull(p);  // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
+  takesNonnull(p);  // expected-warning {{null passed to a callee that requires a non-null 1st parameter}}
 }
 
 void testConditionalNilPassToNonnull(Dummy *p) {
   if (!p) {
-    takesNonnull(p);  // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
+    takesNonnull(p);  // expected-warning {{null passed to a callee that requires a non-null 1st parameter}}
   }
 }
 
 Dummy * _Nonnull testIndirectCastNilToNonnullAndReturn() {
   Dummy *p = (Dummy * _Nonnull)0;
   // FIXME: Ideally the cast above would suppress this warning.
-  return p; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+  return p; // expected-warning {{null returned from a function that is expected to return a non-null value}}
 }
 
 void testInvalidPropagation() {
   Dummy *p = returnsUnspecified();
   takesNullable(p);
   takesNonnull(p);
 }
 

test/Analysis/nullability_nullonly.mm

 
 void testBasicRules() {
   // The tracking of nullable values is turned off.
   Dummy *p = returnsNullable();
   takesNonnull(p); // no warning
   Dummy *q = 0;
   if (getRandom()) {
     takesNullable(q);
-    takesNonnull(q); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
+    takesNonnull(q); // expected-warning {{null passed to a callee that requires a non-null 1st parameter}}
   }
 }
 
 Dummy *_Nonnull testNullReturn() {
   Dummy *p = 0;
-  return p; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+  return p; // expected-warning {{null returned from a function that is expected to return a non-null value}}
 }
 
 void onlyReportFirstPreconditionViolationOnPath() {
   Dummy *p = 0;
-  takesNonnull(p); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
+  takesNonnull(p); // expected-warning {{null passed to a callee that requires a non-null 1st parameter}}
   takesNonnull(p); // No warning.
   // Passing null to nonnull is a sink. Stop the analysis.
   int i = 0;
   i = 5 / i; // no warning
   (void)i;
 }
 
 Dummy *_Nonnull doNotWarnWhenPreconditionIsViolatedInTopFunc(

 TestObject *_Nonnull getNonnullTestObject();
 
 void testObjCARCImplicitZeroInitialization() {
   TestObject * _Nonnull implicitlyZeroInitialized; // no-warning
   implicitlyZeroInitialized = getNonnullTestObject();
 }
 
 void testObjCARCExplicitZeroInitialization() {
-  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}}
 }
 
 // Under ARC, returned expressions of ObjC objects types are are implicitly
 // cast to _Nonnull when the functions return type is _Nonnull, so make
 // sure this doesn't implicit cast doesn't suppress a legitimate warning.
 TestObject * _Nonnull returnsNilObjCInstanceIndirectly() {
-  TestObject *local = 0;
-  return local; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+  TestObject *local = nil;
+  return local; // expected-warning {{nil returned from a function that is expected to return a non-null value}}
 }
 
 TestObject * _Nonnull returnsNilObjCInstanceIndirectlyWithSupressingCast() {
-  TestObject *local = 0;
+  TestObject *local = nil;
   return (TestObject * _Nonnull)local; // no-warning
 }
 
 TestObject * _Nonnull returnsNilObjCInstanceDirectly() {
-  return nil; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+  return nil; // expected-warning {{nil returned from a function that is expected to return a non-null value}}
 }
 
 TestObject * _Nonnull returnsNilObjCInstanceDirectlyWithSuppressingCast() {
   return (TestObject * _Nonnull)nil; // no-warning
 }
 
 @interface SomeClass : NSObject
 @end
 
 @implementation SomeClass (MethodReturn)
 - (SomeClass * _Nonnull)testReturnsNilInNonnull {
   SomeClass *local = nil;
-  return local; // expected-warning {{Null is returned from a method that is expected to return a non-null value}}
+  return local; // expected-warning {{nil returned from a method that is expected to return a non-null value}}
 }
 
 - (SomeClass * _Nonnull)testReturnsCastSuppressedNilInNonnull {
   SomeClass *local = nil;
   return (SomeClass * _Nonnull)local; // no-warning
 }
 
 - (SomeClass * _Nonnull)testReturnsNilInNonnullWhenPreconditionViolated:(SomeClass * _Nonnull) p {
   SomeClass *local = nil;
   if (!p) // Pre-condition violated here.
     return local; // no-warning
   else
     return p; // no-warning
 }
 @end
 
 
 void callFunctionInSystemHeader() {
   NSString *s;
   s = nil;
 
   NSSystemFunctionTakingNonnull(s);
   #if !NOSYSTEMHEADERS
-  // expected-warning@-2{{Null passed to a callee that requires a non-null 1st parameter}}
+  // expected-warning@-2{{nil passed to a callee that requires a non-null 1st parameter}}
   #endif
 }
 
 void callMethodInSystemHeader() {
   NSString *s;
   s = nil;
 
   NSSystemClass *sc = [[NSSystemClass alloc] init];
   [sc takesNonnull:s];
   #if !NOSYSTEMHEADERS
-  // expected-warning@-2{{Null passed to a callee that requires a non-null 1st parameter}}
+  // expected-warning@-2{{nil passed to a callee that requires a non-null 1st parameter}}
   #endif
 }