From 66119352a2cffe73fd7f6613ce1ce48f6d596b45 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Wed, 13 Nov 2019 10:14:58 -0800 Subject: [PATCH] [nullsafe][TypeOrigin refactor] Massage TypeOrigin.Field and TypeOrigin.Proc Summary: This diff is a part of work teaching Nullsafe to explain decisions it's making. 1. Make Field origin a record: consistent with Proc; more readable; will add more elements in follow up diffs. 2. `Proc` is bit ambiguous: it is unclear if this is a method call result or param. Let's make it clear that it is the result of a method call. Reviewed By: ngorogiannis Differential Revision: D18450188 fbshipit-source-id: c5abae3ad --- infer/src/nullsafe/InferredNullability.ml | 2 +- infer/src/nullsafe/eradicateChecks.ml | 2 +- infer/src/nullsafe/typeCheck.ml | 15 +++---- infer/src/nullsafe/typeOrigin.ml | 51 +++++++++++++---------- infer/src/nullsafe/typeOrigin.mli | 23 +++++----- 5 files changed, 53 insertions(+), 40 deletions(-) diff --git a/infer/src/nullsafe/InferredNullability.ml b/infer/src/nullsafe/InferredNullability.ml index ffa3c3316..07374a9ec 100644 --- a/infer/src/nullsafe/InferredNullability.ml +++ b/infer/src/nullsafe/InferredNullability.ml @@ -63,7 +63,7 @@ let get_origin t = t.origin let origin_is_fun_library t = match get_origin t with - | TypeOrigin.Proc proc_origin -> + | TypeOrigin.MethodCall proc_origin -> proc_origin.TypeOrigin.is_library | _ -> false diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 672bb9035..28ef017e3 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -255,7 +255,7 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_construc | TypeOrigin.Undef -> (* Special case: not really an initialization *) false - | TypeOrigin.Field (TypeOrigin.Formal name, _, _) -> + | TypeOrigin.Field {object_origin= TypeOrigin.Formal name} -> (* Exclude circular initialization *) let circular = Mangled.is_this name in not circular diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index 2b4effab9..f1bff3231 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -142,13 +142,13 @@ let rec typecheck_expr ~is_strict_mode find_canonical_duplicate visited checks t (typ, InferredNullability.create_nonnull TypeOrigin.ONone) loc in - let exp_origin = InferredNullability.get_origin inferred_nullability in + let object_origin = InferredNullability.get_origin inferred_nullability in let tr_new = match AnnotatedField.get tenv field_name typ with | Some AnnotatedField.{annotated_type} -> ( annotated_type.typ , InferredNullability.of_annotated_nullability annotated_type.nullability - (TypeOrigin.Field (exp_origin, field_name, loc)) ) + (TypeOrigin.Field {object_origin; field_name; access_loc= loc}) ) | None -> tr_default in @@ -239,17 +239,18 @@ let handle_function_call tenv curr_pname typestate exp default ~is_assignment ~c (* If this is an assignment, update the typestate for a field access pvar. *) -let update_typestate_fld ~is_assignment tenv loc typestate pvar origin fn typ = +let update_typestate_fld ~is_assignment tenv access_loc typestate pvar object_origin field_name typ + = match TypeState.lookup_pvar pvar typestate with | Some _ when not is_assignment -> typestate | _ -> ( - match AnnotatedField.get tenv fn typ with + match AnnotatedField.get tenv field_name typ with | Some AnnotatedField.{annotated_type} -> let range = ( annotated_type.typ , InferredNullability.of_annotated_nullability annotated_type.nullability - (TypeOrigin.Field (origin, fn, loc)) ) + (TypeOrigin.Field {object_origin; field_name; access_loc}) ) in TypeState.add pvar range typestate | None -> @@ -862,9 +863,9 @@ let calc_typestate_after_call find_canonical_duplicate calls_this checks tenv id let ret = callee_annotated_signature.AnnotatedSignature.ret in let is_library = Summary.OnDisk.proc_is_library callee_attributes in let origin = - TypeOrigin.Proc + TypeOrigin.MethodCall { TypeOrigin.pname= callee_pname - ; loc + ; call_loc= loc ; annotated_signature= callee_annotated_signature ; is_library } in diff --git a/infer/src/nullsafe/typeOrigin.ml b/infer/src/nullsafe/typeOrigin.ml index 9b96551d3..bee5c1896 100644 --- a/infer/src/nullsafe/typeOrigin.ml +++ b/infer/src/nullsafe/typeOrigin.ml @@ -9,25 +9,29 @@ open! IStd (** Describe the origin of values propagated by the checker. *) -type proc_origin = - { pname: Typ.Procname.t - ; loc: Location.t - ; annotated_signature: AnnotatedSignature.t - ; is_library: bool } -[@@deriving compare] - type t = | NullConst of Location.t (** A null literal in the source *) | NonnullConst of Location.t (** A constant (not equal to null) in the source. *) - | Field of t * Typ.Fieldname.t * Location.t (** A field access *) + | Field of field_origin (** A field access (result of expression `some_object.some_field`) *) | Formal of Mangled.t (** A formal parameter *) - | Proc of proc_origin (** A procedure call *) + | MethodCall of method_call_origin (** A result of a method call *) | New (** A new object creation *) | ArrayLengthResult (** integer value - result of accessing array.length *) | ONone (** No origin is known *) | Undef (** Undefined value before initialization *) [@@deriving compare] +and field_origin = + { object_origin: t (** field's object origin (object is before field access operator `.`) *) + ; field_name: Typ.Fieldname.t + ; access_loc: Location.t } + +and method_call_origin = + { pname: Typ.Procname.t + ; call_loc: Location.t + ; annotated_signature: AnnotatedSignature.t + ; is_library: bool } + let equal = [%compare.equal: t] let rec to_string = function @@ -35,12 +39,14 @@ let rec to_string = function "null" | NonnullConst _ -> "Const (nonnull)" - | Field (o, fn, _) -> - "Field " ^ Typ.Fieldname.to_simplified_string fn ^ " (inner: " ^ to_string o ^ ")" + | Field {object_origin; field_name} -> + "Field " + ^ Typ.Fieldname.to_simplified_string field_name + ^ " (object: " ^ to_string object_origin ^ ")" | Formal s -> "Formal " ^ Mangled.to_string s - | Proc po -> - Printf.sprintf "Fun %s" (Typ.Procname.to_simplified_string po.pname) + | MethodCall {pname} -> + Printf.sprintf "Fun %s" (Typ.Procname.to_simplified_string pname) | New -> "New" | ArrayLengthResult -> @@ -56,23 +62,26 @@ let get_description origin = match origin with | NullConst loc -> Some ("null constant" ^ atline loc, Some loc, None) - | Field (_, fn, loc) -> - Some ("field " ^ Typ.Fieldname.to_simplified_string fn ^ atline loc, Some loc, None) + | Field {field_name; access_loc} -> + Some + ( "field " ^ Typ.Fieldname.to_simplified_string field_name ^ atline access_loc + , Some access_loc + , None ) | Formal s -> Some ("method parameter " ^ Mangled.to_string s, None, None) - | Proc po -> + | MethodCall {pname; call_loc; annotated_signature} -> let modelled_in = (* TODO(T54088319) don't calculate this info and propagate it from AnnotatedNullability instead *) - if Models.is_modelled_for_nullability_as_internal po.pname then + if Models.is_modelled_for_nullability_as_internal pname then " modelled in " ^ ModelTables.this_file else "" in let description = Printf.sprintf "call to %s%s%s" - (Typ.Procname.to_simplified_string po.pname) - modelled_in (atline po.loc) + (Typ.Procname.to_simplified_string pname) + modelled_in (atline call_loc) in - Some (description, Some po.loc, Some po.annotated_signature) + Some (description, Some call_loc, Some annotated_signature) (* These are origins of non-nullable expressions that are result of evaluating of some rvalue. Because they are non-nullable and they are rvalues, we won't get normal type violations With them. All we could get is things like condition redundant or overannotated. @@ -91,7 +100,7 @@ let join o1 o2 = (* left priority *) | Undef, _ | _, Undef -> Undef - | Field _, (NullConst _ | NonnullConst _ | Formal _ | Proc _ | New) -> + | Field _, (NullConst _ | NonnullConst _ | Formal _ | MethodCall _ | New) -> (* low priority to Field, to support field initialization patterns *) o2 | _ -> diff --git a/infer/src/nullsafe/typeOrigin.mli b/infer/src/nullsafe/typeOrigin.mli index 8d7d27815..606993022 100644 --- a/infer/src/nullsafe/typeOrigin.mli +++ b/infer/src/nullsafe/typeOrigin.mli @@ -7,26 +7,29 @@ open! IStd -(** Case Proc *) -type proc_origin = - { pname: Typ.Procname.t - ; loc: Location.t - ; annotated_signature: AnnotatedSignature.t - ; is_library: bool } -[@@deriving compare] - type t = | NullConst of Location.t (** A null literal in the source *) | NonnullConst of Location.t (** A constant (not equal to null) in the source. *) - | Field of t * Typ.Fieldname.t * Location.t (** A field access *) + | Field of field_origin (** A field access (result of expression `some_object.some_field`) *) | Formal of Mangled.t (** A formal parameter *) - | Proc of proc_origin (** A procedure call *) + | MethodCall of method_call_origin (** A result of a method call *) | New (** A new object creation *) | ArrayLengthResult (** integer value - result of accessing array.length *) | ONone (** No origin is known *) | Undef (** Undefined value before initialization *) [@@deriving compare] +and field_origin = + { object_origin: t (** field's object origin (object is before field access operator `.`) *) + ; field_name: Typ.Fieldname.t + ; access_loc: Location.t } + +and method_call_origin = + { pname: Typ.Procname.t + ; call_loc: Location.t + ; annotated_signature: AnnotatedSignature.t + ; is_library: bool } + val equal : t -> t -> bool val get_description : t -> TypeErr.origin_descr option