From 234aae3e60610cb76078613398da62012d294b33 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Fri, 30 Oct 2020 05:13:19 -0700 Subject: [PATCH] [nullsafe][annotation graph] Introduce ProvisionallyNullable nullability Summary: In the previous diffs, we introduced AnnotatedNullability.ProvisionallyNullable. This is a symmetric change introducing the same for Nullability.t Note how ProvisionallyNullable is made `is_nonnullish`. This means that if we run nullsafe in --nullsafe-annotation-graph mode, the following should start happening: 1/ A lot of new issues will be recorded - every time a local method or non-null param is dereferenced etc. 2/ But all of those new issues will be filtered by corresponding Rules (e.g. AssignmentRule). So those issues will not manifest to user-visible errors. However, now we are capable to see and analyse those hidden issues in ClassLevelAnalysis. This is exactly what we will be doing in follow up diffs. Note that the place of ProvisionallyNullable in the hierarchy is somewhat arbitraily. It is important to make it less pri then Nullable - but when it comes to comparing with other "nonnullish" modes - we can tweak the behavior later on. Reviewed By: artempyanykh Differential Revision: D24621411 fbshipit-source-id: 3b99e5e55 --- infer/src/nullsafe/AnnotatedNullability.ml | 2 +- infer/src/nullsafe/AssignmentRule.ml | 2 + infer/src/nullsafe/ErrorRenderingUtils.ml | 2 + infer/src/nullsafe/Nullability.ml | 48 +++++++++++++--------- infer/src/nullsafe/Nullability.mli | 4 ++ infer/src/nullsafe/typeOrigin.ml | 2 +- 6 files changed, 39 insertions(+), 21 deletions(-) diff --git a/infer/src/nullsafe/AnnotatedNullability.ml b/infer/src/nullsafe/AnnotatedNullability.ml index 66e1e2d77..033b379f0 100644 --- a/infer/src/nullsafe/AnnotatedNullability.ml +++ b/infer/src/nullsafe/AnnotatedNullability.ml @@ -47,7 +47,7 @@ let get_nullability = function | Nullable _ -> Nullability.Nullable | ProvisionallyNullable _ -> - Nullability.Nullable + Nullability.ProvisionallyNullable | ThirdPartyNonnull -> Nullability.ThirdPartyNonnull | UncheckedNonnull _ -> diff --git a/infer/src/nullsafe/AssignmentRule.ml b/infer/src/nullsafe/AssignmentRule.ml index 0103e5106..2018ca5bd 100644 --- a/infer/src/nullsafe/AssignmentRule.ml +++ b/infer/src/nullsafe/AssignmentRule.ml @@ -138,6 +138,8 @@ module ReportableViolation = struct in make_issue_factory ~description ~issue_type |> NullsafeIssue.with_third_party_dependent_methods [(procname, annotated_signature)] + (* Equivalent to non-null from user point of view *) + | Nullability.ProvisionallyNullable | Nullability.LocallyCheckedNonnull | Nullability.LocallyTrustedNonnull | Nullability.UncheckedNonnull diff --git a/infer/src/nullsafe/ErrorRenderingUtils.ml b/infer/src/nullsafe/ErrorRenderingUtils.ml index 8c7a78e5b..0032fe4fb 100644 --- a/infer/src/nullsafe/ErrorRenderingUtils.ml +++ b/infer/src/nullsafe/ErrorRenderingUtils.ml @@ -29,6 +29,8 @@ module UserFriendlyNullable = struct | Nullability.LocallyTrustedNonnull -> (* The value is trusted in the current mode by definition, hence is not treated as nullable. *) None + | Nullability.ProvisionallyNullable -> + (* from the user-facing point of view, this is a non-null *) None | Nullability.StrictNonnull -> None end diff --git a/infer/src/nullsafe/Nullability.ml b/infer/src/nullsafe/Nullability.ml index 1fc8a8dc6..a9945f12b 100644 --- a/infer/src/nullsafe/Nullability.ml +++ b/infer/src/nullsafe/Nullability.ml @@ -15,6 +15,7 @@ type t = | UncheckedNonnull | LocallyTrustedNonnull | LocallyCheckedNonnull + | ProvisionallyNullable | StrictNonnull [@@deriving compare, equal] @@ -32,6 +33,8 @@ let join x y = Nullable | ThirdPartyNonnull, _ | _, ThirdPartyNonnull -> ThirdPartyNonnull + | ProvisionallyNullable, _ | _, ProvisionallyNullable -> + ProvisionallyNullable | UncheckedNonnull, _ | _, UncheckedNonnull -> UncheckedNonnull | LocallyTrustedNonnull, _ | _, LocallyTrustedNonnull -> @@ -45,25 +48,30 @@ let join x y = let is_subtype ~subtype ~supertype = equal (join subtype supertype) supertype let is_considered_nonnull ~nullsafe_mode nullability = - let least_required = - match nullsafe_mode with - | NullsafeMode.Strict -> - StrictNonnull - | NullsafeMode.Local (NullsafeMode.Trust.Only trust_list) - when NullsafeMode.Trust.is_trust_none trust_list -> - (* 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 _) -> - 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 + if equal ProvisionallyNullable nullability then + (* Deliberately ignoring modes for this type + (ProvisionallyNullable is currently not indented to work well with all features for non-default modes) *) + true + else + let least_required = + match nullsafe_mode with + | NullsafeMode.Strict -> + StrictNonnull + | NullsafeMode.Local (NullsafeMode.Trust.Only trust_list) + when NullsafeMode.Trust.is_trust_none trust_list -> + (* 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 _) -> + 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 let is_nonnullish t = is_considered_nonnull ~nullsafe_mode:NullsafeMode.Default t @@ -73,6 +81,8 @@ let to_string = function "Null" | Nullable -> "Nullable" + | ProvisionallyNullable -> + "ProvisionallyNullable" | ThirdPartyNonnull -> "ThirdPartyNonnull" | UncheckedNonnull -> diff --git a/infer/src/nullsafe/Nullability.mli b/infer/src/nullsafe/Nullability.mli index d6373ddac..57920d746 100644 --- a/infer/src/nullsafe/Nullability.mli +++ b/infer/src/nullsafe/Nullability.mli @@ -37,6 +37,10 @@ type t = type-checks files against its dependencies but does not require the dependencies to be transitively checked. Therefore this type of non-nullable value is differentiated from StrictNonnull. *) + | ProvisionallyNullable + (** Only for "annotation graph" mode. Indicates the value coming from a not annotated object + in the class under analysis. Since the object is not annotated, it should be treated as + non-null for all cases related to user-facing issue generation. *) | StrictNonnull (** Non-nullable value with the highest degree of certainty, because it is either: diff --git a/infer/src/nullsafe/typeOrigin.ml b/infer/src/nullsafe/typeOrigin.ml index 01416d1ca..e5b9aedbe 100644 --- a/infer/src/nullsafe/typeOrigin.ml +++ b/infer/src/nullsafe/typeOrigin.ml @@ -65,7 +65,7 @@ let get_nullability = function (* Annotated as Nullable explicitly or implicitly *) Nullability.Nullable | AnnotatedNullability.ProvisionallyNullable _ -> - Nullability.Nullable + Nullability.ProvisionallyNullable | AnnotatedNullability.UncheckedNonnull _ | AnnotatedNullability.ThirdPartyNonnull | AnnotatedNullability.LocallyTrustedNonnull