From 57fce4315a14aacbd38574b53b52a76bee59fa45 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Mon, 4 May 2020 08:14:03 -0700 Subject: [PATCH] [nullsafe] kill Summary.OnDisk.proc_is_library Summary: That function was testing if a proc is defined (ok) and if so if it also had no summary. The second check is wrong because it could be that the function hasn't been analysed yet. We should call "Ondemand" instead. But, it seems like a weird check so I just deleted it and replaced is with just the "is defined" check. Renamed the "is_library" field in typeOrigin to "is_defined" to better reflect the expectations. While I was at it, inlined record type definitions in `TypeOrigin.t`. They were already inlined except for 2! Reviewed By: dulmarod Differential Revision: D21351453 fbshipit-source-id: 1acc7ff90 --- infer/src/backend/Summary.ml | 7 ------ infer/src/backend/Summary.mli | 3 --- infer/src/nullsafe/InferredNullability.ml | 8 ++---- infer/src/nullsafe/InferredNullability.mli | 2 +- infer/src/nullsafe/eradicateChecks.ml | 2 +- infer/src/nullsafe/typeCheck.ml | 8 +++--- infer/src/nullsafe/typeOrigin.ml | 29 ++++++++++------------ infer/src/nullsafe/typeOrigin.mli | 29 ++++++++++------------ 8 files changed, 34 insertions(+), 54 deletions(-) diff --git a/infer/src/backend/Summary.ml b/infer/src/backend/Summary.ml index 6f8775f43..5e1e36576 100644 --- a/infer/src/backend/Summary.ml +++ b/infer/src/backend/Summary.ml @@ -180,13 +180,6 @@ module OnDisk = struct load_summary_to_spec_table proc_name - (** Check if the procedure is from a library: It's not defined, and there is no spec file for it. *) - let proc_is_library proc_attributes = - if not proc_attributes.ProcAttributes.is_defined then - match get proc_attributes.ProcAttributes.proc_name with None -> true | Some _ -> false - else false - - (** Try to find the attributes for a defined proc. First look at specs (to get attributes computed by analysis) then look at the attributes table. If no attributes can be found, return None. *) let proc_resolve_attributes proc_name = diff --git a/infer/src/backend/Summary.mli b/infer/src/backend/Summary.mli index 3373abeb9..7ef1c56c6 100644 --- a/infer/src/backend/Summary.mli +++ b/infer/src/backend/Summary.mli @@ -88,9 +88,6 @@ module OnDisk : sig (** Try to find the attributes for a defined proc. First look at specs (to get attributes computed by analysis) then look at the attributes table. If no attributes can be found, return None. *) - val proc_is_library : ProcAttributes.t -> bool - (** Check if the procedure is from a library: It's not defined, and there is no spec file for it. *) - val store : t -> unit (** Save summary for the procedure into the spec database *) diff --git a/infer/src/nullsafe/InferredNullability.ml b/infer/src/nullsafe/InferredNullability.ml index 8fd04b339..e8f017efa 100644 --- a/infer/src/nullsafe/InferredNullability.ml +++ b/infer/src/nullsafe/InferredNullability.ml @@ -48,9 +48,5 @@ let join t1 t2 = let get_origin t = t.origin -let origin_is_fun_library t = - match get_origin t with - | TypeOrigin.MethodCall proc_origin -> - proc_origin.TypeOrigin.is_library - | _ -> - false +let origin_is_fun_defined t = + match get_origin t with TypeOrigin.MethodCall {is_defined; _} -> is_defined | _ -> false diff --git a/infer/src/nullsafe/InferredNullability.mli b/infer/src/nullsafe/InferredNullability.mli index e09c0afb4..2690ece70 100644 --- a/infer/src/nullsafe/InferredNullability.mli +++ b/infer/src/nullsafe/InferredNullability.mli @@ -38,6 +38,6 @@ val join : t -> t -> t // what is nullability of `a` at this point? ]} *) -val origin_is_fun_library : t -> bool +val origin_is_fun_defined : t -> bool val pp : Format.formatter -> t -> unit diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index f27da0c4b..e2c157604 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -102,7 +102,7 @@ let check_condition_for_redundancy tenv ~is_always_true find_canonical_duplicate InferredNullability.is_nonnullish inferred_nullability && Config.eradicate_condition_redundant && (not is_temp) && PatternMatch.type_is_class typ && (not (from_try_with_resources ())) - && not (InferredNullability.origin_is_fun_library inferred_nullability) + && not (InferredNullability.origin_is_fun_defined inferred_nullability) in if should_report then (* TODO(T61051649) this is not really condition. This is an expression. diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index b85254ad8..585c1d37b 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -510,7 +510,7 @@ let do_preconditions_check_not_null instr_ref tenv find_canonical_duplicate node Config.eradicate_condition_redundant (* TODO: This condition should be extracted into a dedicated rule *) && InferredNullability.is_nonnullish nullability - && not (InferredNullability.origin_is_fun_library nullability) + && not (InferredNullability.origin_is_fun_defined nullability) in ( if checks.eradicate && should_report then let cond = Exp.BinOp (Binop.Ne, Exp.Lvar pvar, Exp.null) in @@ -973,13 +973,13 @@ let calc_typestate_after_call find_canonical_duplicate calls_this checks tenv id (* Infer nullability of function call result based on its signature *) let preliminary_resolved_ret = let ret = callee_annotated_signature.AnnotatedSignature.ret in - let is_library = Summary.OnDisk.proc_is_library callee_attributes in + let is_defined = not callee_attributes.ProcAttributes.is_defined in let origin = TypeOrigin.MethodCall - { TypeOrigin.pname= callee_pname + { pname= callee_pname ; call_loc= loc ; annotated_signature= callee_annotated_signature - ; is_library } + ; is_defined } in (InferredNullability.create origin, ret.ret_annotated_type.typ) in diff --git a/infer/src/nullsafe/typeOrigin.ml b/infer/src/nullsafe/typeOrigin.ml index c3083537a..e9710ccc7 100644 --- a/infer/src/nullsafe/typeOrigin.ml +++ b/infer/src/nullsafe/typeOrigin.ml @@ -9,13 +9,24 @@ open! IStd (** Describe the origin of values propagated by the checker. *) +type method_parameter_origin = Normal of AnnotatedSignature.param_signature | ObjectEqualsOverride +[@@deriving compare] + type t = | NullConst of Location.t | NonnullConst of Location.t - | Field of field_origin + | Field of + { object_origin: t (** field's object origin (object is before field access operator `.`) *) + ; field_name: Fieldname.t + ; field_type: AnnotatedType.t + ; access_loc: Location.t } | CurrMethodParameter of method_parameter_origin | This - | MethodCall of method_call_origin + | MethodCall of + { pname: Procname.t + ; call_loc: Location.t + ; annotated_signature: AnnotatedSignature.t + ; is_defined: bool } | CallToGetKnownToContainsKey | New | ArrayLengthResult @@ -24,20 +35,6 @@ type t = | OptimisticFallback [@@deriving compare] -and method_parameter_origin = Normal of AnnotatedSignature.param_signature | ObjectEqualsOverride - -and field_origin = - { object_origin: t (** field's object origin (object is before field access operator `.`) *) - ; field_name: Fieldname.t - ; field_type: AnnotatedType.t - ; access_loc: Location.t } - -and method_call_origin = - { pname: Procname.t - ; call_loc: Location.t - ; annotated_signature: AnnotatedSignature.t - ; is_library: bool } - let get_nullability = function | NullConst _ -> Nullability.Null diff --git a/infer/src/nullsafe/typeOrigin.mli b/infer/src/nullsafe/typeOrigin.mli index d0a2d504d..f1611a35a 100644 --- a/infer/src/nullsafe/typeOrigin.mli +++ b/infer/src/nullsafe/typeOrigin.mli @@ -7,14 +7,25 @@ open! IStd +type method_parameter_origin = Normal of AnnotatedSignature.param_signature | ObjectEqualsOverride + 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 field_origin (** A field access (result of expression `some_object.some_field`) *) + | Field of + { object_origin: t (** field's object origin (object is before field access operator `.`) *) + ; field_name: Fieldname.t + ; field_type: AnnotatedType.t + ; access_loc: Location.t } + (** A field access (result of expression `some_object.some_field`) *) | CurrMethodParameter of method_parameter_origin (** Parameter of a method we are currently in, *) | This (* `this` object. Can not be null, according to Java rules. *) - | MethodCall of method_call_origin (** A result of a method call *) + | MethodCall of + { pname: Procname.t + ; call_loc: Location.t + ; annotated_signature: AnnotatedSignature.t + ; is_defined: bool } (** A result of a method call *) | CallToGetKnownToContainsKey (** This is a result of accessing a map element that is known to contains this particular key, normally because it was explicitly checked for presense before *) @@ -33,20 +44,6 @@ type t = T54687014 tracks unsoundness issues caused by this type. *) [@@deriving compare] -and method_parameter_origin = Normal of AnnotatedSignature.param_signature | ObjectEqualsOverride - -and field_origin = - { object_origin: t (** field's object origin (object is before field access operator `.`) *) - ; field_name: Fieldname.t - ; field_type: AnnotatedType.t - ; access_loc: Location.t } - -and method_call_origin = - { pname: Procname.t - ; call_loc: Location.t - ; annotated_signature: AnnotatedSignature.t - ; is_library: bool } - val get_nullability : t -> Nullability.t val get_description : t -> string option