From bf1c593d0730a0251db40c972123c7dbd834e7fc Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Mon, 27 Apr 2020 04:56:53 -0700 Subject: [PATCH] [nullsafe] Make Trust list a closed type, and expose only emptiness. Summary: As artempyanykh pointed out, exposing trust list might encourage clients to start writing business logic manipulating with trust lists outside of NullsafeMode module, which we don't like to happen. Reviewed By: artempyanykh Differential Revision: D21230973 fbshipit-source-id: 39bd0b0d8 --- infer/src/nullsafe/ClassLevelAnalysis.ml | 18 ++++++++++-------- infer/src/nullsafe/Nullability.ml | 6 +++--- infer/src/nullsafe/NullsafeMode.ml | 6 +++++- infer/src/nullsafe/NullsafeMode.mli | 6 +++++- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/infer/src/nullsafe/ClassLevelAnalysis.ml b/infer/src/nullsafe/ClassLevelAnalysis.ml index ce3acba5e..a0d43bcb3 100644 --- a/infer/src/nullsafe/ClassLevelAnalysis.ml +++ b/infer/src/nullsafe/ClassLevelAnalysis.ml @@ -44,16 +44,18 @@ type meta_issue = ; severity: Exceptions.severity ; meta_issue_info: Jsonbug_t.nullsafe_meta_issue_info } -let mode_to_json = function - | NullsafeMode.Default -> +let mode_to_json mode = + let open NullsafeMode in + match mode with + | Default -> `Default - | NullsafeMode.Local NullsafeMode.Trust.All -> + | Local Trust.All -> `LocalTrustAll - | NullsafeMode.Local (NullsafeMode.Trust.Only names) when JavaClassName.Set.is_empty names -> + | Local (Trust.Only trust_list) when Trust.is_trust_none trust_list -> `LocalTrustNone - | NullsafeMode.Local (NullsafeMode.Trust.Only _) -> + | Local (Trust.Only _) -> `LocalTrustSome - | NullsafeMode.Strict -> + | Strict -> `Strict @@ -105,8 +107,8 @@ let make_meta_issue all_issues current_mode class_name = match mode_to_promote_to with | NullsafeMode.Local NullsafeMode.Trust.All -> trust_all_mode - | NullsafeMode.Local (NullsafeMode.Trust.Only names) - when JavaClassName.Set.is_empty names -> + | NullsafeMode.Local (NullsafeMode.Trust.Only trust_list) + when NullsafeMode.Trust.is_trust_none trust_list -> trust_none_mode | NullsafeMode.Strict (* We don't recommend "strict" for now as it is harder to keep a class in strict mode than it "trust none" mode. diff --git a/infer/src/nullsafe/Nullability.ml b/infer/src/nullsafe/Nullability.ml index b7eb6a808..1fc8a8dc6 100644 --- a/infer/src/nullsafe/Nullability.ml +++ b/infer/src/nullsafe/Nullability.ml @@ -49,13 +49,13 @@ let is_considered_nonnull ~nullsafe_mode nullability = match nullsafe_mode with | NullsafeMode.Strict -> StrictNonnull - | NullsafeMode.Local (NullsafeMode.Trust.Only classes) when JavaClassName.Set.is_empty classes - -> + | 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 _classes) -> + | NullsafeMode.Local (NullsafeMode.Trust.Only _) -> LocallyTrustedNonnull | NullsafeMode.Local NullsafeMode.Trust.All -> UncheckedNonnull diff --git a/infer/src/nullsafe/NullsafeMode.ml b/infer/src/nullsafe/NullsafeMode.ml index 76f74f894..c6d09ace1 100644 --- a/infer/src/nullsafe/NullsafeMode.ml +++ b/infer/src/nullsafe/NullsafeMode.ml @@ -9,7 +9,11 @@ open! IStd module F = Format module Trust = struct - type t = All | Only of JavaClassName.Set.t [@@deriving compare, equal] + type trust_list = JavaClassName.Set.t [@@deriving compare, equal] + + type t = All | Only of trust_list [@@deriving compare, equal] + + let is_trust_none trust_list = JavaClassName.Set.is_empty trust_list let none = Only JavaClassName.Set.empty diff --git a/infer/src/nullsafe/NullsafeMode.mli b/infer/src/nullsafe/NullsafeMode.mli index cb54b8ec3..80ed9463c 100644 --- a/infer/src/nullsafe/NullsafeMode.mli +++ b/infer/src/nullsafe/NullsafeMode.mli @@ -12,7 +12,11 @@ open! IStd module Trust : sig [@@@warning "-32"] - type t = All | Only of JavaClassName.Set.t [@@deriving compare, equal] + type trust_list + + val is_trust_none : trust_list -> bool + + type t = All | Only of trust_list [@@deriving compare, equal] val none : t