[nullsafe] Primitive types are always Nonnull

Summary:
Primitive types are not annotated. Because of that, we used to implicitly derive
`DeclaredNonnull` type for them. This worked fine, but this leads to errors in Strict mode, which does
not believe DeclaredNonnull type.

Now lets offifically teach nullsafe that primitive types are
non-nullable.

Reviewed By: jvillard

Differential Revision: D18114623

fbshipit-source-id: 227217931

infer/src/nullsafe/AnnotatedField.ml

@@ -28,7 +28,8 @@ let get tenv fn typ =
     { annotation_deprecated= annotation
     ; annotated_type=
         AnnotatedType.
-          {nullability= AnnotatedNullability.of_annot_item annotation ~is_strict_mode; typ} }
+          { nullability= AnnotatedNullability.of_type_and_annotation typ annotation ~is_strict_mode
+          ; typ } }
   in
   Option.map
     (Typ.Struct.get_field_type_and_annotation ~lookup fn typ)

infer/src/nullsafe/AnnotatedNullability.ml

@@ -40,6 +40,7 @@ and declared_nonnull_origin =
 and nonnull_origin =
   | ModelledNonnull  (** nullsafe knows it is non-nullable via its internal models *)
   | StrictMode  (** under strict mode we consider non-null declarations to be trusted *)
+  | PrimitiveType  (** Primitive types are non-nullable by language design *)
 [@@deriving compare]
 
 let get_nullability = function
@@ -67,7 +68,13 @@ let pp fmt t =
     match origin with AnnotatedNonnull -> "@" | ImplicitlyNonnull -> "implicit"
   in
   let string_of_nonnull_origin nonnull_origin =
-    match nonnull_origin with ModelledNonnull -> "model" | StrictMode -> "strict"
+    match nonnull_origin with
+    | ModelledNonnull ->
+        "model"
+    | StrictMode ->
+        "strict"
+    | PrimitiveType ->
+        "primitive"
   in
   match t with
   | Nullable origin ->
@@ -78,14 +85,15 @@ let pp fmt t =
       F.fprintf fmt "Nonnull[%s]" (string_of_nonnull_origin origin)
 
 
-let of_annot_item ~is_strict_mode ia =
-  if Annotations.ia_is_nullable ia then
+let of_type_and_annotation ~is_strict_mode typ annotations =
+  if not (PatternMatch.type_is_class typ) then Nonnull PrimitiveType
+  else if Annotations.ia_is_nullable annotations then
     let nullable_origin =
-      if Annotations.ia_is_propagates_nullable ia then AnnotatedPropagatesNullable
+      if Annotations.ia_is_propagates_nullable annotations then AnnotatedPropagatesNullable
       else AnnotatedNullable
     in
     Nullable nullable_origin
   else if is_strict_mode then Nonnull StrictMode
-  else if Annotations.ia_is_nonnull ia then DeclaredNonnull AnnotatedNonnull
+  else if Annotations.ia_is_nonnull annotations then DeclaredNonnull AnnotatedNonnull
     (* Currently, we treat not annotated types as nonnull *)
   else DeclaredNonnull ImplicitlyNonnull

infer/src/nullsafe/AnnotatedNullability.mli

@@ -43,12 +43,13 @@ and declared_nonnull_origin =
 and nonnull_origin =
   | ModelledNonnull  (** nullsafe knows it is non-nullable via its internal models *)
   | StrictMode  (** under strict mode we consider non-null declarations to be trusted *)
+  | PrimitiveType  (** Primitive types are non-nullable by language design *)
 [@@deriving compare]
 
 val get_nullability : t -> Nullability.t
 
-val of_annot_item : is_strict_mode:bool -> Annot.Item.t -> t
-(** Converts the information from the annotation to nullability.
+val of_type_and_annotation : is_strict_mode:bool -> Typ.t -> Annot.Item.t -> t
+(** Given the type and its annotations, returns its nullability.
     NOTE: it does not take into account models etc., so this is intended to be used
     as a helper function for more high-level annotation processing.
  *)

infer/src/nullsafe/AnnotatedSignature.ml

@@ -28,8 +28,11 @@ and param_signature =
 [@@deriving compare]
 
 (* get nullability of method's return type given its annotations and information about its params *)
-let nullability_for_return ia ~is_strict_mode ~has_propagates_nullable_in_param =
-  let nullability = AnnotatedNullability.of_annot_item ~is_strict_mode ia in
+let nullability_for_return ret_type ret_annotations ~is_strict_mode
+    ~has_propagates_nullable_in_param =
+  let nullability =
+    AnnotatedNullability.of_type_and_annotation ~is_strict_mode ret_type ret_annotations
+  in
   (* if any param is annotated with propagates nullable, then the result is nullable *)
   match nullability with
   | AnnotatedNullability.Nullable _ ->
@@ -43,9 +46,10 @@ let nullability_for_return ia ~is_strict_mode ~has_propagates_nullable_in_param
 
 (* Given annotations for method signature, extract nullability information
    for return type and params *)
-let extract_nullability ~is_strict_mode return_annotation param_annotations =
+let extract_nullability ~is_strict_mode ret_type ret_annotations param_annotated_types =
   let params_nullability =
-    List.map param_annotations ~f:(AnnotatedNullability.of_annot_item ~is_strict_mode)
+    List.map param_annotated_types ~f:(fun (typ, annotations) ->
+        AnnotatedNullability.of_type_and_annotation typ annotations ~is_strict_mode )
   in
   let has_propagates_nullable_in_param =
     List.exists params_nullability ~f:(function
@@ -55,13 +59,14 @@ let extract_nullability ~is_strict_mode return_annotation param_annotations =
           false )
   in
   let return_nullability =
-    nullability_for_return return_annotation ~is_strict_mode ~has_propagates_nullable_in_param
+    nullability_for_return ret_type ret_annotations ~is_strict_mode
+      ~has_propagates_nullable_in_param
   in
   (return_nullability, params_nullability)
 
 
 let get ~is_strict_mode proc_attributes : t =
-  let Annot.Method.{return= return_annotation; params= original_params_annotation} =
+  let Annot.Method.{return= ret_annotation; params= original_params_annotation} =
     proc_attributes.ProcAttributes.method_annotation
   in
   let formals = proc_attributes.ProcAttributes.formals in
@@ -85,12 +90,14 @@ let get ~is_strict_mode proc_attributes : t =
     in
     List.rev (zip_params (List.rev original_params_annotation) (List.rev formals))
   in
-  let _, final_params_annotation = List.unzip params_with_annotations in
+  let param_annotated_types =
+    List.map params_with_annotations ~f:(fun ((_, typ), annotations) -> (typ, annotations))
+  in
   let return_nullability, params_nullability =
-    extract_nullability ~is_strict_mode return_annotation final_params_annotation
+    extract_nullability ~is_strict_mode ret_type ret_annotation param_annotated_types
   in
   let ret =
-    { ret_annotation_deprecated= return_annotation
+    { ret_annotation_deprecated= ret_annotation
     ; ret_annotated_type= AnnotatedType.{nullability= return_nullability; typ= ret_type} }
   in
   let params =

infer/tests/codetoanalyze/java/nullsafe-default/StrictMode.java

@@ -135,6 +135,12 @@ class Strict {
     return (new NonStrict()).getNullable();
   }
 
+  public int usingPrimitiveTypesFromNonStrictIsOK() {
+    // Of course, primitive types can not be nullable so it
+    // does not matter if they are used from NonStrict
+    return NonStrict.getPrimitiveTypeValue();
+  }
+
   private String nonStrictClass_convertingNonnullToNonnullIsBad() {
     // even that it is declared as nonnull, can not convert it to nonnull it without checking before
     return (new NonStrict()).getNonnull();
@@ -235,4 +241,8 @@ class NonStrict {
   public static String staticNonnull() {
     return "";
   }
+
+  public static int getPrimitiveTypeValue() {
+    return 0;
+  }
 }

infer/tests/codetoanalyze/java/nullsafe-default/issues.exp

@@ -216,7 +216,7 @@ codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.n
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 160)]
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch_after_throw():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch_after_throw()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 172)]
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `tryWithResourcesReturnNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to nullToNullableIsOK() at line 142)]
-codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: call to getNonnull() at line 140)]
+codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: call to getNonnull() at line 146)]
 codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNullableIsOK():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNullableIsOK()` is annotated with `@Nullable` but never returns null.]
 codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNullableToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nonStrictClass_convertingNullableToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: call to getNullable() at line 135)]
 codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition NonStrict.nonnull is always true according to the existing annotations.]