[nullsafe] Record info for "Parameter Not Nullable" issues in json

Reviewed By: artempyanykh

Differential Revision: D25219012

fbshipit-source-id: 8999a0252

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" *)

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
+    let issue =
-    | Nullability.Null ->
+      match AnnotatedNullability.get_nullability annotated_param_nullability with
-        Logging.die Logging.InternalError "Unexpected param nullability: Null"
+      | Nullability.Null ->
-    | Nullability.Nullable ->
+          Logging.die Logging.InternalError "Unexpected param nullability: Null"
-        Logging.die Logging.InternalError "Passing anything to a nullable param should be allowed"
+      | Nullability.Nullable ->
-    | Nullability.ThirdPartyNonnull ->
+          Logging.die Logging.InternalError "Passing anything to a nullable param should be allowed"
-        (* This is a special case. While for FB codebase we can assume "not annotated hence not nullable" rule for all_whitelisted signatures,
+      | Nullability.ThirdPartyNonnull ->
-           This is not the case for third party functions, which can have different conventions,
+          (* This is a special case. While for FB codebase we can assume "not annotated hence not nullable" rule for all_whitelisted signatures,
-           So we can not just say "param is declared as non-nullable" like we say for FB-internal or modelled case:
+             This is not the case for third party functions, which can have different conventions,
-           param can be nullable according to API but it was just not annotated.
+             So we can not just say "param is declared as non-nullable" like we say for FB-internal or modelled case:
-           So we phrase it differently to remain truthful, but as specific as possible.
+             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
+          let suggested_third_party_sig_file =
-            (ThirdPartyAnnotationGlobalRepo.get_repo ())
+            ThirdPartyAnnotationInfo.lookup_related_sig_file_for_proc
-            procname
+              (ThirdPartyAnnotationGlobalRepo.get_repo ())
-        in
+              procname
-        let where_to_add_signature =
+          in
-          Option.value_map suggested_third_party_sig_file
+          let where_to_add_signature =
-            ~f:(fun sig_file_name ->
+            Option.value_map suggested_third_party_sig_file
-              ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name
+              ~f:(fun sig_file_name ->
-                ~filename:sig_file_name )
+                ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name
-              (* this can happen when third party is registered in a deprecated way (not in third party repository) *)
+                  ~filename:sig_file_name )
-            ~default:"the third party signature storage"
+                (* this can happen when third party is registered in a deprecated way (not in third party repository) *)
-        in
+              ~default:"the third party signature storage"
-        let procname_str = Procname.Java.to_simplified_string ~withclass:true procname in
+          in
-        let description =
+          let procname_str = Procname.Java.to_simplified_string ~withclass:true procname in
-          Format.asprintf
+          let description =
-            "Third-party %a is missing a signature that would allow passing a nullable to param \
+            Format.asprintf
-             #%d%a. Actual argument %s%s. Consider adding the correct signature of %a to %s."
+              "Third-party %a is missing a signature that would allow passing a nullable to param \
-            MF.pp_monospaced procname_str param_position pp_param_name param_signature.mangled
+               #%d%a. Actual argument %s%s. Consider adding the correct signature of %a to %s."
-            argument_description nullability_evidence_as_suffix MF.pp_monospaced procname_str
+              MF.pp_monospaced procname_str
-            where_to_add_signature
+              (param_index + 1) (* human-readable param number is indexed from 1 *)
-        in
+              pp_param_name param_signature.mangled argument_description
-        make_issue_factory ~description ~issue_type
+              nullability_evidence_as_suffix MF.pp_monospaced procname_str where_to_add_signature
-        |> NullsafeIssue.with_third_party_dependent_methods [(procname, annotated_signature)]
+          in
-    (* Equivalent to non-null from user point of view *)
+          make_issue_factory ~description ~issue_type
-    | Nullability.ProvisionallyNullable
+          |> NullsafeIssue.with_third_party_dependent_methods [(procname, annotated_signature)]
-    | Nullability.LocallyCheckedNonnull
+      (* Equivalent to non-null from user point of view *)
-    | Nullability.LocallyTrustedNonnull
+      | Nullability.ProvisionallyNullable
-    | Nullability.UncheckedNonnull
+      | Nullability.LocallyCheckedNonnull
-    | Nullability.StrictNonnull ->
+      | Nullability.LocallyTrustedNonnull
-        let nonnull_evidence =
+      | Nullability.UncheckedNonnull
-          match annotated_signature.kind with
+      | Nullability.StrictNonnull ->
-          | FirstParty | ThirdParty Unregistered ->
+          let nonnull_evidence =
-              ""
+            match annotated_signature.kind with
-          | ThirdParty ModelledInternally ->
+            | FirstParty | ThirdParty Unregistered ->
-              " (according to nullsafe internal models)"
+                ""
-          | ThirdParty (InThirdPartyRepo {filename; line_number}) ->
+            | ThirdParty ModelledInternally ->
-              Format.sprintf " (see %s at line %d)"
+                " (according to nullsafe internal models)"
-                (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name
+            | ThirdParty (InThirdPartyRepo {filename; line_number}) ->
-                   ~filename)
+                Format.sprintf " (see %s at line %d)"
-                line_number
+                  (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name
-        in
+                     ~filename)
-        let description =
+                  line_number
-          Format.asprintf "%a: parameter #%d%a is declared non-nullable%s but the argument %s%s."
+          in
-            MF.pp_monospaced
+          let description =
-            (Procname.Java.to_simplified_string ~withclass:true procname)
+            Format.asprintf "%a: parameter #%d%a is declared non-nullable%s but the argument %s%s."
-            param_position pp_param_name param_signature.mangled nonnull_evidence
+              MF.pp_monospaced
-            argument_description nullability_evidence_as_suffix
+              (Procname.Java.to_simplified_string ~withclass:true procname)
-        in
+              (param_index + 1) (* human-readable param number is indexed from 1 *)
-        make_issue_factory ~description ~issue_type
+              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

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 }
 

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

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}
+    { third_party_dependent_methods
-    proc_name =
+    ; 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

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

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

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 =