From 42397d2168aedb3a0b22ba790b63b5d16f17b098 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Thu, 7 Nov 2019 08:46:14 -0800 Subject: [PATCH] [nullsafe] Respect external models when decide wether to report on passing wrong param or not Summary: This is a dirty hack to make nullsafe behave consistently for internal and external models. Medium term this code needs to be rewritten in a better way, so that we pass all information in annotated signature (either in form of Inferred nullability, or type origin, which tracks third party calls). As a follow up, we will make reporting more clear. Reviewed By: artempyanykh Differential Revision: D18372660 fbshipit-source-id: 12c2449e1 --- infer/src/nullsafe/eradicateChecks.ml | 3 ++- infer/src/nullsafe/models.ml | 18 +++++++++++++++++- infer/src/nullsafe/typeOrigin.ml | 3 ++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 5441c8daa..622afef30 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -443,7 +443,8 @@ let check_call_parameters ~is_strict_mode tenv find_canonical_duplicate curr_pde (* TODO(T52947663) model is_external as unknown nullability and move the logic out of this check *) (* If method is external, we don't check it, unless it is explicitly modelled *) (not (Typ.Procname.Java.is_external java_pname)) - || Models.is_modelled_for_nullability callee_pname + || Models.is_modelled_for_nullability_as_internal callee_pname + || Models.is_modelled_for_nullability_as_external callee_pname | _ -> false in diff --git a/infer/src/nullsafe/models.ml b/infer/src/nullsafe/models.ml index f164ca654..0f2e382a8 100644 --- a/infer/src/nullsafe/models.ml +++ b/infer/src/nullsafe/models.ml @@ -96,7 +96,10 @@ let get_modelled_annotated_signature tenv proc_attributes = (** Return true when the procedure has been modelled for nullability. *) -let is_modelled_for_nullability proc_name = +let is_modelled_for_nullability_as_internal proc_name = + (* TODO: get rid of this function, and propagate this information in get_modelled_annotated_signature instead + to avoid double calculation and make the code more clear. + *) let proc_id = Typ.Procname.to_unique_id proc_name in try ignore (Hashtbl.find annotated_table_nullability proc_id) ; @@ -104,6 +107,19 @@ let is_modelled_for_nullability proc_name = with Caml.Not_found -> false +(** Return true when the procedure has been modelled for nullability as external third-party code. *) +let is_modelled_for_nullability_as_external proc_name = + (* TODO: get rid of this function, and propagate this information in get_modelled_annotated_signature instead + to avoid double calculation and make the code more clear. + *) + get_unique_repr proc_name + |> Option.map + ~f: + (ThirdPartyAnnotationInfo.find_nullability_info + (ThirdPartyAnnotationGlobalRepo.get_repo ())) + |> Option.is_some + + (** Check if the procedure is one of the known Preconditions.checkNotNull. *) let is_check_not_null proc_name = table_has_procedure check_not_null_table proc_name || match_method_name proc_name "checkNotNull" diff --git a/infer/src/nullsafe/typeOrigin.ml b/infer/src/nullsafe/typeOrigin.ml index 2f1a6fe67..b2ae08666 100644 --- a/infer/src/nullsafe/typeOrigin.ml +++ b/infer/src/nullsafe/typeOrigin.ml @@ -57,7 +57,8 @@ let get_description origin = | Proc po -> let modelled_in = (* TODO(T54088319) don't calculate this info and propagate it from AnnotatedNullability instead *) - if Models.is_modelled_for_nullability po.pname then " modelled in " ^ ModelTables.this_file + if Models.is_modelled_for_nullability_as_internal po.pname then + " modelled in " ^ ModelTables.this_file else "" in let description =