[nullsafe][refactor] AnnotatedSignature distincs between first- and third-party

Summary:
In the current implementation, we record 3 states: modelled internally,
modelled intenally, not modelled.

The follow up diffs will need to distinct first- and third- party, given
annotated signature. This diff makes it possible.

Reviewed By: artempyanykh

Differential Revision: D22977962

fbshipit-source-id: b8f3616b5
master
Mitya Lyubarskiy 4 years ago committed by Facebook GitHub Bot
parent c0152f2278
commit 59592a60e4

@ -16,10 +16,7 @@ module L = Logging
*) *)
type t = type t =
{ nullsafe_mode: NullsafeMode.t {nullsafe_mode: NullsafeMode.t; kind: kind; ret: ret_signature; params: param_signature list}
; model_source: model_source option
; ret: ret_signature
; params: param_signature list }
[@@deriving compare] [@@deriving compare]
and ret_signature = {ret_annotation_deprecated: Annot.Item.t; ret_annotated_type: AnnotatedType.t} 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 } ; param_annotated_type: AnnotatedType.t }
[@@deriving compare] [@@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] [@@deriving compare]
(* get nullability of method's return type given its annotations and information about its params *) (* 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 ; mangled
; param_annotated_type= AnnotatedType.{nullability; typ} } ) ; param_annotated_type= AnnotatedType.{nullability; typ} } )
in 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 = 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 in
model_param_nullability asig.params params_nullability model_param_nullability asig.params params_nullability
in in
{ asig with match model_source with
ret= set_modelled_nullability_for_ret asig.ret nullability_for_ret | Unregistered ->
; model_source= Some model_source Logging.die InternalError "the method should be either internally or externally modelled"
; params= final_params } | ModelledInternally | InThirdPartyRepo _ ->
{ asig with
ret= set_modelled_nullability_for_ret asig.ret nullability_for_ret
; kind= ThirdParty model_source
; params= final_params }

@ -10,10 +10,7 @@
open! IStd open! IStd
type t = type t =
{ nullsafe_mode: NullsafeMode.t {nullsafe_mode: NullsafeMode.t; kind: kind; ret: ret_signature; params: param_signature list}
; model_source: model_source option (** None, if signature is not modelled *)
; ret: ret_signature
; params: param_signature list }
[@@deriving compare] [@@deriving compare]
and ret_signature = {ret_annotation_deprecated: Annot.Item.t; ret_annotated_type: AnnotatedType.t} 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 } ; param_annotated_type: AnnotatedType.t }
[@@deriving compare] [@@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] [@@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 (** Override nullability for a function signature given its modelled nullability (for ret value and
params) *) params) *)

@ -19,7 +19,7 @@ module ReportableViolation = struct
and function_info = and function_info =
{ param_signature: AnnotatedSignature.param_signature { param_signature: AnnotatedSignature.param_signature
; model_source: AnnotatedSignature.model_source option ; kind: AnnotatedSignature.kind
; actual_param_expression: string ; actual_param_expression: string
; param_position: int ; param_position: int
; function_procname: Procname.t } ; function_procname: Procname.t }
@ -62,7 +62,7 @@ module ReportableViolation = struct
let mk_description_for_bad_param_passed 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 = ~param_nullability_kind ~nullability_evidence =
let nullability_evidence_as_suffix = let nullability_evidence_as_suffix =
Option.value_map nullability_evidence ~f:(fun evidence -> ": " ^ evidence) ~default:"" Option.value_map nullability_evidence ~f:(fun evidence -> ": " ^ evidence) ~default:""
@ -118,12 +118,12 @@ module ReportableViolation = struct
| Nullability.UncheckedNonnull | Nullability.UncheckedNonnull
| Nullability.StrictNonnull -> | Nullability.StrictNonnull ->
let nonnull_evidence = let nonnull_evidence =
match model_source with match kind with
| None -> | FirstParty | ThirdParty Unregistered ->
"" ""
| Some InternalModel -> | ThirdParty ModelledInternally ->
" (according to nullsafe internal models)" " (according to nullsafe internal models)"
| Some (ThirdPartyRepo {filename; line_number}) -> | ThirdParty (InThirdPartyRepo {filename; line_number}) ->
Format.sprintf " (see %s at line %d)" Format.sprintf " (see %s at line %d)"
(ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name
~filename) ~filename)

@ -32,7 +32,7 @@ module ReportableViolation : sig
and function_info = and function_info =
{ param_signature: AnnotatedSignature.param_signature { param_signature: AnnotatedSignature.param_signature
; model_source: AnnotatedSignature.model_source option ; kind: AnnotatedSignature.kind
; actual_param_expression: string ; actual_param_expression: string
; param_position: int ; param_position: int
; function_procname: Procname.t } ; function_procname: Procname.t }

@ -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. *) 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 let field_name_str = Fieldname.get_field_name field_name in
String.is_suffix object_expression ~suffix:field_name_str String.is_suffix object_expression ~suffix:field_name_str
| MethodCall {pname; annotated_signature= {model_source}} -> | MethodCall {pname; annotated_signature= {kind}} -> (
let is_modelled = Option.is_some model_source in match kind with
if is_modelled then (* This is non-trivial and should always be explained to the user *) | FirstParty | ThirdParty Unregistered ->
false
else
(* Either local variable or expression like <smth>.method_name(...). (* Either local variable or expression like <smth>.method_name(...).
Latter case is self-explanatory: it is easy to the user to jump to definition Latter case is self-explanatory: it is easy to the user to jump to definition
and check out the method annotation. and check out the method annotation.
*) *)
let method_name = Procname.to_simplified_string pname in let method_name = Procname.to_simplified_string pname in
String.is_suffix object_expression ~suffix:method_name 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 (* These cases are not yet supported because they normally mean non-nullable case, for which
we don't render important messages yet. we don't render important messages yet.
*) *)

@ -427,7 +427,7 @@ let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~
; assignment_type= ; assignment_type=
PassingParamToFunction PassingParamToFunction
{ param_signature= formal { param_signature= formal
; model_source= callee_annotated_signature.AnnotatedSignature.model_source ; kind= callee_annotated_signature.AnnotatedSignature.kind
; actual_param_expression ; actual_param_expression
; param_position ; param_position
; function_procname= callee_pname } }) ; function_procname= callee_pname } })

@ -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 ) get_special_method_modelled_nullability tenv proc_name )
in in
Option.value_map modelled_nullability 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 ~default:ann_sig
in in
(* Look at external models *) (* 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 *) (* If we found information in third-party repo, overwrite annotated signature *)
~f:(fun (modelled_nullability, filename, line_number) -> ~f:(fun (modelled_nullability, filename, line_number) ->
AnnotatedSignature.set_modelled_nullability proc_name ann_sig AnnotatedSignature.set_modelled_nullability proc_name ann_sig
(ThirdPartyRepo {filename; line_number}) (InThirdPartyRepo {filename; line_number})
modelled_nullability ) modelled_nullability )
~default:ann_sig ~default:ann_sig
in in

@ -114,14 +114,18 @@ let rec to_string = function
let atline loc = " at line " ^ string_of_int loc.Location.line let atline loc = " at line " ^ string_of_int loc.Location.line
let get_method_ret_description pname call_loc 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 = let should_show_class_name =
(* Class name is generally redundant: the user knows line number and (* Class name is generally redundant: the user knows line number and
can immediatelly go to definition and see the function annotation. 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 When something is modelled though, let's show the class name as well, so it is
super clear what exactly is modelled. super clear what exactly is modelled.
*) *)
Option.is_some model_source match kind with
| FirstParty | ThirdParty Unregistered ->
false
| ThirdParty ModelledInternally | ThirdParty (InThirdPartyRepo _) ->
true
in in
let ret_nullability = let ret_nullability =
match nullability with match nullability with
@ -135,12 +139,12 @@ let get_method_ret_description pname call_loc
"non-nullable" "non-nullable"
in in
let model_info = let model_info =
match model_source with match kind with
| None -> | FirstParty | ThirdParty Unregistered ->
"" ""
| Some InternalModel -> | ThirdParty ModelledInternally ->
Format.sprintf " (%s according to nullsafe internal models)" ret_nullability Format.sprintf " (%s according to nullsafe internal models)" ret_nullability
| Some (ThirdPartyRepo {filename; line_number}) -> | ThirdParty (InThirdPartyRepo {filename; line_number}) ->
let filename_to_show = let filename_to_show =
ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name ~filename ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name ~filename
in in

Loading…
Cancel
Save