diff --git a/infer/src/nullsafe/AnnotatedSignature.ml b/infer/src/nullsafe/AnnotatedSignature.ml index 29ef65d75..c6295331f 100644 --- a/infer/src/nullsafe/AnnotatedSignature.ml +++ b/infer/src/nullsafe/AnnotatedSignature.ml @@ -16,10 +16,7 @@ module L = Logging *) type t = - { nullsafe_mode: NullsafeMode.t - ; model_source: model_source option - ; ret: ret_signature - ; params: param_signature list } + {nullsafe_mode: NullsafeMode.t; kind: kind; ret: ret_signature; params: param_signature list} [@@deriving compare] and ret_signature = {ret_annotation_deprecated: Annot.Item.t; ret_annotated_type: AnnotatedType.t} @@ -31,7 +28,12 @@ and param_signature = ; param_annotated_type: AnnotatedType.t } [@@deriving compare] -and model_source = InternalModel | ThirdPartyRepo of {filename: string; line_number: int} +and kind = FirstParty | ThirdParty of third_party_model_source [@deriving compare] + +and third_party_model_source = + | Unregistered + | ModelledInternally + | InThirdPartyRepo of {filename: string; line_number: int} [@@deriving compare] (* get nullability of method's return type given its annotations and information about its params *) @@ -102,7 +104,8 @@ let get ~is_callee_in_trust_list ~nullsafe_mode ; mangled ; param_annotated_type= AnnotatedType.{nullability; typ} } ) in - {nullsafe_mode; model_source= None; ret; params} + let kind = if is_third_party then ThirdParty Unregistered else FirstParty in + {nullsafe_mode; kind; ret; params} let get_for_class_under_analysis tenv proc_attributes = @@ -191,7 +194,11 @@ let set_modelled_nullability proc_name asig model_source (nullability_for_ret, p in model_param_nullability asig.params params_nullability in - { asig with - ret= set_modelled_nullability_for_ret asig.ret nullability_for_ret - ; model_source= Some model_source - ; params= final_params } + match model_source with + | Unregistered -> + Logging.die InternalError "the method should be either internally or externally modelled" + | ModelledInternally | InThirdPartyRepo _ -> + { asig with + ret= set_modelled_nullability_for_ret asig.ret nullability_for_ret + ; kind= ThirdParty model_source + ; params= final_params } diff --git a/infer/src/nullsafe/AnnotatedSignature.mli b/infer/src/nullsafe/AnnotatedSignature.mli index 54b76342e..0e1253458 100644 --- a/infer/src/nullsafe/AnnotatedSignature.mli +++ b/infer/src/nullsafe/AnnotatedSignature.mli @@ -10,10 +10,7 @@ open! IStd type t = - { nullsafe_mode: NullsafeMode.t - ; model_source: model_source option (** None, if signature is not modelled *) - ; ret: ret_signature - ; params: param_signature list } + {nullsafe_mode: NullsafeMode.t; kind: kind; ret: ret_signature; params: param_signature list} [@@deriving compare] and ret_signature = {ret_annotation_deprecated: Annot.Item.t; ret_annotated_type: AnnotatedType.t} @@ -25,10 +22,19 @@ and param_signature = ; param_annotated_type: AnnotatedType.t } [@@deriving compare] -and model_source = InternalModel | ThirdPartyRepo of {filename: string; line_number: int} +and kind = + | FirstParty (** Code under control. Its nullability should be expressed via annotations. *) + | ThirdParty of third_party_model_source [@deriving compare] + +and third_party_model_source = + | Unregistered + (** This is an unregistered third party method. It's nullability is best effort based on its + annotations. Lack of annotation is treated depending on the mode. *) + | ModelledInternally + | InThirdPartyRepo of {filename: string; line_number: int} [@@deriving compare] -val set_modelled_nullability : Procname.t -> t -> model_source -> bool * bool list -> t +val set_modelled_nullability : Procname.t -> t -> third_party_model_source -> bool * bool list -> t (** Override nullability for a function signature given its modelled nullability (for ret value and params) *) diff --git a/infer/src/nullsafe/AssignmentRule.ml b/infer/src/nullsafe/AssignmentRule.ml index f88e67369..324d28abd 100644 --- a/infer/src/nullsafe/AssignmentRule.ml +++ b/infer/src/nullsafe/AssignmentRule.ml @@ -19,7 +19,7 @@ module ReportableViolation = struct and function_info = { param_signature: AnnotatedSignature.param_signature - ; model_source: AnnotatedSignature.model_source option + ; kind: AnnotatedSignature.kind ; actual_param_expression: string ; param_position: int ; function_procname: Procname.t } @@ -62,7 +62,7 @@ module ReportableViolation = struct let mk_description_for_bad_param_passed - {model_source; param_signature; actual_param_expression; param_position; function_procname} + {kind; param_signature; actual_param_expression; param_position; function_procname} ~param_nullability_kind ~nullability_evidence = let nullability_evidence_as_suffix = Option.value_map nullability_evidence ~f:(fun evidence -> ": " ^ evidence) ~default:"" @@ -118,12 +118,12 @@ module ReportableViolation = struct | Nullability.UncheckedNonnull | Nullability.StrictNonnull -> let nonnull_evidence = - match model_source with - | None -> + match kind with + | FirstParty | ThirdParty Unregistered -> "" - | Some InternalModel -> + | ThirdParty ModelledInternally -> " (according to nullsafe internal models)" - | Some (ThirdPartyRepo {filename; line_number}) -> + | ThirdParty (InThirdPartyRepo {filename; line_number}) -> Format.sprintf " (see %s at line %d)" (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name ~filename) diff --git a/infer/src/nullsafe/AssignmentRule.mli b/infer/src/nullsafe/AssignmentRule.mli index 54b9cdb91..8fdc03a52 100644 --- a/infer/src/nullsafe/AssignmentRule.mli +++ b/infer/src/nullsafe/AssignmentRule.mli @@ -32,7 +32,7 @@ module ReportableViolation : sig and function_info = { param_signature: AnnotatedSignature.param_signature - ; model_source: AnnotatedSignature.model_source option + ; kind: AnnotatedSignature.kind ; actual_param_expression: string ; param_position: int ; function_procname: Procname.t } diff --git a/infer/src/nullsafe/ErrorRenderingUtils.ml b/infer/src/nullsafe/ErrorRenderingUtils.ml index 17d21d570..c01ca4a3f 100644 --- a/infer/src/nullsafe/ErrorRenderingUtils.ml +++ b/infer/src/nullsafe/ErrorRenderingUtils.ml @@ -62,17 +62,18 @@ let is_object_nullability_self_explanatory ~object_expression (object_origin : T the user can quickly go to field_name definition and see if its annotation. *) let field_name_str = Fieldname.get_field_name field_name in String.is_suffix object_expression ~suffix:field_name_str - | MethodCall {pname; annotated_signature= {model_source}} -> - let is_modelled = Option.is_some model_source in - if is_modelled then (* This is non-trivial and should always be explained to the user *) - false - else + | MethodCall {pname; annotated_signature= {kind}} -> ( + match kind with + | FirstParty | ThirdParty Unregistered -> (* Either local variable or expression like .method_name(...). Latter case is self-explanatory: it is easy to the user to jump to definition and check out the method annotation. *) let method_name = Procname.to_simplified_string pname in String.is_suffix object_expression ~suffix:method_name + | ThirdParty ModelledInternally | ThirdParty (InThirdPartyRepo _) -> + (* This is non-trivial and should always be explained to the user *) + false ) (* These cases are not yet supported because they normally mean non-nullable case, for which we don't render important messages yet. *) diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index bf8f9071a..6688960df 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -427,7 +427,7 @@ let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~ ; assignment_type= PassingParamToFunction { param_signature= formal - ; model_source= callee_annotated_signature.AnnotatedSignature.model_source + ; kind= callee_annotated_signature.AnnotatedSignature.kind ; actual_param_expression ; param_position ; function_procname= callee_pname } }) diff --git a/infer/src/nullsafe/models.ml b/infer/src/nullsafe/models.ml index faeb72e3d..ef5d2ff51 100644 --- a/infer/src/nullsafe/models.ml +++ b/infer/src/nullsafe/models.ml @@ -81,7 +81,7 @@ let get_modelled_annotated_signature ~is_callee_in_trust_list tenv proc_attribut get_special_method_modelled_nullability tenv proc_name ) in Option.value_map modelled_nullability - ~f:(AnnotatedSignature.set_modelled_nullability proc_name ann_sig InternalModel) + ~f:(AnnotatedSignature.set_modelled_nullability proc_name ann_sig ModelledInternally) ~default:ann_sig in (* Look at external models *) @@ -97,7 +97,7 @@ let get_modelled_annotated_signature ~is_callee_in_trust_list tenv proc_attribut (* If we found information in third-party repo, overwrite annotated signature *) ~f:(fun (modelled_nullability, filename, line_number) -> AnnotatedSignature.set_modelled_nullability proc_name ann_sig - (ThirdPartyRepo {filename; line_number}) + (InThirdPartyRepo {filename; line_number}) modelled_nullability ) ~default:ann_sig in diff --git a/infer/src/nullsafe/typeOrigin.ml b/infer/src/nullsafe/typeOrigin.ml index e9710ccc7..f2cf948e3 100644 --- a/infer/src/nullsafe/typeOrigin.ml +++ b/infer/src/nullsafe/typeOrigin.ml @@ -114,14 +114,18 @@ let rec to_string = function let atline loc = " at line " ^ string_of_int loc.Location.line let get_method_ret_description pname call_loc - AnnotatedSignature.{model_source; ret= {ret_annotated_type= {nullability}}} = + AnnotatedSignature.{kind; ret= {ret_annotated_type= {nullability}}} = let should_show_class_name = (* Class name is generally redundant: the user knows line number and can immediatelly go to definition and see the function annotation. When something is modelled though, let's show the class name as well, so it is super clear what exactly is modelled. *) - Option.is_some model_source + match kind with + | FirstParty | ThirdParty Unregistered -> + false + | ThirdParty ModelledInternally | ThirdParty (InThirdPartyRepo _) -> + true in let ret_nullability = match nullability with @@ -135,12 +139,12 @@ let get_method_ret_description pname call_loc "non-nullable" in let model_info = - match model_source with - | None -> + match kind with + | FirstParty | ThirdParty Unregistered -> "" - | Some InternalModel -> + | ThirdParty ModelledInternally -> Format.sprintf " (%s according to nullsafe internal models)" ret_nullability - | Some (ThirdPartyRepo {filename; line_number}) -> + | ThirdParty (InThirdPartyRepo {filename; line_number}) -> let filename_to_show = ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name ~filename in