[nullsafe] Introduce LocallyTrustedNonnull nullability

Summary: This diff finishes the stack fixing wrong promotion recommendations!

Reviewed By: artempyanykh

Differential Revision: D20949029

fbshipit-source-id: 59b6ad93f

infer/src/nullsafe/AnnotatedNullability.ml

@@ -19,6 +19,7 @@ type t =
   | Nullable of nullable_origin
   | ThirdPartyNonnull
   | UncheckedNonnull of unchecked_nonnull_origin
+  | LocallyTrustedNonnull
   | LocallyCheckedNonnull
   | StrictNonnull of strict_nonnull_origin
 [@@deriving compare]
@@ -47,6 +48,8 @@ let get_nullability = function
       Nullability.ThirdPartyNonnull
   | UncheckedNonnull _ ->
       Nullability.UncheckedNonnull
+  | LocallyTrustedNonnull ->
+      Nullability.LocallyTrustedNonnull
   | LocallyCheckedNonnull ->
       Nullability.LocallyCheckedNonnull
   | StrictNonnull _ ->
@@ -88,6 +91,8 @@ let pp fmt t =
       F.fprintf fmt "ThirdPartyNonnull"
   | UncheckedNonnull origin ->
       F.fprintf fmt "UncheckedNonnull[%s]" (string_of_declared_nonnull_origin origin)
+  | LocallyTrustedNonnull ->
+      F.fprintf fmt "LocallyTrustedNonnull"
   | LocallyCheckedNonnull ->
       F.fprintf fmt "LocallyCheckedNonnull"
   | StrictNonnull origin ->
@@ -131,4 +136,4 @@ let of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode ~is_third_par
             if Annotations.ia_is_nonnull annotations then UncheckedNonnull AnnotatedNonnull
             else UncheckedNonnull ImplicitlyNonnull
           in
-          if is_callee_in_trust_list then LocallyCheckedNonnull else preliminary_nullability
+          if is_callee_in_trust_list then LocallyTrustedNonnull else preliminary_nullability

infer/src/nullsafe/AnnotatedNullability.mli

@@ -21,6 +21,7 @@ type t =
   | Nullable of nullable_origin
   | ThirdPartyNonnull
   | UncheckedNonnull of unchecked_nonnull_origin
+  | LocallyTrustedNonnull
   | LocallyCheckedNonnull
   | StrictNonnull of strict_nonnull_origin
 [@@deriving compare]

infer/src/nullsafe/AssignmentRule.ml

@@ -113,8 +113,10 @@ module ReportableViolation = struct
           MF.pp_monospaced procname_str param_position pp_param_name param_signature.mangled
           argument_description nullability_evidence_as_suffix MF.pp_monospaced procname_str
           where_to_add_signature
-    | Nullability.LocallyCheckedNonnull | Nullability.UncheckedNonnull | Nullability.StrictNonnull
-      ->
+    | Nullability.LocallyCheckedNonnull
+    | Nullability.LocallyTrustedNonnull
+    | Nullability.UncheckedNonnull
+    | Nullability.StrictNonnull ->
         let nonnull_evidence =
           match model_source with
           | None ->

infer/src/nullsafe/ErrorRenderingUtils.ml

@@ -26,6 +26,9 @@ module UserFriendlyNullable = struct
         Some (UntrustedNonnull LocallyCheckedNonnull)
     | Nullability.ThirdPartyNonnull ->
         Some (UntrustedNonnull ThirdPartyNonnull)
+    | Nullability.LocallyTrustedNonnull ->
+        (* The value is trusted in the current mode by definition, hence is not treated as nullable. *)
+        None
     | Nullability.StrictNonnull ->
         None
 end

infer/src/nullsafe/Nullability.ml

@@ -13,6 +13,7 @@ type t =
   | Nullable
   | ThirdPartyNonnull
   | UncheckedNonnull
+  | LocallyTrustedNonnull
   | LocallyCheckedNonnull
   | StrictNonnull
 [@@deriving compare, equal]
@@ -33,6 +34,8 @@ let join x y =
       ThirdPartyNonnull
   | UncheckedNonnull, _ | _, UncheckedNonnull ->
       UncheckedNonnull
+  | LocallyTrustedNonnull, _ | _, LocallyTrustedNonnull ->
+      LocallyTrustedNonnull
   | LocallyCheckedNonnull, _ | _, LocallyCheckedNonnull ->
       LocallyCheckedNonnull
   | StrictNonnull, StrictNonnull ->
@@ -46,12 +49,17 @@ let is_considered_nonnull ~nullsafe_mode nullability =
     match nullsafe_mode with
     | NullsafeMode.Strict ->
         StrictNonnull
-    | NullsafeMode.Local (NullsafeMode.Trust.Only _classes) ->
-        (* TODO(T61473665). For now treat trust with specified classes as trust=none.  *)
+    | NullsafeMode.Local (NullsafeMode.Trust.Only []) ->
+        (* Though "trust none" is technically a subcase of trust some,
+           we need this pattern to be different from the one below so we can detect possible
+           promotions from "trust some" to "trust none" *)
         LocallyCheckedNonnull
+    | NullsafeMode.Local (NullsafeMode.Trust.Only _classes) ->
+        LocallyTrustedNonnull
     | NullsafeMode.Local NullsafeMode.Trust.All ->
         UncheckedNonnull
     | NullsafeMode.Default ->
+        (* In default mode, we trust everything, even not annotated third party. *)
         ThirdPartyNonnull
   in
   is_subtype ~subtype:nullability ~supertype:least_required
@@ -68,6 +76,8 @@ let to_string = function
       "ThirdPartyNonnull"
   | UncheckedNonnull ->
       "UncheckedNonnull"
+  | LocallyTrustedNonnull ->
+      "LocallyTrustedNonnull"
   | LocallyCheckedNonnull ->
       "LocallyCheckedNonnull"
   | StrictNonnull ->

infer/src/nullsafe/Nullability.mli

@@ -28,6 +28,10 @@ type t =
           conventions) as non-nullable. Hovewer, the class is not checked under [@Nullsafe], so its
           actual nullability is not truthworhy, as the class might contain arbitrary number of
           nullability issues *)
+  | LocallyTrustedNonnull
+      (** The same as [UncheckedNonnull], but the value comes from a class explicitly listed as
+          trusted in the class under analysis. It is less truthworthy than [LocallyCheckedNonnull]
+          as no actual verification were performed. *)
   | LocallyCheckedNonnull
       (** Non-nullable value that comes from a class checked under [@NullsafeLocal] mode. Local mode
           type-checks files against its dependencies but does not require the dependencies to be

infer/src/nullsafe/typeOrigin.ml

@@ -66,6 +66,7 @@ let get_nullability = function
         Nullability.Nullable
     | AnnotatedNullability.UncheckedNonnull _
     | AnnotatedNullability.ThirdPartyNonnull
+    | AnnotatedNullability.LocallyTrustedNonnull
     | AnnotatedNullability.LocallyCheckedNonnull
     | AnnotatedNullability.StrictNonnull _ ->
         (* Nonnull param should be treated as trusted inside this function context:
@@ -131,6 +132,7 @@ let get_method_ret_description pname call_loc
         "nullable"
     | AnnotatedNullability.ThirdPartyNonnull
     | AnnotatedNullability.UncheckedNonnull _
+    | AnnotatedNullability.LocallyTrustedNonnull
     | AnnotatedNullability.LocallyCheckedNonnull
     | AnnotatedNullability.StrictNonnull _ ->
         "non-nullable"

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

@@ -81,11 +81,10 @@ class TrustSome_DoesNotUseTrusted_CanBePromotedToTrustNone {
   }
 }
 
-// FIXME: promo is incorrectly calculated as trust none
 @Nullsafe(
     value = Nullsafe.Mode.LOCAL,
     trustOnly = @Nullsafe.TrustList({Default_NoDeps_CanBePromotedToStrict.class}))
-class TrustSome_UsesTrusted_NoPromo_FIXME {
+class TrustSome_UsesTrusted_NoPromo {
   static String f() {
     return Default_NoDeps_CanBePromotedToStrict.f();
   }

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

@@ -172,7 +172,7 @@ codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], Strict_NoDeps_NoPromos, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Strict"
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_DoesNotUseTrusted_CanBePromotedToTrustNone, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "LocalTrustNone"
 codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_TrustToLocalIsNotNeeded_CanBePromotedToTrustNone, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "LocalTrustNone"
-codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_UsesTrusted_NoPromo_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "LocalTrustNone"
+codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_UsesTrusted_NoPromo, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome"
 codetoanalyze/java/nullsafe-default/MyPreconditions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.MyPreconditions is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], MyPreconditions, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
 codetoanalyze/java/nullsafe-default/NestedFieldAccess.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.NestedFieldAccess is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], NestedFieldAccess, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
 codetoanalyze/java/nullsafe-default/NestedFieldAccess.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.NestedFieldAccess$C is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], NestedFieldAccess$C, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"