From 5d8b4947fe1399811a59a7430cea6820b1dfc563 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Fri, 24 Apr 2020 06:22:52 -0700 Subject: [PATCH] [nullsafe] NullsafeMode operates over JavaClassName Summary: Lets move the logic dealing with non-java classes outside of this module so we can modify it easier in the next diff. Reviewed By: artempyanykh Differential Revision: D21204822 fbshipit-source-id: 67b5937bc --- infer/src/IR/Typ.mli | 3 +++ infer/src/nullsafe/AnnotatedField.ml | 5 +++- infer/src/nullsafe/ClassLevelAnalysis.ml | 2 +- infer/src/nullsafe/NullsafeMode.ml | 31 ++++++++++++------------ infer/src/nullsafe/NullsafeMode.mli | 8 +++--- infer/src/nullsafe/eradicate.ml | 4 ++- infer/src/nullsafe/models.ml | 5 +--- infer/src/nullsafe/typeCheck.ml | 4 ++- 8 files changed, 35 insertions(+), 27 deletions(-) diff --git a/infer/src/IR/Typ.mli b/infer/src/IR/Typ.mli index 3caac57b4..375f56160 100644 --- a/infer/src/IR/Typ.mli +++ b/infer/src/IR/Typ.mli @@ -227,6 +227,9 @@ module Name : sig val is_class : t -> bool (** [is_class name] holds if [name] names a Java class *) + val get_java_class_name_exn : t -> JavaClassName.t + (** Ensure [name] is a java class name and return underlying JavaClassName *) + val is_external : t -> bool (** return true if the typename is in the .inferconfig list of external classes *) diff --git a/infer/src/nullsafe/AnnotatedField.ml b/infer/src/nullsafe/AnnotatedField.ml index de9a9099d..10e362ea3 100644 --- a/infer/src/nullsafe/AnnotatedField.ml +++ b/infer/src/nullsafe/AnnotatedField.ml @@ -44,7 +44,10 @@ let get tenv field_name class_typ = (* We currently don't support field-level strict mode annotation, so fetch it from class *) let nullsafe_mode = Typ.name class_typ - |> Option.value_map ~f:(NullsafeMode.of_class tenv) ~default:NullsafeMode.Default + |> Option.value_map + ~f:(fun class_name -> + Typ.Name.Java.get_java_class_name_exn class_name |> NullsafeMode.of_class tenv ) + ~default:NullsafeMode.Default in let is_third_party = ThirdPartyAnnotationInfo.is_third_party_typ diff --git a/infer/src/nullsafe/ClassLevelAnalysis.ml b/infer/src/nullsafe/ClassLevelAnalysis.ml index 2333a840f..6294b2faa 100644 --- a/infer/src/nullsafe/ClassLevelAnalysis.ml +++ b/infer/src/nullsafe/ClassLevelAnalysis.ml @@ -154,7 +154,7 @@ let get_class_loc Struct.{java_class_info} = (* Meta issues are those related to null-safety of the class in general, not concrete nullability violations *) let report_meta_issues tenv source_file class_name class_struct class_info issue_log = (* For purposes of aggregation, we consider all nested anonymous summaries as belonging to this class *) - let current_mode = NullsafeMode.of_class tenv (Typ.JavaClass class_name) in + let current_mode = NullsafeMode.of_class tenv class_name in let summaries = AggregatedSummaries.ClassInfo.get_all_summaries class_info in let class_loc = get_class_loc class_struct diff --git a/infer/src/nullsafe/NullsafeMode.ml b/infer/src/nullsafe/NullsafeMode.ml index 179dba219..56ab7d4e3 100644 --- a/infer/src/nullsafe/NullsafeMode.ml +++ b/infer/src/nullsafe/NullsafeMode.ml @@ -9,7 +9,7 @@ open! IStd module F = Format module Trust = struct - type t = All | Only of Typ.name list [@@deriving compare, equal] + type t = All | Only of JavaClassName.t list [@@deriving compare, equal] let none = Only [] @@ -17,7 +17,12 @@ module Trust = struct | Annot.Array class_values -> (* The only elements of this array can be class names; therefore short-circuit and return None if it's not the case. *) IList.traverse_opt class_values ~f:(fun el -> - match el with Annot.Class class_typ -> Typ.name class_typ | _ -> None ) + match el with + | Annot.Class class_typ -> + Typ.name class_typ + |> Option.map ~f:(fun name -> Typ.Name.Java.get_java_class_name_exn name) + | _ -> + None ) | _ -> None @@ -42,7 +47,7 @@ module Trust = struct (* We are interested only in explicit lists *) false | Only classes -> - List.exists classes ~f:(Typ.Name.equal name) + List.exists classes ~f:(JavaClassName.equal name) let is_stricter ~stricter ~weaker = @@ -50,7 +55,7 @@ module Trust = struct (* stricter trust list should be a strict subset of the weaker one *) List.length stricter_list < List.length weaker_list && List.for_all stricter_list ~f:(fun strict_name -> - List.exists weaker_list ~f:(fun name -> Typ.Name.equal name strict_name) ) + List.exists weaker_list ~f:(fun name -> JavaClassName.equal name strict_name) ) in match (stricter, weaker) with | All, All | All, Only _ -> @@ -103,18 +108,14 @@ let of_annot annot = None -let extract_user_defined_class_name typ_name = - match typ_name with - | Typ.JavaClass java_class_name -> - (* Anonymous inner classes are not proper classes and can not be annotated. Refer to underlying user class *) - JavaClassName.get_user_defined_class_if_anonymous_inner java_class_name - |> Option.value ~default:java_class_name - | _ -> - Logging.die InternalError "Unexpected non-Java class name" +let extract_user_defined_class_name java_class_name = + (* Anonymous inner classes are not proper classes and can not be annotated. Refer to underlying user class *) + JavaClassName.get_user_defined_class_if_anonymous_inner java_class_name + |> Option.value ~default:java_class_name -let of_class tenv typ_name = - let user_defined_class = extract_user_defined_class_name typ_name in +let of_class tenv class_name = + let user_defined_class = extract_user_defined_class_name class_name in match PatternMatch.type_name_get_annotation tenv (Typ.JavaClass user_defined_class) with | Some annots -> ( if Annotations.ia_is_nullsafe_strict annots then Strict @@ -137,7 +138,7 @@ let of_procname tenv pname = | _ -> Logging.die InternalError "Unexpected non-Java procname %a" Procname.pp pname in - of_class tenv class_name + of_class tenv (Typ.Name.Java.get_java_class_name_exn class_name) let is_in_trust_list t name = diff --git a/infer/src/nullsafe/NullsafeMode.mli b/infer/src/nullsafe/NullsafeMode.mli index 7238fff81..9421226be 100644 --- a/infer/src/nullsafe/NullsafeMode.mli +++ b/infer/src/nullsafe/NullsafeMode.mli @@ -12,7 +12,7 @@ open! IStd module Trust : sig [@@@warning "-32"] - type t = All | Only of Typ.name list [@@deriving compare, equal] + type t = All | Only of JavaClassName.t list [@@deriving compare, equal] val none : t @@ -29,14 +29,14 @@ val of_annot : Annot.t -> t option [@@warning "-32"] (** Returns [t] when provided annotation matches the format of [@Nullsafe], otherwise [None]. *) -val of_class : Tenv.t -> Typ.name -> t +val of_class : Tenv.t -> JavaClassName.t -> t (** Extracts mode information from class annotations *) val of_procname : Tenv.t -> Procname.t -> t (** Extracts mode information from a class where procname is defined *) -val is_in_trust_list : t -> Typ.name -> bool -(** Check whether [Typ.name] is in explicit trust list specified in the mode *) +val is_in_trust_list : t -> JavaClassName.t -> bool +(** Check whether [JavaClassName.t] 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] *) diff --git a/infer/src/nullsafe/eradicate.ml b/infer/src/nullsafe/eradicate.ml index f04db57fc..34bf3c08c 100644 --- a/infer/src/nullsafe/eradicate.ml +++ b/infer/src/nullsafe/eradicate.ml @@ -124,7 +124,9 @@ let analyze_procedure tenv proc_name proc_desc calls_this checks Callbacks.{get_ let is_callee_in_trust_list = let callee_class = Procname.get_class_type_name pname in Option.value_map callee_class - ~f:(NullsafeMode.is_in_trust_list caller_nullsafe_mode) + ~f:(fun class_name -> + Typ.Name.Java.get_java_class_name_exn class_name + |> NullsafeMode.is_in_trust_list caller_nullsafe_mode ) ~default:false in let ann_sig = diff --git a/infer/src/nullsafe/models.ml b/infer/src/nullsafe/models.ml index e71948e20..06d9c069c 100644 --- a/infer/src/nullsafe/models.ml +++ b/infer/src/nullsafe/models.ml @@ -69,10 +69,7 @@ let to_modelled_nullability ThirdPartyMethod.{ret_nullability; param_nullability take precedence over internal ones. *) 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 nullsafe_mode = NullsafeMode.of_procname tenv proc_name in let annotated_signature = AnnotatedSignature.get ~is_callee_in_trust_list ~nullsafe_mode proc_attributes in diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index 6df61cc3f..62fd11b70 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -1080,7 +1080,9 @@ let typecheck_sil_call_function find_canonical_duplicate checks tenv instr_ref t 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_in_trust_list caller_nullsafe_mode) + ~f:(fun class_name -> + Typ.Name.Java.get_java_class_name_exn class_name + |> NullsafeMode.is_in_trust_list caller_nullsafe_mode ) ~default:false in Models.get_modelled_annotated_signature ~is_callee_in_trust_list tenv callee_attributes