pghs975uc
/
infer_clone
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

[nullsafe] Convert AnnotatedSignature subtypes from tuples to records

Browse Source 
Summary:
1/ Nikos Gorogiannis pointed out that
   - for highly reused public types, records (especially when >= 3 params) are generally more readable than tuples.
   - Records simplify code modifications, especially adding new fields. And we are going to add some, namely param flags, in the future.

2/ Let's make the fact that annotated signature is deprecated more
visible; it will also simplify searching for usages when we will be
getting rid of them.

Reviewed By: ngorogiannis

Differential Revision: D17475033

fbshipit-source-id: 7740c979b
master
Mitya Lyubarskiy 5 years ago committed by Facebook Github Bot
parent b76ab1f8b9
commit 693f2944c7
6 changed files with 111 additions and 76 deletions
Show Stats Download Patch File Download Diff File

81
infer/src/nullsafe/AnnotatedSignature.ml
Unescape View File

@ -15,8 +15,16 @@ module L = Logging
b. Known param annotations b. Known param annotations
c. Known method-level annotations. c. Known method-level annotations.
*) *)
type t =
{ret: Annot.Item.t * NullsafeType.t; params: (Mangled.t * Annot.Item.t * NullsafeType.t) list} type t = {ret: ret_signature; params: param_signature list} [@@deriving compare]
and ret_signature = {ret_annotation_deprecated: Annot.Item.t; ret_nullsafe_type: NullsafeType.t}
[@@deriving compare]
and param_signature =
{ param_annotation_deprecated: Annot.Item.t
; mangled: Mangled.t
; param_nullsafe_type: NullsafeType.t }
[@@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 *)
@ -51,10 +59,11 @@ let extract_nullability return_annotation param_annotations =
let get proc_attributes : t = let get proc_attributes : t =
let method_annotation = proc_attributes.ProcAttributes.method_annotation in let Annot.Method.{return= return_annotation; params= original_params_annotation} =
proc_attributes.ProcAttributes.method_annotation
in
let formals = proc_attributes.ProcAttributes.formals in let formals = proc_attributes.ProcAttributes.formals in
let ret_type = proc_attributes.ProcAttributes.ret_type in let ret_type = proc_attributes.ProcAttributes.ret_type in
let Annot.Method.{return; params} = method_annotation in
(* zip formal params with annotation *) (* zip formal params with annotation *)
let params_with_annotations = let params_with_annotations =
let rec zip_params ial parl = let rec zip_params ial parl =
@ -72,33 +81,40 @@ let get proc_attributes : t =
this should never happen *) this should never happen *)
assert false assert false
in in
List.rev (zip_params (List.rev params) (List.rev formals)) List.rev (zip_params (List.rev original_params_annotation) (List.rev formals))
in
let _, final_params_annotation = List.unzip params_with_annotations in
let return_nullability, params_nullability =
extract_nullability return_annotation final_params_annotation
in
let ret =
{ ret_annotation_deprecated= return_annotation
; ret_nullsafe_type= NullsafeType.{nullability= return_nullability; typ= ret_type} }
in in
let _, param_annotations = List.unzip params_with_annotations in
let return_nullability, params_nullability = extract_nullability return param_annotations in
let ret = (return, NullsafeType.{nullability= return_nullability; typ= ret_type}) in
let params = let params =
List.zip_exn params_with_annotations params_nullability List.map2_exn params_with_annotations params_nullability
|> List.map ~f:(function ((mangled, typ), annot), nullability -> ~f:(fun ((mangled, typ), param_annotation_deprecated) nullability ->
(mangled, annot, NullsafeType.{nullability; typ}) ) {param_annotation_deprecated; mangled; param_nullsafe_type= NullsafeType.{nullability; typ}}
)
in in
{ret; params} {ret; params}
let param_has_annot predicate pvar ann_sig = let param_has_annot predicate pvar ann_sig =
List.exists List.exists
~f:(fun (param, param_annot, _) -> ~f:(fun {mangled; param_annotation_deprecated} ->
Mangled.equal param (Pvar.get_name pvar) && predicate param_annot ) Mangled.equal mangled (Pvar.get_name pvar) && predicate param_annotation_deprecated )
ann_sig.params ann_sig.params
let pp proc_name fmt annotated_signature = let pp proc_name fmt annotated_signature =
let pp_ia fmt ia = if ia <> [] then F.fprintf fmt "%a " Annot.Item.pp ia in let pp_ia fmt ia = if ia <> [] then F.fprintf fmt "%a " Annot.Item.pp ia in
let pp_annotated_param fmt (mangled, ia, nullsafe_type) = let pp_annotated_param fmt {mangled; param_annotation_deprecated; param_nullsafe_type} =
F.fprintf fmt " %a%a %a" pp_ia ia NullsafeType.pp nullsafe_type Mangled.pp mangled F.fprintf fmt " %a%a %a" pp_ia param_annotation_deprecated NullsafeType.pp param_nullsafe_type
Mangled.pp mangled
in in
let ia, nullsafe_type = annotated_signature.ret in let {ret_annotation_deprecated; ret_nullsafe_type} = annotated_signature.ret in
F.fprintf fmt "%a%a %s (%a )" pp_ia ia NullsafeType.pp nullsafe_type F.fprintf fmt "%a%a %s (%a )" pp_ia ret_annotation_deprecated NullsafeType.pp ret_nullsafe_type
(Typ.Procname.to_simplified_string proc_name) (Typ.Procname.to_simplified_string proc_name)
(Pp.comma_seq pp_annotated_param) annotated_signature.params (Pp.comma_seq pp_annotated_param) annotated_signature.params
@ -122,14 +138,18 @@ let set_modelled_nullability_for_nullsafe_type nullsafe_type should_set_nullable
let set_modelled_nullability proc_name asig (nullability_for_ret, params_nullability) = let set_modelled_nullability proc_name asig (nullability_for_ret, params_nullability) =
let set_modelled_nullability_for_param (mangled, original_annotation, original_nullsafe_type) let set_modelled_nullability_for_param param should_set_nullable =
should_set_nullable = { param with
let final_annotation = param_annotation_deprecated=
if should_set_nullable then mk_ia_nullable original_annotation else original_annotation mark_ia_nullability param.param_annotation_deprecated should_set_nullable
; param_nullsafe_type=
set_modelled_nullability_for_nullsafe_type param.param_nullsafe_type should_set_nullable }
in in
( mangled let set_modelled_nullability_for_ret ret should_set_nullable =
, final_annotation { ret_annotation_deprecated=
, set_modelled_nullability_for_nullsafe_type original_nullsafe_type should_set_nullable ) mark_ia_nullability ret.ret_annotation_deprecated should_set_nullable
; ret_nullsafe_type=
set_modelled_nullability_for_nullsafe_type ret.ret_nullsafe_type should_set_nullable }
in in
let final_params = let final_params =
let fail () = let fail () =
@ -140,11 +160,9 @@ let set_modelled_nullability proc_name asig (nullability_for_ret, params_nullabi
in in
let rec model_param_nullability original_params params_nullability = let rec model_param_nullability original_params params_nullability =
match (original_params, params_nullability) with match (original_params, params_nullability) with
| (mangled, annotation, nullsafe_type) :: params_tail, nullability_tail | param :: params_tail, nullability_tail when Mangled.is_this param.mangled ->
when Mangled.is_this mangled ->
(* Skip "this" param - there is no notion of "nullable this" *) (* Skip "this" param - there is no notion of "nullable this" *)
(mangled, annotation, nullsafe_type) param :: model_param_nullability params_tail nullability_tail
:: model_param_nullability params_tail nullability_tail
| param :: params_tail, should_set_nullable :: nullability_tail -> | param :: params_tail, should_set_nullable :: nullability_tail ->
set_modelled_nullability_for_param param should_set_nullable set_modelled_nullability_for_param param should_set_nullable
:: model_param_nullability params_tail nullability_tail :: model_param_nullability params_tail nullability_tail
@ -156,9 +174,4 @@ let set_modelled_nullability proc_name asig (nullability_for_ret, params_nullabi
in in
model_param_nullability asig.params params_nullability model_param_nullability asig.params params_nullability
in in
let original_ret_annotations, original_ret_nullsafe_type = asig.ret in {ret= set_modelled_nullability_for_ret asig.ret nullability_for_ret; params= final_params}
let final_ret_nullsafe_type =
set_modelled_nullability_for_nullsafe_type original_ret_nullsafe_type nullability_for_ret
in
let final_ret_annotation = mark_ia_nullability original_ret_annotations nullability_for_ret in
{ret= (final_ret_annotation, final_ret_nullsafe_type); params= final_params}

12
infer/src/nullsafe/AnnotatedSignature.mli
Unescape View File

@ -9,9 +9,15 @@
open! IStd open! IStd
type t = type t = {ret: ret_signature; params: param_signature list} [@@deriving compare]
{ ret: Annot.Item.t * NullsafeType.t (** Annotated return type. *)
; params: (Mangled.t * Annot.Item.t * NullsafeType.t) list (** Annotated parameters. *) } and ret_signature = {ret_annotation_deprecated: Annot.Item.t; ret_nullsafe_type: NullsafeType.t}
[@@deriving compare]
and param_signature =
{ param_annotation_deprecated: Annot.Item.t
; mangled: Mangled.t
; param_nullsafe_type: NullsafeType.t }
[@@deriving compare] [@@deriving compare]
val param_has_annot : (Annot.Item.t -> bool) -> Pvar.t -> t -> bool val param_has_annot : (Annot.Item.t -> bool) -> Pvar.t -> t -> bool

16
infer/src/nullsafe/eradicate.ml
Unescape View File

@ -32,14 +32,13 @@ end
module MkCallback (Extension : ExtensionT) : CallBackT = struct module MkCallback (Extension : ExtensionT) : CallBackT = struct
let callback1 tenv find_canonical_duplicate calls_this checks idenv curr_pname curr_pdesc let callback1 tenv find_canonical_duplicate calls_this checks idenv curr_pname curr_pdesc
annotated_signature linereader proc_loc : bool * TypeState.t option = annotated_signature linereader proc_loc : bool * TypeState.t option =
let mk s = Pvar.mk s curr_pname in let add_formal typestate (param_signature : AnnotatedSignature.param_signature) =
let add_formal typestate (s, ia, NullsafeType.{typ}) = let pvar = Pvar.mk param_signature.mangled curr_pname in
let pvar = mk s in
let ta = let ta =
let origin = TypeOrigin.Formal s in let origin = TypeOrigin.Formal param_signature.mangled in
TypeAnnotation.from_item_annotation ia origin TypeAnnotation.from_item_annotation param_signature.param_annotation_deprecated origin
in in
TypeState.add pvar (typ, ta, []) typestate TypeState.add pvar (param_signature.param_nullsafe_type.typ, ta, []) typestate
in in
let get_initial_typestate () = let get_initial_typestate () =
let typestate_empty = TypeState.empty in let typestate_empty = TypeState.empty in
@ -48,7 +47,7 @@ module MkCallback (Extension : ExtensionT) : CallBackT = struct
(* Check the nullable flag computed for the return value and report inconsistencies. *) (* Check the nullable flag computed for the return value and report inconsistencies. *)
let check_return find_canonical_duplicate exit_node final_typestate annotated_signature loc : let check_return find_canonical_duplicate exit_node final_typestate annotated_signature loc :
unit = unit =
let _, ret_nullsafe_type = annotated_signature.AnnotatedSignature.ret in let AnnotatedSignature.{ret_nullsafe_type} = annotated_signature.AnnotatedSignature.ret in
let ret_pvar = Procdesc.get_ret_var curr_pdesc in let ret_pvar = Procdesc.get_ret_var curr_pdesc in
let ret_range = TypeState.lookup_pvar ret_pvar final_typestate in let ret_range = TypeState.lookup_pvar ret_pvar final_typestate in
let typ_found_opt = let typ_found_opt =
@ -251,8 +250,9 @@ module MkCallback (Extension : ExtensionT) : CallBackT = struct
(let is_initializer proc_attributes = (let is_initializer proc_attributes =
PatternMatch.method_is_initializer tenv proc_attributes PatternMatch.method_is_initializer tenv proc_attributes
|| ||
let ia, _ = let ia =
(Models.get_modelled_annotated_signature proc_attributes).AnnotatedSignature.ret (Models.get_modelled_annotated_signature proc_attributes).AnnotatedSignature.ret
.ret_annotation_deprecated
in in
Annotations.ia_is_initializer ia Annotations.ia_is_initializer ia
in in

34
infer/src/nullsafe/eradicateChecks.ml
Unescape View File

@ -37,7 +37,12 @@ let explain_expr tenv node e =
None None
let is_virtual = function (p, _, _) :: _ when Mangled.is_this p -> true | _ -> false let is_virtual = function
| AnnotatedSignature.{mangled} :: _ when Mangled.is_this mangled ->
true
| _ ->
false
(** Check an access (read or write) to a field. *) (** Check an access (read or write) to a field. *)
let check_field_access tenv find_canonical_duplicate curr_pname node instr_ref exp fname ta loc : let check_field_access tenv find_canonical_duplicate curr_pname node instr_ref exp fname ta loc :
@ -155,8 +160,10 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r
false false
in in
let field_is_in_cleanup_context () = let field_is_in_cleanup_context () =
let ia, _ = (Models.get_modelled_annotated_signature curr_pattrs).AnnotatedSignature.ret in let AnnotatedSignature.{ret_annotation_deprecated} =
Annotations.ia_is_cleanup ia (Models.get_modelled_annotated_signature curr_pattrs).ret
in
Annotations.ia_is_cleanup ret_annotation_deprecated
in in
let should_report_nullable = let should_report_nullable =
(not (AndroidFramework.is_destroy_method curr_pname)) (not (AndroidFramework.is_destroy_method curr_pname))
@ -256,12 +263,13 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_pname cu
(** Check the annotations when returning from a method. *) (** Check the annotations when returning from a method. *)
let check_return_annotation tenv find_canonical_duplicate curr_pdesc ret_range let check_return_annotation tenv find_canonical_duplicate curr_pdesc ret_range
(annotated_signature : AnnotatedSignature.t) ret_implicitly_nullable loc : unit = (annotated_signature : AnnotatedSignature.t) ret_implicitly_nullable loc : unit =
let ret_ia, _ = annotated_signature.ret in let AnnotatedSignature.{ret_annotation_deprecated} = annotated_signature.ret in
let curr_pname = Procdesc.get_proc_name curr_pdesc in let curr_pname = Procdesc.get_proc_name curr_pdesc in
let ret_annotated_nullable = let ret_annotated_nullable =
Annotations.ia_is_nullable ret_ia Annotations.ia_is_nullable ret_annotation_deprecated
|| List.exists || List.exists
~f:(fun (_, ia, _) -> Annotations.ia_is_propagates_nullable ia) ~f:(fun AnnotatedSignature.{param_annotation_deprecated} ->
Annotations.ia_is_propagates_nullable param_annotation_deprecated )
annotated_signature.params annotated_signature.params
in in
match ret_range with match ret_range with
@ -369,20 +377,20 @@ let check_overridden_annotations find_canonical_duplicate tenv proc_name proc_de
let loc = Procdesc.Node.get_loc start_node in let loc = Procdesc.Node.get_loc start_node in
let check_return overriden_proc_name overriden_signature = let check_return overriden_proc_name overriden_signature =
let ret_is_nullable = let ret_is_nullable =
let ia, _ = annotated_signature.AnnotatedSignature.ret in Annotations.ia_is_nullable
Annotations.ia_is_nullable ia annotated_signature.AnnotatedSignature.ret.ret_annotation_deprecated
and ret_overridden_nullable = and ret_overridden_nullable =
let overriden_ia, _ = overriden_signature.AnnotatedSignature.ret in Annotations.ia_is_nullable
Annotations.ia_is_nullable overriden_ia overriden_signature.AnnotatedSignature.ret.ret_annotation_deprecated
in in
if ret_is_nullable && not ret_overridden_nullable then if ret_is_nullable && not ret_overridden_nullable then
report_error tenv find_canonical_duplicate report_error tenv find_canonical_duplicate
(TypeErr.Inconsistent_subclass_return_annotation (proc_name, overriden_proc_name)) (TypeErr.Inconsistent_subclass_return_annotation (proc_name, overriden_proc_name))
None loc proc_desc None loc proc_desc
and check_params overriden_proc_name overriden_signature = and check_params overriden_proc_name overriden_signature =
let compare pos current_param overriden_param : int = let compare pos
let current_name, current_ia, _ = current_param in AnnotatedSignature.{mangled= current_name; param_annotation_deprecated= current_ia}
let _, overriden_ia, _ = overriden_param in AnnotatedSignature.{param_annotation_deprecated= overriden_ia} =
let () = let () =
if (not (Annotations.ia_is_nullable current_ia)) && Annotations.ia_is_nullable overriden_ia if (not (Annotations.ia_is_nullable current_ia)) && Annotations.ia_is_nullable overriden_ia
then then

37
infer/src/nullsafe/typeCheck.ml
Unescape View File

@ -19,8 +19,8 @@ module ComplexExpressions = struct
match PatternMatch.lookup_attributes tenv pn with match PatternMatch.lookup_attributes tenv pn with
| Some proc_attributes -> | Some proc_attributes ->
let annotated_signature = Models.get_modelled_annotated_signature proc_attributes in let annotated_signature = Models.get_modelled_annotated_signature proc_attributes in
let ret_ann, _ = annotated_signature.AnnotatedSignature.ret in let AnnotatedSignature.{ret_annotation_deprecated} = annotated_signature.ret in
Annotations.ia_is_false_on_null ret_ann Annotations.ia_is_false_on_null ret_annotation_deprecated
| None -> | None ->
false false
@ -30,8 +30,8 @@ module ComplexExpressions = struct
match PatternMatch.lookup_attributes tenv pn with match PatternMatch.lookup_attributes tenv pn with
| Some proc_attributes -> | Some proc_attributes ->
let annotated_signature = Models.get_modelled_annotated_signature proc_attributes in let annotated_signature = Models.get_modelled_annotated_signature proc_attributes in
let ret_ann, _ = annotated_signature.AnnotatedSignature.ret in let AnnotatedSignature.{ret_annotation_deprecated} = annotated_signature.ret in
Annotations.ia_is_true_on_null ret_ann Annotations.ia_is_true_on_null ret_annotation_deprecated
| None -> | None ->
false false
in in
@ -289,7 +289,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p
let is_parameter_field pvar = let is_parameter_field pvar =
(* parameter.field *) (* parameter.field *)
let name = Pvar.get_name pvar in let name = Pvar.get_name pvar in
let filter (s, _, _) = Mangled.equal s name in let filter AnnotatedSignature.{mangled} = Mangled.equal mangled name in
List.exists ~f:filter curr_annotated_signature.AnnotatedSignature.params List.exists ~f:filter curr_annotated_signature.AnnotatedSignature.params
in in
let is_static_field pvar = let is_static_field pvar =
@ -684,19 +684,26 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p
in in
let typestate_after_call, resolved_ret = let typestate_after_call, resolved_ret =
let resolve_param i (sparam, cparam) = let resolve_param i (sparam, cparam) =
let s1, ia1, NullsafeType.{typ= t1} = sparam in let AnnotatedSignature.{mangled; param_annotation_deprecated; param_nullsafe_type} =
sparam
in
let (orig_e2, e2), t2 = cparam in let (orig_e2, e2), t2 = cparam in
let ta1 = TypeAnnotation.from_item_annotation ia1 (TypeOrigin.Formal s1) in let ta1 =
TypeAnnotation.from_item_annotation param_annotation_deprecated
(TypeOrigin.Formal mangled)
in
let _, ta2, _ = let _, ta2, _ =
typecheck_expr find_canonical_duplicate calls_this checks tenv node instr_ref typecheck_expr find_canonical_duplicate calls_this checks tenv node instr_ref
curr_pdesc typestate e2 curr_pdesc typestate e2
(t2, TypeAnnotation.const_nullable false TypeOrigin.ONone, []) (t2, TypeAnnotation.const_nullable false TypeOrigin.ONone, [])
loc loc
in in
let formal = (s1, ta1, t1) in let formal = (mangled, ta1, param_nullsafe_type.typ) in
let actual = (orig_e2, ta2) in let actual = (orig_e2, ta2) in
let num = i + 1 in let num = i + 1 in
let formal_is_propagates_nullable = Annotations.ia_is_propagates_nullable ia1 in let formal_is_propagates_nullable =
Annotations.ia_is_propagates_nullable param_annotation_deprecated
in
let actual_is_nullable = TypeAnnotation.is_nullable ta2 in let actual_is_nullable = TypeAnnotation.is_nullable ta2 in
let propagates_nullable = formal_is_propagates_nullable && actual_is_nullable in let propagates_nullable = formal_is_propagates_nullable && actual_is_nullable in
EradicateChecks.{num; formal; actual; propagates_nullable} EradicateChecks.{num; formal; actual; propagates_nullable}
@ -727,7 +734,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p
handle_params resolved_ret resolved_params handle_params resolved_ret resolved_params
in in
let resolved_ret_ = let resolved_ret_ =
let ret_ia, ret_typ = callee_annotated_signature.AnnotatedSignature.ret in let ret = callee_annotated_signature.AnnotatedSignature.ret in
let is_library = Summary.OnDisk.proc_is_library callee_attributes in let is_library = Summary.OnDisk.proc_is_library callee_attributes in
let origin = let origin =
TypeOrigin.Proc TypeOrigin.Proc
@ -736,8 +743,8 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p
; annotated_signature= callee_annotated_signature ; annotated_signature= callee_annotated_signature
; is_library } ; is_library }
in in
let ret_ta = TypeAnnotation.from_item_annotation ret_ia origin in let ret_ta = TypeAnnotation.from_item_annotation ret.ret_annotation_deprecated origin in
(ret_ta, ret_typ) (ret_ta, ret.ret_nullsafe_type)
in in
let sig_len = List.length signature_params in let sig_len = List.length signature_params in
let call_len = List.length call_params in let call_len = List.length call_params in
@ -750,10 +757,10 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p
let call_slice = slice call_params in let call_slice = slice call_params in
let sig_call_params = let sig_call_params =
List.filter List.filter
~f:(fun (sparam, _) -> ~f:(fun ((sparam : AnnotatedSignature.param_signature), _) ->
let s, _, _ = sparam in
let param_is_this = let param_is_this =
Mangled.is_this s || String.is_prefix ~prefix:"this$" (Mangled.to_string s) Mangled.is_this sparam.mangled
|| String.is_prefix ~prefix:"this$" (Mangled.to_string sparam.mangled)
in in
not param_is_this ) not param_is_this )
(List.zip_exn sig_slice call_slice) (List.zip_exn sig_slice call_slice)

5
infer/src/nullsafe/typeErr.ml
Unescape View File

@ -178,8 +178,9 @@ module Severity = struct
let this_type_get_severity tenv (signature : AnnotatedSignature.t) = let this_type_get_severity tenv (signature : AnnotatedSignature.t) =
match signature.params with match signature.params with
| (p, _, NullsafeType.{typ= this_type}) :: _ when Mangled.is_this p -> | AnnotatedSignature.{mangled; param_nullsafe_type} :: _ when Mangled.is_this mangled ->
Option.bind ~f:get_severity (PatternMatch.type_get_annotation tenv this_type) (* TODO(T54088319) get rid of direct access to annotation *)
Option.bind ~f:get_severity (PatternMatch.type_get_annotation tenv param_nullsafe_type.typ)
| _ -> | _ ->
None None

Write Preview
Loading…
Cancel
Save