[nullsafe][refactor] Migrate parts of Nullsafe to Procname.Java.t

Summary:
This is part of work aimed to reduce usage of language-agnostics modules
in Java-specific parts of nullsafe.

This diff:
1/ migrates AssignmentRule
2/ passes Procname.Java.t around from the start of analysis.

For now we have places where we have both procname and java procname.
This is fine as we can get rid of it in follow up diffs.

Reviewed By: artempyanykh

Differential Revision: D23052226

fbshipit-source-id: 0125de297
master
Mitya Lyubarskiy 4 years ago committed by Facebook GitHub Bot
parent d4bea60440
commit 4bc4376c85

@ -393,6 +393,14 @@ let with_block_parameters base blocks = WithBlockParameters (base, blocks)
let is_java = function Java _ -> true | _ -> false
let as_java_exn ~explanation t =
match t with
| Java java ->
java
| _ ->
Logging.die InternalError "Expected Java procname: %s" explanation
(* TODO: deprecate this unfortunately named function and use is_clang instead *)
let is_c_method = function ObjC_Cpp _ -> true | _ -> false

@ -292,6 +292,9 @@ val is_constructor : t -> bool
val is_java : t -> bool
(** Check if this is a Java procedure name. *)
val as_java_exn : explanation:string -> t -> Java.t
(** Converts to a Java.t. Throws if [is_java] is false *)
val with_block_parameters : t -> Block.block_name list -> t
(** Create a procedure name instantiated with block parameters from a base procedure name and a list
of block procedure names (the arguments). *)

@ -14,7 +14,7 @@ module ReportableViolation = struct
type assignment_type =
| PassingParamToFunction of function_info
| AssigningToField of Fieldname.t
| ReturningFromFunction of Procname.t
| ReturningFromFunction of Procname.Java.t
[@@deriving compare]
and function_info =
@ -22,7 +22,7 @@ module ReportableViolation = struct
; kind: AnnotatedSignature.kind
; actual_param_expression: string
; param_position: int
; function_procname: Procname.t }
; function_procname: Procname.Java.t }
let from nullsafe_mode ({lhs; rhs} as violation) =
let falls_under_optimistic_third_party =
@ -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_proc
ThirdPartyAnnotationInfo.lookup_related_sig_file_for_java_proc
(ThirdPartyAnnotationGlobalRepo.get_repo ())
function_procname
in
@ -106,7 +106,7 @@ module ReportableViolation = struct
(* this can happen when third party is registered in a deprecated way (not in third party repository) *)
~default:"the third party signature storage"
in
let procname_str = Procname.to_simplified_string ~withclass:true function_procname in
let procname_str = Procname.Java.to_simplified_string ~withclass:true function_procname in
Format.asprintf
"Third-party %a is missing a signature that would allow passing a nullable to param \
#%d%a. Actual argument %s%s. Consider adding the correct signature of %a to %s."
@ -131,7 +131,7 @@ module ReportableViolation = struct
in
Format.asprintf "%a: parameter #%d%a is declared non-nullable%s but the argument %s%s."
MF.pp_monospaced
(Procname.to_simplified_string ~withclass:true function_procname)
(Procname.Java.to_simplified_string ~withclass:true function_procname)
param_position pp_param_name param_signature.mangled nonnull_evidence argument_description
nullability_evidence_as_suffix
@ -193,7 +193,7 @@ module ReportableViolation = struct
in
Format.asprintf "%a: return type is declared non-nullable but the method returns %s%s.%s"
MF.pp_monospaced
(Procname.to_simplified_string ~withclass:false function_proc_name)
(Procname.Java.to_simplified_string ~withclass:false function_proc_name)
return_description nullability_evidence_as_suffix alternative_recommendation
in
let issue_type = get_issue_type assignment_type in

@ -27,7 +27,7 @@ module ReportableViolation : sig
type assignment_type =
| PassingParamToFunction of function_info
| AssigningToField of Fieldname.t
| ReturningFromFunction of Procname.t
| ReturningFromFunction of Procname.Java.t
[@@deriving compare]
and function_info =
@ -35,7 +35,7 @@ module ReportableViolation : sig
; kind: AnnotatedSignature.kind
; actual_param_expression: string
; param_position: int
; function_procname: Procname.t }
; function_procname: Procname.Java.t }
val get_severity : t -> IssueType.severity
(** Severity of the violation to be reported *)

@ -103,17 +103,19 @@ 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_proc storage procname =
let package =
match procname with
| Procname.Java java_pname ->
Procname.Java.get_package java_pname
| _ ->
None
in
let lookup_related_sig_file_for_java_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

@ -50,6 +50,9 @@ val lookup_related_sig_file : storage -> package:string -> string option
val lookup_related_sig_file_for_proc : storage -> Procname.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
(** 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

@ -12,7 +12,7 @@ module F = Format
(* make sure that this is initialized *)
let () = NullsafeInit.init ()
let callback1 ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_data)
let callback1 ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_data) ~curr_java_pname
find_canonical_duplicate calls_this checks idenv annotated_signature linereader proc_loc :
bool * TypeState.t option =
let curr_pname = Procdesc.get_proc_name curr_pdesc in
@ -39,8 +39,8 @@ let callback1 ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_da
List.fold ~f:add_formal ~init:typestate_empty annotated_signature.AnnotatedSignature.params
in
(* 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 : unit
=
let check_return ~java_pname find_canonical_duplicate exit_node final_typestate
annotated_signature loc : unit =
let {AnnotatedSignature.ret_annotated_type} = annotated_signature.AnnotatedSignature.ret in
let ret_pvar = Procdesc.get_ret_var curr_pdesc in
let ret_range = TypeState.lookup_pvar ret_pvar final_typestate in
@ -57,16 +57,17 @@ let callback1 ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_da
~f:(fun f -> f curr_pdesc ret_annotated_type.typ typ_found_opt loc)
checks.TypeCheck.check_ret_type ;
if checks.TypeCheck.eradicate then
EradicateChecks.check_return_annotation analysis_data find_canonical_duplicate ret_range
annotated_signature ret_implicitly_nullable loc
EradicateChecks.check_return_annotation analysis_data ~java_pname find_canonical_duplicate
ret_range annotated_signature ret_implicitly_nullable loc
in
let do_before_dataflow initial_typestate =
if Config.eradicate_verbose then
L.result "Initial Typestate@\n%a@." TypeState.pp initial_typestate
in
let do_after_dataflow find_canonical_duplicate final_typestate =
let do_after_dataflow ~java_pname find_canonical_duplicate final_typestate =
let exit_node = Procdesc.get_exit_node curr_pdesc in
check_return find_canonical_duplicate exit_node final_typestate annotated_signature proc_loc
check_return ~java_pname find_canonical_duplicate exit_node final_typestate annotated_signature
proc_loc
in
let module DFTypeCheck = DataFlow.MakeDF (struct
type t = TypeState.t
@ -98,14 +99,15 @@ let callback1 ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_da
let transitions = DFTypeCheck.run curr_pdesc initial_typestate in
match transitions (Procdesc.get_exit_node curr_pdesc) with
| DFTypeCheck.Transition (final_typestate, _, _) ->
do_after_dataflow find_canonical_duplicate final_typestate ;
do_after_dataflow ~java_pname:curr_java_pname find_canonical_duplicate final_typestate ;
(!calls_this, Some final_typestate)
| DFTypeCheck.Dead_state ->
(!calls_this, None)
let analyze_one_procedure ({IntraproceduralAnalysis.tenv; proc_desc; _} as analysis_data) calls_this
checks annotated_signature linereader proc_loc : unit =
let analyze_one_procedure ~java_pname
({IntraproceduralAnalysis.tenv; proc_desc; _} as analysis_data) calls_this checks
annotated_signature linereader proc_loc : unit =
let idenv = IDEnv.create proc_desc in
let find_duplicate_nodes = State.mk_find_duplicate_nodes proc_desc in
let find_canonical_duplicate node =
@ -153,10 +155,12 @@ let analyze_one_procedure ({IntraproceduralAnalysis.tenv; proc_desc; _} as analy
&& checks.TypeCheck.eradicate
then (
let typestates_for_curr_constructor_and_all_initializer_methods =
Initializers.final_initializer_typestates_lazy tenv proc_name proc_desc typecheck_proc
Initializers.final_initializer_typestates_lazy tenv proc_name proc_desc
(typecheck_proc ~curr_java_pname:java_pname)
in
let typestates_for_all_constructors_incl_current =
Initializers.final_constructor_typestates_lazy tenv proc_name typecheck_proc
Initializers.final_constructor_typestates_lazy tenv proc_name
(typecheck_proc ~curr_java_pname:java_pname)
in
EradicateChecks.check_constructor_initialization analysis_data find_canonical_duplicate
start_node ~nullsafe_mode:annotated_signature.AnnotatedSignature.nullsafe_mode
@ -167,7 +171,8 @@ let analyze_one_procedure ({IntraproceduralAnalysis.tenv; proc_desc; _} as analy
match typestate_opt with None -> () | Some typestate -> do_typestate typestate
in
let calls_this, final_typestate_opt =
typecheck_proc true proc_name proc_desc (Some (annotated_signature, proc_loc, idenv))
typecheck_proc ~curr_java_pname:java_pname true proc_name proc_desc
(Some (annotated_signature, proc_loc, idenv))
in
do_final_typestate final_typestate_opt calls_this ;
if checks.TypeCheck.eradicate then
@ -206,6 +211,9 @@ let analyze checks ({IntraproceduralAnalysis.proc_desc; tenv; _} as analysis_dat
| None ->
(* start the analysis! *)
let calls_this = ref false in
let java_pname =
Procname.as_java_exn ~explanation:"Would have skipped the analysis otherwise" proc_name
in
let annotated_signature =
AnnotatedSignature.get_for_class_under_analysis tenv (Procdesc.get_attributes proc_desc)
in
@ -218,7 +226,8 @@ let analyze checks ({IntraproceduralAnalysis.proc_desc; tenv; _} as analysis_dat
(* The main method - during this the actual analysis will happen and TypeErr will be populated with
issues (and some of them - reported).
*)
analyze_one_procedure analysis_data calls_this checks annotated_signature linereader loc ;
analyze_one_procedure ~java_pname analysis_data calls_this checks annotated_signature
linereader loc ;
(* Collect issues that were detected during analysis and put them in summary for further processing *)
let issues = TypeErr.get_errors () |> List.map ~f:(fun (issues, _) -> issues) in
(* Report errors of "farall" class - those could not be reported during analysis phase. *)

@ -325,19 +325,17 @@ let check_constructor_initialization
()
let check_return_not_nullable ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_data)
~nullsafe_mode find_canonical_duplicate loc (ret_signature : AnnotatedSignature.ret_signature)
ret_inferred_nullability =
let check_return_not_nullable analysis_data ~nullsafe_mode ~java_pname find_canonical_duplicate loc
(ret_signature : AnnotatedSignature.ret_signature) ret_inferred_nullability =
(* Returning from a function is essentially an assignment the actual return value to the formal `return` *)
let lhs = ret_signature.ret_annotated_type.nullability in
let rhs = ret_inferred_nullability in
Result.iter_error (AssignmentRule.check ~lhs ~rhs) ~f:(fun assignment_violation ->
let curr_pname = Procdesc.get_proc_name curr_pdesc in
TypeErr.register_error analysis_data find_canonical_duplicate
(Bad_assignment
{ assignment_violation
; assignment_location= loc
; assignment_type= ReturningFromFunction curr_pname })
; assignment_type= ReturningFromFunction java_pname })
None ~nullsafe_mode loc )
@ -360,20 +358,18 @@ let check_return_overrannotated
(** Check the annotations when returning from a method. *)
let check_return_annotation ({IntraproceduralAnalysis.proc_desc= curr_pdesc; _} as analysis_data)
find_canonical_duplicate ret_range (annotated_signature : AnnotatedSignature.t)
ret_implicitly_nullable loc : unit =
let curr_pname = Procdesc.get_proc_name curr_pdesc in
let check_return_annotation analysis_data ~java_pname find_canonical_duplicate ret_range
(annotated_signature : AnnotatedSignature.t) ret_implicitly_nullable loc : unit =
match ret_range with
(* Disables the warnings since it is not clear how to annotate the return value of lambdas *)
| Some _
when match curr_pname with Java java_pname -> Procname.Java.is_lambda java_pname | _ -> false ->
| Some _ when Procname.Java.is_lambda java_pname ->
()
| Some (_, ret_inferred_nullability) ->
(* TODO(T54308240) Model ret_implicitly_nullable in AnnotatedNullability *)
if not ret_implicitly_nullable then
check_return_not_nullable analysis_data ~nullsafe_mode:annotated_signature.nullsafe_mode
find_canonical_duplicate loc annotated_signature.ret ret_inferred_nullability ;
check_return_not_nullable ~java_pname analysis_data
~nullsafe_mode:annotated_signature.nullsafe_mode find_canonical_duplicate loc
annotated_signature.ret ret_inferred_nullability ;
if Config.eradicate_return_over_annotated then
check_return_overrannotated analysis_data find_canonical_duplicate loc
annotated_signature.ret ~nullsafe_mode:annotated_signature.nullsafe_mode
@ -408,9 +404,8 @@ type resolved_param =
(** Check the parameters of a call. *)
let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~nullsafe_mode
~callee_annotated_signature find_canonical_duplicate node callee_attributes resolved_params loc
~callee_pname ~callee_annotated_signature find_canonical_duplicate node resolved_params loc
instr_ref : unit =
let callee_pname = callee_attributes.ProcAttributes.proc_name in
let check {num= param_position; formal; actual= orig_e2, nullability_actual} =
let report ~nullsafe_mode assignment_violation =
let actual_param_expression =

@ -1061,9 +1061,9 @@ let calc_typestate_after_call
node typestate1 call_params callee_pname_java instr_ref loc
(typecheck_expr analysis_data ~nullsafe_mode find_canonical_duplicate calls_this checks) ;
if checks.eradicate then
EradicateChecks.check_call_parameters analysis_data ~nullsafe_mode
~callee_annotated_signature find_canonical_duplicate node callee_attributes
resolved_params loc instr_ref ;
EradicateChecks.check_call_parameters ~callee_pname:callee_pname_java analysis_data
~nullsafe_mode ~callee_annotated_signature find_canonical_duplicate node resolved_params
loc instr_ref ;
if Models.is_check_not_null callee_pname then
match Models.get_check_not_null_parameter callee_pname with
| Some index ->

@ -101,7 +101,7 @@ let is_synthetic_err = function
| PassingParamToFunction {function_procname; _} ->
(* NOTE: this currently doesn't cover the case when the actual
argument involves syntethic code *)
Procname.is_java_autogen_method function_procname
Procname.Java.is_autogen_method function_procname
| AssigningToField fieldname ->
Fieldname.is_java_synthetic fieldname
| _ ->

Loading…
Cancel
Save