diff --git a/infer/src/atd/jsonbug.atd b/infer/src/atd/jsonbug.atd index cfd2953cf..612ed2c9f 100644 --- a/infer/src/atd/jsonbug.atd +++ b/infer/src/atd/jsonbug.atd @@ -67,12 +67,20 @@ type issue_method = { params: string list; } +type parameter_not_nullable_info = { + class_name: string; + package_name: string nullable; + method_info: issue_method; + param_index: int; +} + type nullsafe_extra = { class_name: string; package: string nullable; ?method_info: issue_method option; (* The method the issue is detected on. Not present for issues not associated with a method (e.g. meta issues) *) ?field: field_name option; (* For field-specific issues (e.g. Field Not Nullable and Field Not Initialized). *) ?inconsistent_param_index: int option; (* Iff the issue is "Inconsistent subclass param annotation", this is the index of the offending param *) + ?parameter_not_nullable_info: parameter_not_nullable_info option; (* Iff the issue is "Parameter Not Nullable", this is the details of the offending method *) ?nullable_methods: method_info list option; (* If the issue is related to unsafe use of methods being nullable, here's the list of them *) ?unvetted_3rd_party: string list option; (* If the issue is related to the use of a not yet registered third-party methods, here's the list of their signatures *) ?meta_issue_info: nullsafe_meta_issue_info option; (* Should be present if the issue is "nullsafe meta issue" *) diff --git a/infer/src/nullsafe/AssignmentRule.ml b/infer/src/nullsafe/AssignmentRule.ml index 3011817ee..ff4ac1614 100644 --- a/infer/src/nullsafe/AssignmentRule.ml +++ b/infer/src/nullsafe/AssignmentRule.ml @@ -43,7 +43,7 @@ module ReportableViolation = struct and function_info = { param_signature: AnnotatedSignature.param_signature ; actual_param_expression: string - ; param_position: int + ; param_index: int ; annotated_signature: AnnotatedSignature.t ; procname: Procname.Java.t } @@ -106,7 +106,7 @@ module ReportableViolation = struct let mk_issue_for_bad_param_passed - {annotated_signature; param_signature; actual_param_expression; param_position; procname} + {annotated_signature; param_signature; actual_param_expression; param_index; procname} ~param_nullability_kind ~nullability_evidence ~(make_issue_factory : description:string -> issue_type:IssueType.t -> NullsafeIssue.t) = let nullability_evidence_as_suffix = @@ -127,68 +127,73 @@ module ReportableViolation = struct Format.asprintf "%a is %s" MF.pp_monospaced actual_param_expression nullability_descr in let issue_type = IssueType.eradicate_parameter_not_nullable in - match AnnotatedNullability.get_nullability annotated_param_nullability with - | Nullability.Null -> - Logging.die Logging.InternalError "Unexpected param nullability: Null" - | Nullability.Nullable -> - Logging.die Logging.InternalError "Passing anything to a nullable param should be allowed" - | Nullability.ThirdPartyNonnull -> - (* This is a special case. While for FB codebase we can assume "not annotated hence not nullable" rule for all_whitelisted signatures, - This is not the case for third party functions, which can have different conventions, - So we can not just say "param is declared as non-nullable" like we say for FB-internal or modelled case: - param can be nullable according to API but it was just not annotated. - 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 - (ThirdPartyAnnotationGlobalRepo.get_repo ()) - procname - in - let where_to_add_signature = - Option.value_map suggested_third_party_sig_file - ~f:(fun sig_file_name -> - ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name - ~filename:sig_file_name ) - (* 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.Java.to_simplified_string ~withclass:true procname in - let description = - 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." - MF.pp_monospaced procname_str param_position pp_param_name param_signature.mangled - argument_description nullability_evidence_as_suffix MF.pp_monospaced procname_str - where_to_add_signature - in - make_issue_factory ~description ~issue_type - |> NullsafeIssue.with_third_party_dependent_methods [(procname, annotated_signature)] - (* Equivalent to non-null from user point of view *) - | Nullability.ProvisionallyNullable - | Nullability.LocallyCheckedNonnull - | Nullability.LocallyTrustedNonnull - | Nullability.UncheckedNonnull - | Nullability.StrictNonnull -> - let nonnull_evidence = - match annotated_signature.kind with - | FirstParty | ThirdParty Unregistered -> - "" - | ThirdParty ModelledInternally -> - " (according to nullsafe internal models)" - | ThirdParty (InThirdPartyRepo {filename; line_number}) -> - Format.sprintf " (see %s at line %d)" - (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name - ~filename) - line_number - in - let description = - Format.asprintf "%a: parameter #%d%a is declared non-nullable%s but the argument %s%s." - MF.pp_monospaced - (Procname.Java.to_simplified_string ~withclass:true procname) - param_position pp_param_name param_signature.mangled nonnull_evidence - argument_description nullability_evidence_as_suffix - in - make_issue_factory ~description ~issue_type + let issue = + match AnnotatedNullability.get_nullability annotated_param_nullability with + | Nullability.Null -> + Logging.die Logging.InternalError "Unexpected param nullability: Null" + | Nullability.Nullable -> + Logging.die Logging.InternalError "Passing anything to a nullable param should be allowed" + | Nullability.ThirdPartyNonnull -> + (* This is a special case. While for FB codebase we can assume "not annotated hence not nullable" rule for all_whitelisted signatures, + This is not the case for third party functions, which can have different conventions, + So we can not just say "param is declared as non-nullable" like we say for FB-internal or modelled case: + param can be nullable according to API but it was just not annotated. + 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 + (ThirdPartyAnnotationGlobalRepo.get_repo ()) + procname + in + let where_to_add_signature = + Option.value_map suggested_third_party_sig_file + ~f:(fun sig_file_name -> + ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name + ~filename:sig_file_name ) + (* 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.Java.to_simplified_string ~withclass:true procname in + let description = + 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." + MF.pp_monospaced procname_str + (param_index + 1) (* human-readable param number is indexed from 1 *) + pp_param_name param_signature.mangled argument_description + nullability_evidence_as_suffix MF.pp_monospaced procname_str where_to_add_signature + in + make_issue_factory ~description ~issue_type + |> NullsafeIssue.with_third_party_dependent_methods [(procname, annotated_signature)] + (* Equivalent to non-null from user point of view *) + | Nullability.ProvisionallyNullable + | Nullability.LocallyCheckedNonnull + | Nullability.LocallyTrustedNonnull + | Nullability.UncheckedNonnull + | Nullability.StrictNonnull -> + let nonnull_evidence = + match annotated_signature.kind with + | FirstParty | ThirdParty Unregistered -> + "" + | ThirdParty ModelledInternally -> + " (according to nullsafe internal models)" + | ThirdParty (InThirdPartyRepo {filename; line_number}) -> + Format.sprintf " (see %s at line %d)" + (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name + ~filename) + line_number + in + let description = + Format.asprintf "%a: parameter #%d%a is declared non-nullable%s but the argument %s%s." + MF.pp_monospaced + (Procname.Java.to_simplified_string ~withclass:true procname) + (param_index + 1) (* human-readable param number is indexed from 1 *) + pp_param_name param_signature.mangled nonnull_evidence argument_description + nullability_evidence_as_suffix + in + make_issue_factory ~description ~issue_type + in + issue |> NullsafeIssue.with_parameter_not_nullable_info ~param_index ~proc_name:procname let field_name_of_assignment_type = function diff --git a/infer/src/nullsafe/AssignmentRule.mli b/infer/src/nullsafe/AssignmentRule.mli index d1e7bb4a5..e3a3245e2 100644 --- a/infer/src/nullsafe/AssignmentRule.mli +++ b/infer/src/nullsafe/AssignmentRule.mli @@ -49,7 +49,7 @@ module ReportableViolation : sig and function_info = { param_signature: AnnotatedSignature.param_signature ; actual_param_expression: string - ; param_position: int + ; param_index: int ; annotated_signature: AnnotatedSignature.t ; procname: Procname.Java.t } diff --git a/infer/src/nullsafe/ClassLevelAnalysis.ml b/infer/src/nullsafe/ClassLevelAnalysis.ml index e1d8e01d7..bee8bdef0 100644 --- a/infer/src/nullsafe/ClassLevelAnalysis.ml +++ b/infer/src/nullsafe/ClassLevelAnalysis.ml @@ -188,6 +188,7 @@ let report_meta_issue_for_top_level_class tenv source_file class_name class_stru ; package ; method_info= None ; inconsistent_param_index= None + ; parameter_not_nullable_info= None ; meta_issue_info= Some meta_issue_info ; unvetted_3rd_party= None ; nullable_methods= None @@ -222,6 +223,7 @@ let analyze_nullsafe_annotations tenv source_file class_name class_struct issue_ ; package ; method_info= None ; inconsistent_param_index= None + ; parameter_not_nullable_info= None ; meta_issue_info= None ; unvetted_3rd_party= None ; nullable_methods= None @@ -272,6 +274,7 @@ let report_annotation_graph source_file class_name class_struct annotation_graph ; package ; method_info= None ; inconsistent_param_index= None + ; parameter_not_nullable_info= None ; meta_issue_info= None ; unvetted_3rd_party= None ; nullable_methods= None diff --git a/infer/src/nullsafe/NullsafeIssue.ml b/infer/src/nullsafe/NullsafeIssue.ml index 505293bd9..dd41e5485 100644 --- a/infer/src/nullsafe/NullsafeIssue.ml +++ b/infer/src/nullsafe/NullsafeIssue.ml @@ -7,6 +7,9 @@ open! IStd +type parameter_not_nullable_info = + {param_index: int; proc_name: Procname.Java.t (** Offending method called *)} + type t = { issue_type: IssueType.t ; description: string (** Human-readable description *) @@ -14,6 +17,8 @@ type t = ; field_name: Fieldname.t option (** If the issue is about a field, here's this field *) ; inconsistent_param_index: int option (** Only for "inconsistent subclass param annotation" issue *) + ; parameter_not_nullable_info: parameter_not_nullable_info option + (** Only for "Parameter Not Nullable" issue *) ; severity: IssueType.severity ; nullable_methods: TypeOrigin.method_call_origin list (** If the issue is associated with misusing nullable values coming from method calls, @@ -25,6 +30,7 @@ let make ~issue_type ~description ~loc ~severity ~field_name = ; description ; loc ; inconsistent_param_index= None + ; parameter_not_nullable_info= None ; severity ; third_party_dependent_methods= [] ; nullable_methods= [] @@ -37,6 +43,10 @@ let with_nullable_methods methods t = {t with nullable_methods= methods} let with_inconsistent_param_index index t = {t with inconsistent_param_index= index} +let with_parameter_not_nullable_info ~param_index ~proc_name t = + {t with parameter_not_nullable_info= Some {param_index; proc_name}} + + let get_issue_type {issue_type} = issue_type let get_description {description} = description @@ -93,9 +103,26 @@ let to_nullable_method_json nullable_methods = let java_type_to_string java_type = Pp.string_of_pp (Typ.pp_java ~verbose:true) java_type +let get_params_string_list procname = + Procname.Java.get_parameters procname |> List.map ~f:java_type_to_string + + +let parameter_not_nullable_info_to_json {param_index; proc_name} : + Jsonbug_t.parameter_not_nullable_info = + let package_name = Procname.Java.get_package proc_name in + let class_name = Procname.Java.get_simple_class_name proc_name in + let method_info = + Jsonbug_t.{name= Procname.Java.get_method proc_name; params= get_params_string_list proc_name} + in + Jsonbug_t.{class_name; package_name; method_info; param_index} + + let get_nullsafe_extra - {third_party_dependent_methods; nullable_methods; inconsistent_param_index; field_name} - proc_name = + { third_party_dependent_methods + ; nullable_methods + ; inconsistent_param_index + ; parameter_not_nullable_info + ; field_name } proc_name = let class_name = Procname.Java.get_simple_class_name proc_name in let package = Procname.Java.get_package proc_name in let unvetted_3rd_party_list = @@ -119,13 +146,17 @@ let get_nullsafe_extra ; package_name= JavaClassName.package java_class_name ; field } ) in - let method_params = Procname.Java.get_parameters proc_name |> List.map ~f:java_type_to_string in + let method_params = get_params_string_list proc_name in let method_info = Jsonbug_t.{name= Procname.Java.get_method proc_name; params= method_params} in + let parameter_not_nullable_info = + Option.map ~f:parameter_not_nullable_info_to_json parameter_not_nullable_info + in Jsonbug_t. { class_name ; package ; method_info= Some method_info ; inconsistent_param_index + ; parameter_not_nullable_info ; meta_issue_info= None ; unvetted_3rd_party ; nullable_methods diff --git a/infer/src/nullsafe/NullsafeIssue.mli b/infer/src/nullsafe/NullsafeIssue.mli index fd89fb14f..4af5016b4 100644 --- a/infer/src/nullsafe/NullsafeIssue.mli +++ b/infer/src/nullsafe/NullsafeIssue.mli @@ -26,6 +26,9 @@ val with_nullable_methods : TypeOrigin.method_call_origin list -> t -> t val with_inconsistent_param_index : int option -> t -> t (** Only for the "Inconsistent subclass param annotation" issue *) +val with_parameter_not_nullable_info : param_index:int -> proc_name:Procname.Java.t -> t -> t +(** Only for the "Paremeter not nullable" issue *) + val get_issue_type : t -> IssueType.t val get_description : t -> string diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index b50a8ba19..46d1291a0 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -399,7 +399,7 @@ let check_call_receiver analysis_data ~nullsafe_mode find_canonical_duplicate no type resolved_param = - { num: int + { param_index: int ; formal: AnnotatedSignature.param_signature ; actual: Exp.t * InferredNullability.t ; is_formal_propagates_nullable: bool } @@ -408,7 +408,7 @@ type resolved_param = let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~nullsafe_mode ~callee_pname ~callee_annotated_signature find_canonical_duplicate node resolved_params loc instr_ref : unit = - let check {num= param_position; formal; actual= orig_e2, nullability_actual} = + let check {param_index; formal; actual= orig_e2, nullability_actual} = let report ~nullsafe_mode assignment_violation = let actual_param_expression = match explain_expr tenv node orig_e2 with @@ -425,7 +425,7 @@ let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~ PassingParamToFunction { param_signature= formal ; actual_param_expression - ; param_position + ; param_index ; annotated_signature= callee_annotated_signature ; procname= callee_pname } }) (Some instr_ref) ~nullsafe_mode diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index fa1e76098..152bd50d8 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -1022,7 +1022,7 @@ let calc_typestate_after_call find_canonical_duplicate calls_this checks idenv instr_ref signature_params cflags call_params ~is_anonymous_inner_class_constructor ~callee_annotated_signature ~callee_attributes ~callee_pname ~curr_annotated_signature ~nullsafe_mode ~typestate ~typestate1 loc node = - let resolve_param i (formal_param, actual_param) = + let resolve_param param_index (formal_param, actual_param) = let (orig_e2, e2), t2 = actual_param in let _, inferred_nullability_actual = typecheck_expr analysis_data ~nullsafe_mode find_canonical_duplicate calls_this checks node @@ -1032,12 +1032,11 @@ let calc_typestate_after_call loc in let actual = (orig_e2, inferred_nullability_actual) in - let num = i + 1 in let is_formal_propagates_nullable = Annotations.ia_is_propagates_nullable formal_param.AnnotatedSignature.param_annotation_deprecated in - EradicateChecks.{num; formal= formal_param; actual; is_formal_propagates_nullable} + EradicateChecks.{param_index; formal= formal_param; actual; is_formal_propagates_nullable} in (* Infer nullability of function call result based on its signature *) let preliminary_resolved_ret =