[nullsafe] Mode is unchecked unless it is in explicit trust list

Summary:
This diff is a step forward to the state when the list of type violations is
independent of the mode (and we use mode solely to decide re: whether to
report or not).

This fixes a case when we incorrectly defined possible promo mode (see
the test payload)

Reviewed By: artempyanykh

Differential Revision: D20948897

fbshipit-source-id: 616b96f96

infer/src/nullsafe/AnnotatedField.ml

@@ -55,7 +55,7 @@ let get tenv field_name class_typ =
   let is_enum_value = is_enum_value tenv ~class_typ field_info in
   let nullability =
     (* TODO(T62825735): support trusted callees for fields *)
-    AnnotatedNullability.of_type_and_annotation ~is_trusted_callee:false ~nullsafe_mode
+    AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list:false ~nullsafe_mode
       ~is_third_party field_typ annotations
   in
   let corrected_nullability =

infer/src/nullsafe/AnnotatedNullability.ml

@@ -94,7 +94,7 @@ let pp fmt t =
       F.fprintf fmt "StrictNonnull[%s]" (string_of_nonnull_origin origin)
 
 
-let of_type_and_annotation ~is_trusted_callee ~nullsafe_mode ~is_third_party typ annotations =
+let of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party typ annotations =
   if not (PatternMatch.type_is_class typ) then StrictNonnull PrimitiveType
   else if Annotations.ia_is_nullable annotations then
     (* Explicitly nullable always means Nullable *)
@@ -131,4 +131,4 @@ let of_type_and_annotation ~is_trusted_callee ~nullsafe_mode ~is_third_party typ
             if Annotations.ia_is_nonnull annotations then UncheckedNonnull AnnotatedNonnull
             else UncheckedNonnull ImplicitlyNonnull
           in
-          if is_trusted_callee then LocallyCheckedNonnull else preliminary_nullability
+          if is_callee_in_trust_list then LocallyCheckedNonnull else preliminary_nullability

infer/src/nullsafe/AnnotatedNullability.mli

@@ -65,7 +65,7 @@ and strict_nonnull_origin =
 val get_nullability : t -> Nullability.t
 
 val of_type_and_annotation :
-     is_trusted_callee:bool
+     is_callee_in_trust_list:bool
   -> nullsafe_mode:NullsafeMode.t
   -> is_third_party:bool
   -> Typ.t
@@ -73,7 +73,7 @@ val of_type_and_annotation :
   -> 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. [is_trusted_callee] defines whether the callee class is in the caller's trust list
-    and therefore whether its nullability should be refined. *)
+    processing. [is_callee_in_trust_list] defines whether the callee class is in the caller's
+    explicitly provided trust list and therefore whether its nullability should be refined. *)
 
 val pp : Format.formatter -> t -> unit

infer/src/nullsafe/AnnotatedSignature.ml

@@ -35,11 +35,11 @@ and model_source = InternalModel | ThirdPartyRepo of {filename: string; line_num
 [@@deriving compare]
 
 (* get nullability of method's return type given its annotations and information about its params *)
-let nullability_for_return ~is_trusted_callee ~nullsafe_mode ~is_third_party ret_type
+let nullability_for_return ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party ret_type
     ret_annotations ~has_propagates_nullable_in_param =
   let nullability =
-    AnnotatedNullability.of_type_and_annotation ~is_trusted_callee ~nullsafe_mode ~is_third_party
-      ret_type ret_annotations
+    AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode
+      ~is_third_party ret_type ret_annotations
   in
   (* if any param is annotated with propagates nullable, then the result is nullable *)
   match nullability with
@@ -54,11 +54,11 @@ let nullability_for_return ~is_trusted_callee ~nullsafe_mode ~is_third_party ret
 
 (* Given annotations for method signature, extract nullability information
    for return type and params *)
-let extract_nullability ~is_trusted_callee ~nullsafe_mode ~is_third_party ret_type ret_annotations
-    param_annotated_types =
+let extract_nullability ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party ret_type
+    ret_annotations param_annotated_types =
   let params_nullability =
     List.map param_annotated_types ~f:(fun (typ, annotations) ->
-        AnnotatedNullability.of_type_and_annotation ~is_trusted_callee ~nullsafe_mode
+        AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode
           ~is_third_party typ annotations )
   in
   let has_propagates_nullable_in_param =
@@ -69,13 +69,13 @@ let extract_nullability ~is_trusted_callee ~nullsafe_mode ~is_third_party ret_ty
           false )
   in
   let return_nullability =
-    nullability_for_return ~is_trusted_callee ~nullsafe_mode ~is_third_party ret_type
+    nullability_for_return ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party ret_type
       ret_annotations ~has_propagates_nullable_in_param
   in
   (return_nullability, params_nullability)
 
 
-let get ~is_trusted_callee ~nullsafe_mode proc_attributes : t =
+let get ~is_callee_in_trust_list ~nullsafe_mode proc_attributes : t =
   let Annot.Method.{return= ret_annotation; params= original_params_annotation} =
     proc_attributes.ProcAttributes.method_annotation
   in
@@ -110,8 +110,8 @@ let get ~is_trusted_callee ~nullsafe_mode proc_attributes : t =
     List.map params_with_annotations ~f:(fun ((_, typ), annotations) -> (typ, annotations))
   in
   let return_nullability, params_nullability =
-    extract_nullability ~is_trusted_callee ~nullsafe_mode ~is_third_party ret_type ret_annotation
-      param_annotated_types
+    extract_nullability ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party ret_type
+      ret_annotation param_annotated_types
   in
   let ret =
     { ret_annotation_deprecated= ret_annotation
@@ -135,7 +135,9 @@ let get_for_class_under_analysis tenv proc_attributes =
      nullable and (strict) non-null.
      We achieve it via passing Strict mode to the signature extractor.
   *)
-  let result = get ~is_trusted_callee:false ~nullsafe_mode:NullsafeMode.Strict proc_attributes in
+  let result =
+    get ~is_callee_in_trust_list:false ~nullsafe_mode:NullsafeMode.Strict proc_attributes
+  in
   (* Don't forget about the original mode *)
   let nullsafe_mode = NullsafeMode.of_procname tenv proc_attributes.ProcAttributes.proc_name in
   {result with nullsafe_mode}

infer/src/nullsafe/AnnotatedSignature.mli

@@ -35,7 +35,7 @@ val set_modelled_nullability : Procname.t -> t -> model_source -> bool * bool li
 (** Override nullability for a function signature given its modelled nullability (for ret value and
     params) *)
 
-val get : is_trusted_callee:bool -> nullsafe_mode:NullsafeMode.t -> ProcAttributes.t -> t
+val get : is_callee_in_trust_list:bool -> nullsafe_mode:NullsafeMode.t -> ProcAttributes.t -> t
 (** Get a method signature with annotations from a proc_attributes. *)
 
 val get_for_class_under_analysis : Tenv.t -> ProcAttributes.t -> t

infer/src/nullsafe/Initializers.ml

@@ -112,7 +112,8 @@ let final_initializer_typestates_lazy tenv curr_pname curr_pdesc get_procs_in_fi
        ||
        let ia =
          (* TODO(T62825735): support trusted callees for fields *)
-         (Models.get_modelled_annotated_signature ~is_trusted_callee:false tenv proc_attributes)
+         (Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv
+            proc_attributes)
            .AnnotatedSignature.ret
            .ret_annotation_deprecated
        in

infer/src/nullsafe/NullsafeMode.ml

@@ -36,8 +36,13 @@ module Trust = struct
         None
 
 
-  let is_trusted_name t name =
-    match t with All -> true | Only classes -> List.exists classes ~f:(Typ.Name.equal name)
+  let is_in_trust_list t name =
+    match t with
+    | All ->
+        (* We are interested only in explicit lists *)
+        false
+    | Only classes ->
+        List.exists classes ~f:(Typ.Name.equal name)
 
 
   let is_stricter ~stricter ~weaker =
@@ -124,8 +129,8 @@ let of_procname tenv pname =
   of_class tenv class_name
 
 
-let is_trusted_name t name =
-  match t with Strict -> false | Default -> true | Local trust -> Trust.is_trusted_name trust name
+let is_in_trust_list t name =
+  match t with Strict | Default -> false | Local trust -> Trust.is_in_trust_list trust name
 
 
 let is_stricter_than ~stricter ~weaker =

infer/src/nullsafe/NullsafeMode.mli

@@ -35,8 +35,8 @@ val of_class : Tenv.t -> Typ.name -> t
 val of_procname : Tenv.t -> Procname.t -> t
 (** Extracts mode information from a class where procname is defined *)
 
-val is_trusted_name : t -> Typ.name -> bool
-(** Check whether [Typ.name] can be trusted under a given mode *)
+val is_in_trust_list : t -> Typ.name -> bool
+(** Check whether [Typ.name] is in explicit trust list specified in the mode *)
 
 val is_stricter_than : stricter:t -> weaker:t -> bool
 (** Check whether [stricter] is (strongly) stricter than [weaker] *)

infer/src/nullsafe/eradicate.ml

@@ -121,14 +121,14 @@ let analyze_procedure tenv proc_name proc_desc calls_this checks Callbacks.{get_
       | Some (ann_sig, loc, idenv_pn) ->
           (ann_sig, loc, idenv_pn)
       | None ->
-          let is_trusted_callee =
+          let is_callee_in_trust_list =
             let callee_class = Procname.get_class_type_name pname in
             Option.value_map callee_class
-              ~f:(NullsafeMode.is_trusted_name caller_nullsafe_mode)
+              ~f:(NullsafeMode.is_in_trust_list caller_nullsafe_mode)
               ~default:false
           in
           let ann_sig =
-            Models.get_modelled_annotated_signature ~is_trusted_callee tenv
+            Models.get_modelled_annotated_signature ~is_callee_in_trust_list tenv
               (Procdesc.get_attributes pdesc)
           in
           let loc = Procdesc.get_loc pdesc in

infer/src/nullsafe/eradicateChecks.ml

@@ -136,7 +136,8 @@ let check_field_assignment ~nullsafe_mode tenv find_canonical_duplicate curr_pde
       let field_is_in_cleanup_context () =
         let AnnotatedSignature.{ret_annotation_deprecated} =
           (* TODO(T62825735): support trusted callees for fields *)
-          (Models.get_modelled_annotated_signature ~is_trusted_callee:false tenv curr_pattrs).ret
+          (Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv curr_pattrs)
+            .ret
         in
         Annotations.ia_is_cleanup ret_annotation_deprecated
       in
@@ -557,7 +558,8 @@ let check_overridden_annotations find_canonical_duplicate tenv proc_name proc_de
         let base_signature =
           (* TODO(T62825735): fully support trusted callees. Note that for inheritance
              rule it doesn't make much difference, but would be nice to refactor anyway. *)
-          Models.get_modelled_annotated_signature ~is_trusted_callee:false tenv base_attributes
+          Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv
+            base_attributes
         in
         check_inheritance_rule_for_signature
           ~nullsafe_mode:annotated_signature.AnnotatedSignature.nullsafe_mode

infer/src/nullsafe/models.ml

@@ -34,7 +34,7 @@ let table_has_procedure table proc_name =
 let get_modelled_annotated_signature_for_biabduction proc_attributes =
   let proc_name = proc_attributes.ProcAttributes.proc_name in
   let annotated_signature =
-    AnnotatedSignature.get ~is_trusted_callee:false ~nullsafe_mode:NullsafeMode.Default
+    AnnotatedSignature.get ~is_callee_in_trust_list:false ~nullsafe_mode:NullsafeMode.Default
       proc_attributes
   in
   let proc_id = Procname.to_unique_id proc_name in
@@ -67,14 +67,14 @@ let to_modelled_nullability ThirdPartyMethod.{ret_nullability; param_nullability
 
 (** Return the annotated signature of the procedure, taking into account models. External models
     take precedence over internal ones. *)
-let get_modelled_annotated_signature ~is_trusted_callee tenv proc_attributes =
+let get_modelled_annotated_signature ~is_callee_in_trust_list tenv proc_attributes =
   let proc_name = proc_attributes.ProcAttributes.proc_name in
   let nullsafe_mode =
     Procname.get_class_type_name proc_name
     |> Option.value_map ~default:NullsafeMode.Default ~f:(NullsafeMode.of_class tenv)
   in
   let annotated_signature =
-    AnnotatedSignature.get ~is_trusted_callee ~nullsafe_mode proc_attributes
+    AnnotatedSignature.get ~is_callee_in_trust_list ~nullsafe_mode proc_attributes
   in
   let proc_id = Procname.to_unique_id proc_name in
   (* Look in the infer internal models *)

infer/src/nullsafe/typeCheck.ml

@@ -22,7 +22,8 @@ module ComplexExpressions = struct
     | Some proc_attributes ->
         let annotated_signature =
           (* TODO(T62825735): fully support trusted callees *)
-          Models.get_modelled_annotated_signature ~is_trusted_callee:false tenv proc_attributes
+          Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv
+            proc_attributes
         in
         let AnnotatedSignature.{ret_annotation_deprecated} = annotated_signature.ret in
         predicate ret_annotation_deprecated
@@ -1075,14 +1076,14 @@ let typecheck_sil_call_function find_canonical_duplicate checks tenv instr_ref t
             AnnotatedSignature.get_for_class_under_analysis tenv callee_attributes
         | _ ->
             (* The call is to external (relatively to the class under analysis) method. Lookup models, trust lists, etc. *)
-            let is_trusted_callee =
+            let is_callee_in_trust_list =
               let caller_nullsafe_mode = NullsafeMode.of_procname tenv curr_pname in
               let callee_class = Procname.get_class_type_name callee_pname in
               Option.value_map callee_class
-                ~f:(NullsafeMode.is_trusted_name caller_nullsafe_mode)
+                ~f:(NullsafeMode.is_in_trust_list caller_nullsafe_mode)
                 ~default:false
             in
-            Models.get_modelled_annotated_signature ~is_trusted_callee tenv callee_attributes
+            Models.get_modelled_annotated_signature ~is_callee_in_trust_list tenv callee_attributes
       in
       if Config.write_html then
         L.d_printfln "Callee signature: %a"

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

@@ -36,8 +36,7 @@ class Strict_NoDeps_NoPromos {
   }
 }
 
-// FIXME - promo is incorrectly calculated as Trust None.
-class Default_UsesDefault_CanBePromotedToTrustAll_FIXME {
+class Default_UsesDefault_CanBePromotedToTrustAll {
   static String f() {
     // We use unknown default function. Since we don't support trust some in promotions,
     // the possible promotion is trust all.

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

@@ -164,7 +164,7 @@ codetoanalyze/java/nullsafe-default/MapNullability.java, codetoanalyze.java.null
 codetoanalyze/java/nullsafe-default/MapNullability.java, codetoanalyze.java.nullsafe_default.MapNullability$TestThatGetIsAllowedOnlyAfterContainsKeyWasChecked.usingGetWithoutCheckingKeyIsBAD(java.util.Map):void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`m.get(...)` is nullable and is not locally checked for null when calling `isEmpty()`: call to Map.get(...) at line 24 (nullable according to nullsafe internal models).]
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesItself_CanBePromotedToStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesItself_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesLocal_CanBePromotedToTrustNone is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesLocal_CanBePromotedToTrustNone, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustNone"
-codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesDefault_CanBePromotedToTrustAll_FIXME is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesDefault_CanBePromotedToTrustAll_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustNone"
+codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesDefault_CanBePromotedToTrustAll is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesDefault_CanBePromotedToTrustAll, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustAll"
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesStrict_CanBePromotedToStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesStrict_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_NoDeps_CanBePromotedToStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_NoDeps_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], Local_NoDeps_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustAll", promote_mode: "Strict"
@@ -328,7 +328,7 @@ codetoanalyze/java/nullsafe-default/PropagatesNullable.java, codetoanalyze.java.
 codetoanalyze/java/nullsafe-default/PropagatesNullable.java, codetoanalyze.java.nullsafe_default.TestPropagatesNullable$TestSecondParameter.test(java.lang.String,java.lang.String):void, 11, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`nullable(...)` is nullable and is not locally checked for null when calling `length()`.]
 codetoanalyze/java/nullsafe-default/PropagatesNullable.java, codetoanalyze.java.nullsafe_default.TestPropagatesNullable$TestSecondParameter.test(java.lang.String,java.lang.String):void, 15, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`nullable(...)` is nullable and is not locally checked for null when calling `length()`.]
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.ReturnNotNullable$E is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], ReturnNotNullable$E, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
-codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.ReturnNotNullable$Lambda$_9_1 is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], ReturnNotNullable$Lambda$_9_1, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustNone"
+codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.ReturnNotNullable$Lambda$_9_1 is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], ReturnNotNullable$Lambda$_9_1, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustAll"
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], ReturnNotNullable$ConditionalAssignment, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Default"
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], ReturnNotNullable, codetoanalyze.java.nullsafe_default, issues: 9, curr_mode: "Default"
 codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable$ConditionalAssignment.test(boolean):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`test(...)`: return type is declared non-nullable but the method returns a nullable value: field f1 at line 199.]