From 3fa8c56ce2d64a17fc344390b90bd8770f7d3a74 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Wed, 12 Aug 2020 09:28:59 -0700 Subject: [PATCH] [nullsafe][refactor] Convert models.ml to Procname.Java.t Summary: This is part of work aimed to reduce usage of language-agnostics modules in Java-specific parts of nullsafe. As usual, in this diff we don't convert everything and take some shorthands. Reviewed By: ngorogiannis Differential Revision: D23054169 fbshipit-source-id: 70913ddfd --- infer/src/nullsafe/AnnotatedSignature.ml | 4 ++ infer/src/nullsafe/AssignmentRule.ml | 2 +- infer/src/nullsafe/ErrorRenderingUtils.ml | 2 +- infer/src/nullsafe/NullsafeMode.ml | 18 ++++--- infer/src/nullsafe/NullsafeMode.mli | 7 ++- .../src/nullsafe/ThirdPartyAnnotationInfo.ml | 18 +------ .../src/nullsafe/ThirdPartyAnnotationInfo.mli | 8 +-- infer/src/nullsafe/eradicate.ml | 2 +- infer/src/nullsafe/models.ml | 53 ++++++++----------- infer/src/nullsafe/typeCheck.ml | 39 +++++++------- 10 files changed, 71 insertions(+), 82 deletions(-) diff --git a/infer/src/nullsafe/AnnotatedSignature.ml b/infer/src/nullsafe/AnnotatedSignature.ml index c6295331f..c11c3e103 100644 --- a/infer/src/nullsafe/AnnotatedSignature.ml +++ b/infer/src/nullsafe/AnnotatedSignature.ml @@ -80,6 +80,10 @@ let extract_nullability ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party let get ~is_callee_in_trust_list ~nullsafe_mode ( {ProcAttributes.proc_name; ret_type; method_annotation= {return= ret_annotation}} as proc_attributes ) : t = + let proc_name = + Procname.as_java_exn ~explanation:"AnnotatedSignature.get:: should call only for Java methods" + proc_name + in let is_third_party = ThirdPartyAnnotationInfo.is_third_party_proc (ThirdPartyAnnotationGlobalRepo.get_repo ()) diff --git a/infer/src/nullsafe/AssignmentRule.ml b/infer/src/nullsafe/AssignmentRule.ml index 42030f38c..761e45284 100644 --- a/infer/src/nullsafe/AssignmentRule.ml +++ b/infer/src/nullsafe/AssignmentRule.ml @@ -94,7 +94,7 @@ module ReportableViolation = struct So we phrase it differently to remain truthful, but as specific as possible. *) let suggested_third_party_sig_file = - ThirdPartyAnnotationInfo.lookup_related_sig_file_for_java_proc + ThirdPartyAnnotationInfo.lookup_related_sig_file_for_proc (ThirdPartyAnnotationGlobalRepo.get_repo ()) function_procname in diff --git a/infer/src/nullsafe/ErrorRenderingUtils.ml b/infer/src/nullsafe/ErrorRenderingUtils.ml index 5afcdc8b5..3e9e83543 100644 --- a/infer/src/nullsafe/ErrorRenderingUtils.ml +++ b/infer/src/nullsafe/ErrorRenderingUtils.ml @@ -163,7 +163,7 @@ let get_info object_origin nullsafe_mode untrusted_kind = match untrusted_kind with | UserFriendlyNullable.ThirdPartyNonnull -> let suggested_third_party_sig_file = - ThirdPartyAnnotationInfo.lookup_related_sig_file_for_java_proc + ThirdPartyAnnotationInfo.lookup_related_sig_file_for_proc (ThirdPartyAnnotationGlobalRepo.get_repo ()) pname in diff --git a/infer/src/nullsafe/NullsafeMode.ml b/infer/src/nullsafe/NullsafeMode.ml index 0d09474f0..fdfc08483 100644 --- a/infer/src/nullsafe/NullsafeMode.ml +++ b/infer/src/nullsafe/NullsafeMode.ml @@ -174,17 +174,19 @@ let of_class tenv class_name = of_class_and_outer_classes user_defined_class -let of_procname tenv pname = - let class_name = - match pname with - | Procname.Java jn -> - Procname.Java.get_class_type_name jn - | _ -> - Logging.die InternalError "Unexpected non-Java procname %a" Procname.pp pname - in +let of_java_procname tenv pname = + let class_name = Procname.Java.get_class_type_name pname in of_class tenv (Typ.Name.Java.get_java_class_name_exn class_name) +let of_procname tenv pname = + match pname with + | Procname.Java jn -> + of_java_procname tenv jn + | _ -> + Logging.die InternalError "Unexpected non-Java procname %a" Procname.pp pname + + let is_stricter_than ~stricter ~weaker = let strict_level mode = match mode with Default -> 0 | Local _ -> 1 | Strict -> 2 in match (stricter, weaker) with diff --git a/infer/src/nullsafe/NullsafeMode.mli b/infer/src/nullsafe/NullsafeMode.mli index 1198b39d2..b9eced049 100644 --- a/infer/src/nullsafe/NullsafeMode.mli +++ b/infer/src/nullsafe/NullsafeMode.mli @@ -37,7 +37,12 @@ val of_class : Tenv.t -> JavaClassName.t -> t (** Extracts mode information from class annotations *) val of_procname : Tenv.t -> Procname.t -> t -(** Extracts mode information from a class where procname is defined *) +(** Extracts mode information from a class where procname is defined. Should be called for Java + procnames only; throws otherwise *) + +val of_java_procname : Tenv.t -> Procname.Java.t -> t +(** Extracts mode information from a class where procname is defined. Should be called for Java + procnames only; throws otherwise *) val is_in_trust_list : t -> JavaClassName.t -> bool (** Check whether [JavaClassName.t] is in explicit trust list specified in the mode *) diff --git a/infer/src/nullsafe/ThirdPartyAnnotationInfo.ml b/infer/src/nullsafe/ThirdPartyAnnotationInfo.ml index 2728b91d1..efe809b8d 100644 --- a/infer/src/nullsafe/ThirdPartyAnnotationInfo.ml +++ b/infer/src/nullsafe/ThirdPartyAnnotationInfo.ml @@ -103,27 +103,13 @@ let lookup_related_sig_file {filenames} ~package = |> List.max_elt ~compare:(fun name1 name2 -> String.length name1 - String.length name2) -let lookup_related_sig_file_for_java_proc storage procname = +let lookup_related_sig_file_for_proc storage procname = let package = Procname.Java.get_package procname in Option.bind package ~f:(fun package -> lookup_related_sig_file storage ~package) -let lookup_related_sig_file_for_proc storage procname = - match procname with - | Procname.Java java_pname -> - lookup_related_sig_file_for_java_proc storage java_pname - | _ -> - None - - let is_third_party_proc storage procname = - let is_from_config = - match procname with - | Procname.Java java_pname -> - Procname.Java.is_external java_pname - | _ -> - false - in + let is_from_config = Procname.Java.is_external procname in let lookup_sig_file _ = lookup_related_sig_file_for_proc storage procname in is_from_config || Option.is_some (lookup_sig_file ()) diff --git a/infer/src/nullsafe/ThirdPartyAnnotationInfo.mli b/infer/src/nullsafe/ThirdPartyAnnotationInfo.mli index 77630e81d..bd9481bfb 100644 --- a/infer/src/nullsafe/ThirdPartyAnnotationInfo.mli +++ b/infer/src/nullsafe/ThirdPartyAnnotationInfo.mli @@ -47,14 +47,10 @@ val find_nullability_info : storage -> unique_repr -> signature_info option val lookup_related_sig_file : storage -> package:string -> string option (** If the package is third-party, return the relevant .sig file to add signatures for this package. *) -val lookup_related_sig_file_for_proc : storage -> Procname.t -> string option - [@@warning "-32"] +val lookup_related_sig_file_for_proc : storage -> Procname.Java.t -> string option (** If the function is third-party (based on its package), return relevant .sig file *) -val lookup_related_sig_file_for_java_proc : storage -> Procname.Java.t -> string option -(** If the function is third-party (based on its package), return relevant .sig file *) - -val is_third_party_proc : storage -> Procname.t -> bool +val is_third_party_proc : storage -> Procname.Java.t -> bool (** Checks whether a required procname comes from third-party code based on available .sig files and config flags. NOTE: considering config flags is done for compatibility with the legacy behaviour and will be removed in the future *) diff --git a/infer/src/nullsafe/eradicate.ml b/infer/src/nullsafe/eradicate.ml index 5f5d7e25c..bc0a90775 100644 --- a/infer/src/nullsafe/eradicate.ml +++ b/infer/src/nullsafe/eradicate.ml @@ -189,7 +189,7 @@ let find_reason_to_skip_analysis proc_name proc_desc = else if ThirdPartyAnnotationInfo.is_third_party_proc (ThirdPartyAnnotationGlobalRepo.get_repo ()) - proc_name + java_pname then Some "third party method" else if (Procdesc.get_attributes proc_desc).ProcAttributes.is_bridge_method then Some "bridge method" diff --git a/infer/src/nullsafe/models.ml b/infer/src/nullsafe/models.ml index 4f9ed6723..dc06c7d36 100644 --- a/infer/src/nullsafe/models.ml +++ b/infer/src/nullsafe/models.ml @@ -11,29 +11,19 @@ open ModelTables (** Module for standard library models. *) -let match_method_name pn name = - match pn with - | Procname.Java pn_java -> - String.equal (Procname.Java.get_method pn_java) name - | _ -> - false +(** Unique representation of a method signature in form of a string. *) +let to_unique_id proc_name = Procname.to_unique_id (Procname.Java proc_name) +let match_method_name proc_name name = String.equal (Procname.Java.get_method proc_name) name let table_has_procedure table proc_name = - let proc_id = Procname.to_unique_id proc_name in + let proc_id = to_unique_id proc_name in try ignore (Hashtbl.find table proc_id) ; true with Caml.Not_found -> false -let get_unique_repr proc_name = - let java_proc_name = - match proc_name with Procname.Java java_proc_name -> Some java_proc_name | _ -> None - in - Option.map java_proc_name ~f:ThirdPartyAnnotationInfo.unique_repr_of_java_proc_name - - let to_modelled_nullability ThirdPartyMethod.{ret_nullability; params} = let is_nullable = function | ThirdPartyMethod.Nullable -> @@ -45,8 +35,10 @@ let to_modelled_nullability ThirdPartyMethod.{ret_nullability; params} = (* Some methods *) -let get_special_method_modelled_nullability tenv proc_name = +let get_special_method_modelled_nullability tenv java_proc_name = let open IOption.Let_syntax in + (* TODO: convert the implementation that does not use PatternMatch *) + let proc_name = Procname.Java java_proc_name in let* class_name = Procname.get_class_type_name proc_name in if PatternMatch.Java.is_enum tenv class_name then match (Procname.get_method proc_name, Procname.get_parameters proc_name) with @@ -65,12 +57,15 @@ let get_special_method_modelled_nullability tenv proc_name = (** Return the annotated signature of the procedure, taking into account models. External models take precedence over internal ones. *) let get_modelled_annotated_signature ~is_callee_in_trust_list tenv proc_attributes = - let proc_name = proc_attributes.ProcAttributes.proc_name in - let nullsafe_mode = NullsafeMode.of_procname tenv proc_name in + let proc_name = + Procname.as_java_exn proc_attributes.ProcAttributes.proc_name + ~explanation:"get_modelled_annotated_signature should be called for Java methods only" + in + let nullsafe_mode = NullsafeMode.of_java_procname tenv proc_name in let annotated_signature = AnnotatedSignature.get ~is_callee_in_trust_list ~nullsafe_mode proc_attributes in - let proc_id = Procname.to_unique_id proc_name in + let proc_id = to_unique_id proc_name in (* Look in the infer internal models *) let correct_by_internal_models ann_sig = let modelled_nullability = @@ -81,22 +76,21 @@ let get_modelled_annotated_signature ~is_callee_in_trust_list tenv proc_attribut get_special_method_modelled_nullability tenv proc_name ) in Option.value_map modelled_nullability - ~f:(AnnotatedSignature.set_modelled_nullability proc_name ann_sig ModelledInternally) + ~f: + (AnnotatedSignature.set_modelled_nullability (Procname.Java proc_name) ann_sig + ModelledInternally) ~default:ann_sig in (* Look at external models *) let correct_by_external_models ann_sig = - get_unique_repr proc_name - |> Option.bind - ~f: - (ThirdPartyAnnotationInfo.find_nullability_info - (ThirdPartyAnnotationGlobalRepo.get_repo ())) + ThirdPartyAnnotationInfo.unique_repr_of_java_proc_name proc_name + |> ThirdPartyAnnotationInfo.find_nullability_info (ThirdPartyAnnotationGlobalRepo.get_repo ()) |> Option.map ~f:(fun ThirdPartyAnnotationInfo.{signature; filename; line_number} -> (to_modelled_nullability signature, filename, line_number) ) |> Option.value_map (* If we found information in third-party repo, overwrite annotated signature *) ~f:(fun (modelled_nullability, filename, line_number) -> - AnnotatedSignature.set_modelled_nullability proc_name ann_sig + AnnotatedSignature.set_modelled_nullability (Procname.Java proc_name) ann_sig (InThirdPartyRepo {filename; line_number}) modelled_nullability ) ~default:ann_sig @@ -114,7 +108,7 @@ let is_check_not_null proc_name = (** Parameter number (starting from 1) for a procedure known to produce a non-nullable assertion. [None] if the function is not known to be an aseertion OR the parameter number is not known *) let get_check_not_null_parameter proc_name = - let proc_id = Procname.to_unique_id proc_name in + let proc_id = to_unique_id proc_name in Hashtbl.find_opt check_not_null_parameter_table proc_id @@ -138,7 +132,7 @@ let is_true_on_null proc_name = table_has_procedure true_on_null_table proc_name let is_false_on_null proc_name = (* The only usecase for now - consider all overrides of `Object.equals()` correctly implementing the Java specification contract (returning false on null). *) - PatternMatch.Java.is_override_of_lang_object_equals proc_name + PatternMatch.Java.is_override_of_lang_object_equals (Procname.Java proc_name) (** Check if the procedure is Map.containsKey(). *) @@ -149,11 +143,10 @@ let is_mapPut proc_name = table_has_procedure mapPut_table proc_name (** Check if a (nullable) method has a non-nullable alternative: A method that does the same as [proc_name] but asserts the result is not null before returning to the caller. *) -let find_nonnullable_alternative java_proc_name = +let find_nonnullable_alternative proc_name = (* NOTE: For now we fetch this info from internal models. It is a good idea to support this feature in a user-facing third party repository. *) - let proc_name = Procname.Java java_proc_name in - let proc_id = Procname.to_unique_id proc_name in + let proc_id = to_unique_id proc_name in Hashtbl.find_opt nonnull_alternatives_table proc_id diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index 6a2510915..dbdf89355 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -15,7 +15,7 @@ type typecheck_result = (** Module to treat selected complex expressions as constants. *) module ComplexExpressions = struct - let procname_instanceof = Procname.equal BuiltinDecl.__instanceof + let procname_instanceof pname = Procname.equal BuiltinDecl.__instanceof pname let is_annotated_with predicate tenv procname = match PatternMatch.lookup_attributes tenv procname with @@ -31,17 +31,23 @@ module ComplexExpressions = struct false + (* given a predicate that requries Java procname, return a filter that requires generic prodname + *) + let java_predicate_to_pname_predicate java_predicate pname = + match pname with Procname.Java java_pname -> java_predicate java_pname | _ -> false + + let procname_is_false_on_null tenv procname = is_annotated_with Annotations.ia_is_false_on_null tenv procname - || Models.is_false_on_null procname + || java_predicate_to_pname_predicate Models.is_false_on_null procname let procname_is_true_on_null tenv procname = is_annotated_with Annotations.ia_is_true_on_null tenv procname - || Models.is_true_on_null procname + || java_predicate_to_pname_predicate Models.is_true_on_null procname - let procname_containsKey = Models.is_containsKey + let procname_containsKey = java_predicate_to_pname_predicate Models.is_containsKey (** Recognize *all* the procedures treated specially in conditionals *) let procname_used_in_condition pn = @@ -645,9 +651,9 @@ let do_map_put ({IntraproceduralAnalysis.proc_desc= curr_pdesc; tenv; _} as anal | ((_, Exp.Lvar pv_map), _) :: ((_, exp_key), _) :: ((_, exp_value), typ_value) :: _ -> ( (* Convert the dexp for k to the dexp for m.get(k) *) let convert_dexp_key_to_dexp_get dopt = - match (dopt, callee_pname) with - | Some dexp_key, Procname.Java callee_pname_java -> - let pname_get = Procname.Java (pname_get_from_pname_put callee_pname_java) in + match dopt with + | Some dexp_key -> + let pname_get = Procname.Java (pname_get_from_pname_put callee_pname) in let dexp_get = DExp.Dconst (Const.Cfun pname_get) in let dexp_map = DExp.Dpvar pv_map in let args = [dexp_map; dexp_key] in @@ -1062,8 +1068,8 @@ let calc_typestate_after_call if checks.eradicate then EradicateChecks.check_call_parameters ~callee_pname analysis_data ~nullsafe_mode ~callee_annotated_signature find_canonical_duplicate node resolved_params loc instr_ref ; - if Models.is_check_not_null (Procname.Java callee_pname) then - match Models.get_check_not_null_parameter (Procname.Java callee_pname) with + if Models.is_check_not_null callee_pname then + match Models.get_check_not_null_parameter callee_pname with | Some index -> do_preconditions_check_not_null analysis_data instr_ref find_canonical_duplicate node loc curr_annotated_signature checks call_params idenv index ~is_vararg:false @@ -1077,16 +1083,13 @@ let calc_typestate_after_call (* assume the first parameter is checked for null *) do_preconditions_check_not_null analysis_data instr_ref find_canonical_duplicate node loc curr_annotated_signature checks call_params idenv 1 ~is_vararg:false typestate1 - else if - Models.is_check_state (Procname.Java callee_pname) - || Models.is_check_argument (Procname.Java callee_pname) - then + else if Models.is_check_state callee_pname || Models.is_check_argument callee_pname then let curr_pname = Procdesc.get_proc_name curr_pdesc in do_preconditions_check_state instr_ref idenv tenv curr_pname curr_annotated_signature call_params loc node typestate1 - else if Models.is_mapPut (Procname.Java callee_pname) then - do_map_put analysis_data call_params (Procname.Java callee_pname) loc node calls_this checks - instr_ref ~nullsafe_mode find_canonical_duplicate typestate1 + else if Models.is_mapPut callee_pname then + do_map_put analysis_data call_params callee_pname loc node calls_this checks instr_ref + ~nullsafe_mode find_canonical_duplicate typestate1 else typestate1 ) else typestate1 in @@ -1343,8 +1346,8 @@ let can_instrunction_throw tenv node instr = (* true if after this instruction the program interrupts *) let is_noreturn_instruction = function - | Sil.Call (_, Exp.Const (Const.Cfun callee_pname), _, _, _) when Models.is_noreturn callee_pname - -> + | Sil.Call (_, Exp.Const (Const.Cfun (Procname.Java callee_pname)), _, _, _) + when Models.is_noreturn callee_pname -> true | _ -> false