[nullsafe] Add nullsafe extras for meta-issues

mityal · facebook-github-bot · commit 3ccacb1f6ee9 · 2020-04-08T06:50:11.000-07:00
Summary:
This information can be useful for tooling responsible for further
processing (e.g. metric calculation and logging)

Reviewed By: artempyanykh

Differential Revision: D20914583

fbshipit-source-id: 61804d88f
diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt
@@ -673,7 +673,7 @@ OPTIONS
        --issues-tests-fields ,-separated sequence of { bug_type | bucket |
        qualifier | severity | line | column | procedure |
        procedure_start_line | file | bug_trace | key | hash | line_offset |
-       qualifier_contains_potential_exception_note }
+       qualifier_contains_potential_exception_note | nullsafe_extra }
            Fields to emit with --issues-tests           See also infer-report(1).
 
        --java-jar-compiler path
diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt
@@ -282,7 +282,7 @@ OPTIONS
        --issues-tests-fields ,-separated sequence of { bug_type | bucket |
        qualifier | severity | line | column | procedure |
        procedure_start_line | file | bug_trace | key | hash | line_offset |
-       qualifier_contains_potential_exception_note }
+       qualifier_contains_potential_exception_note | nullsafe_extra }
            Fields to emit with --issues-tests
 
        --print-logs
diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt
@@ -673,7 +673,7 @@ OPTIONS
        --issues-tests-fields ,-separated sequence of { bug_type | bucket |
        qualifier | severity | line | column | procedure |
        procedure_start_line | file | bug_trace | key | hash | line_offset |
-       qualifier_contains_potential_exception_note }
+       qualifier_contains_potential_exception_note | nullsafe_extra }
            Fields to emit with --issues-tests           See also infer-report(1).
 
        --java-jar-compiler path
diff --git a/infer/src/atd/jsonbug.atd b/infer/src/atd/jsonbug.atd
@@ -20,9 +20,23 @@ type loc = {
   enum: int;
 }
 
+type nullsafe_mode = [Default | LocalTrustAll | LocalTrustSome | LocalTrustNone | Strict]
+
+type nullsafe_meta_issue_info = {
+  num_issues: int;
+  curr_nullsafe_mode: nullsafe_mode;
+}
+
+type nullsafe_extra = {
+  class_name: string;
+  package: string option;
+  ?meta_issue_info: nullsafe_meta_issue_info option (* Should be present if the issue is "nullsafe meta issue" *)
+}
+
 type extra = {
   ?cost_polynomial : string option;
-  ?cost_degree : int option
+  ?cost_degree : int option;
+  ?nullsafe_extra : nullsafe_extra option
 }
 
 type jsonbug = {
diff --git a/infer/src/backend/reporting.ml b/infer/src/backend/reporting.ml
@@ -86,11 +86,12 @@ let log_error = log_issue_from_summary_simplified Exceptions.Error
 
 let log_warning = log_issue_from_summary_simplified Exceptions.Warning
 
-let log_issue_external procname ~issue_log severity ~loc ~ltr ?access issue_type error_message =
+let log_issue_external procname ~issue_log severity ~loc ~ltr ?access ?extras issue_type
+    error_message =
   let exn = checker_exception issue_type error_message in
   let issue_log, errlog = IssueLog.get_or_add issue_log ~proc:procname in
   let node = Errlog.UnknownNode in
-  log_issue_from_errlog severity errlog ~loc ~node ~session:0 ~ltr ~access ~extras:None exn ;
+  log_issue_from_errlog severity errlog ~loc ~node ~session:0 ~ltr ~access ~extras exn ;
   issue_log
 
 
diff --git a/infer/src/backend/reporting.mli b/infer/src/backend/reporting.mli
@@ -48,6 +48,7 @@ val log_issue_external :
   -> loc:Location.t
   -> ltr:Errlog.loc_trace
   -> ?access:string
+  -> ?extras:Jsonbug_t.extra
   -> IssueType.t
   -> string
   -> IssueLog.t
diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml
@@ -1435,7 +1435,9 @@ and iphoneos_target_sdk_version_path_regex =
 and issues_tests_fields =
   CLOpt.mk_symbol_seq ~long:"issues-tests-fields"
     ~in_help:InferCommand.[(Report, manual_generic)]
-    ~default:IssuesTestField.[File; Procedure; LineOffset; BugType; Bucket; Severity; BugTrace]
+    ~default:
+      IssuesTestField.
+        [File; Procedure; LineOffset; BugType; Bucket; Severity; BugTrace; NullsafeExtra]
     ~symbols:IssuesTestField.all_symbols ~eq:IssuesTestField.equal
     "Fields to emit with $(b,--issues-tests)"
 
diff --git a/infer/src/base/IssuesTestField.ml b/infer/src/base/IssuesTestField.ml
@@ -22,6 +22,7 @@ type t =
   | Hash
   | LineOffset
   | QualifierContainsPotentialExceptionNote
+  | NullsafeExtra
 [@@deriving equal]
 
 let all_symbols =
@@ -38,4 +39,5 @@ let all_symbols =
   ; ("key", Key)
   ; ("hash", Hash)
   ; ("line_offset", LineOffset)
-  ; ("qualifier_contains_potential_exception_note", QualifierContainsPotentialExceptionNote) ]
+  ; ("qualifier_contains_potential_exception_note", QualifierContainsPotentialExceptionNote)
+  ; ("nullsafe_extra", NullsafeExtra) ]
diff --git a/infer/src/base/IssuesTestField.mli b/infer/src/base/IssuesTestField.mli
@@ -22,6 +22,7 @@ type t =
   | Hash
   | LineOffset
   | QualifierContainsPotentialExceptionNote
+  | NullsafeExtra
 [@@deriving equal]
 
 val all_symbols : (string * t) list
diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml
@@ -150,7 +150,8 @@ end
 
 let compute_errlog_extras cost =
   { Jsonbug_t.cost_polynomial= Some (Format.asprintf "%a" BasicCost.pp_hum cost)
-  ; cost_degree= BasicCost.degree cost |> Option.map ~f:Polynomials.Degree.encode_to_int }
+  ; cost_degree= BasicCost.degree cost |> Option.map ~f:Polynomials.Degree.encode_to_int
+  ; nullsafe_extra= None }
 
 
 module ThresholdReports = struct
diff --git a/infer/src/integration/IssuesTest.ml b/infer/src/integration/IssuesTest.ml
@@ -8,6 +8,13 @@
 open! IStd
 module F = Format
 
+let pp_nullsafe_extra fmt Jsonbug_t.{class_name; package; meta_issue_info} =
+  F.fprintf fmt "%s, %s" class_name (Option.value package ~default:"<no package>") ;
+  Option.iter meta_issue_info ~f:(fun Jsonbug_t.{num_issues; curr_nullsafe_mode} ->
+      F.fprintf fmt ", issues: %d, curr_mode: %s" num_issues
+        (Jsonbug_j.string_of_nullsafe_mode curr_nullsafe_mode) )
+
+
 let pp_custom_of_report fmt report fields =
   let pp_custom_of_issue fmt (issue : Jsonbug_t.jsonbug) =
     let open Jsonbug_t in
@@ -61,6 +68,10 @@ let pp_custom_of_report fmt report fields =
       | QualifierContainsPotentialExceptionNote ->
           F.pp_print_bool fmt
             (String.is_substring issue.qualifier ~substring:JsonReports.potential_exception_message)
+      | NullsafeExtra ->
+          let nullsafe_extra = Option.bind issue.extras ~f:(fun extras -> extras.nullsafe_extra) in
+          Option.iter nullsafe_extra ~f:(fun nullsafe_extra ->
+              F.fprintf fmt "%s%a" (comma_separator index) pp_nullsafe_extra nullsafe_extra )
     in
     List.iteri ~f:pp_field fields ; F.fprintf fmt "@."
   in
diff --git a/infer/src/nullsafe/ClassLevelAnalysis.ml b/infer/src/nullsafe/ClassLevelAnalysis.ml
@@ -7,9 +7,12 @@
 
 open! IStd
 
-let log_issue ?proc_name ~issue_log ~loc ~trace ~severity issue_type error_message =
+let log_issue ?proc_name ~issue_log ~loc ~trace ~severity ~nullsafe_extra issue_type error_message =
+  let extras =
+    Jsonbug_t.{nullsafe_extra= Some nullsafe_extra; cost_polynomial= None; cost_degree= None}
+  in
   let proc_name = Option.value proc_name ~default:Procname.Linters_dummy_method in
-  Reporting.log_issue_external proc_name severity ~issue_log ~loc ~ltr:trace issue_type
+  Reporting.log_issue_external proc_name severity ~issue_log ~loc ~extras ~ltr:trace issue_type
     error_message
 
 
@@ -34,8 +37,26 @@ let get_reportable_typing_rules_violations ~nullsafe_mode issues =
   List.filter issues ~f:(is_reportable_typing_rules_violation ~nullsafe_mode)
 
 
+type meta_issue =
+  { issue_type: IssueType.t
+  ; description: string
+  ; severity: Exceptions.severity
+  ; meta_issue_info: Jsonbug_t.nullsafe_meta_issue_info }
+
+let mode_to_json = function
+  | NullsafeMode.Default ->
+      `Default
+  | NullsafeMode.Local NullsafeMode.Trust.All ->
+      `LocalTrustAll
+  | NullsafeMode.Local (NullsafeMode.Trust.Only []) ->
+      `LocalTrustNone
+  | NullsafeMode.Local (NullsafeMode.Trust.Only _) ->
+      `LocalTrustSome
+  | NullsafeMode.Strict ->
+      `Strict
+
+
 (* analyze all isses for the current class and classify them into one meta-issue.
-   return meta issue, description, and severity
  *)
 let make_meta_issue all_issues current_mode class_name =
   (* If current mode is Default, we want to see what would it take to make it @Nullsafe.
@@ -44,7 +65,10 @@ let make_meta_issue all_issues current_mode class_name =
     if NullsafeMode.equal current_mode Default then NullsafeMode.Local NullsafeMode.Trust.All
     else current_mode
   in
-  let reportable_issue_count =
+  let issue_count_in_curr_mode =
+    get_reportable_typing_rules_violations ~nullsafe_mode:current_mode all_issues |> List.length
+  in
+  let issue_count_in_target_mode =
     (* NOTE: This is a tricky place. There are issues that are not surfaced in Default mode, but
        would be surfaced if mode becomes @Nullsafe.
        We want to take those issues into account, hence we evaluate against target_mode, and not current mode!
@@ -53,33 +77,43 @@ let make_meta_issue all_issues current_mode class_name =
     *)
     get_reportable_typing_rules_violations ~nullsafe_mode:target_mode all_issues |> List.length
   in
-  if Int.equal reportable_issue_count 0 then
+  let meta_issue_info =
+    Jsonbug_t.{num_issues= issue_count_in_curr_mode; curr_nullsafe_mode= mode_to_json current_mode}
+  in
+  if Int.equal issue_count_in_target_mode 0 then
     (* Good news. No issues in target mode! *)
     if NullsafeMode.equal current_mode Default then
       (* This class is not @Nullsafe yet, but can become such! *)
-      ( IssueType.eradicate_meta_class_can_be_nullsafe
-      , Format.asprintf
-          "Congrats! Class %a is free of nullability issues. Mark it \
-           `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions."
-          JavaClassName.pp class_name
-      , Exceptions.Advice )
+      { issue_type= IssueType.eradicate_meta_class_can_be_nullsafe
+      ; description=
+          Format.asprintf
+            "Congrats! Class %a is free of nullability issues. Mark it \
+             `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions."
+            JavaClassName.pp class_name
+      ; severity= Exceptions.Advice
+      ; meta_issue_info }
     else
       (* This class is already nullsafe *)
-      ( IssueType.eradicate_meta_class_is_nullsafe
-      , Format.asprintf "Class %a is free of nullability issues." JavaClassName.pp class_name
-      , Exceptions.Info )
+      { issue_type= IssueType.eradicate_meta_class_is_nullsafe
+      ; description=
+          Format.asprintf "Class %a is free of nullability issues." JavaClassName.pp class_name
+      ; severity= Exceptions.Info
+      ; meta_issue_info }
   else
     (* At least one nullability issue. *)
     let description =
       if NullsafeMode.equal current_mode Default then
         Format.asprintf "Class %a needs %d issues to be fixed in order to be marked @Nullsafe."
-          JavaClassName.pp class_name reportable_issue_count
+          JavaClassName.pp class_name issue_count_in_target_mode
       else
         Format.asprintf
           "@Nullsafe classes should have exactly zero nullability issues. Class %a has %d."
-          JavaClassName.pp class_name reportable_issue_count
+          JavaClassName.pp class_name issue_count_in_target_mode
     in
-    (IssueType.eradicate_meta_class_needs_improvement, description, Exceptions.Info)
+    { issue_type= IssueType.eradicate_meta_class_needs_improvement
+    ; description
+    ; severity= Exceptions.Info
+    ; meta_issue_info }
 
 
 (* Meta issues are those related to null-safety of the class in general, not concrete nullability violations *)
@@ -93,9 +127,14 @@ let report_meta_issues tenv source_file class_name class_info issue_log =
         Option.value_map nullsafe ~f:(fun NullsafeSummary.{issues} -> issues) ~default:[] )
     |> List.fold ~init:[] ~f:( @ )
   in
-  let issue_type, description, severity = make_meta_issue all_issues current_mode class_name in
+  let {issue_type; description; severity; meta_issue_info} =
+    make_meta_issue all_issues current_mode class_name
+  in
+  let package = JavaClassName.package class_name in
+  let class_name = JavaClassName.classname class_name in
+  let nullsafe_extra = Jsonbug_t.{class_name; package; meta_issue_info= Some meta_issue_info} in
   let trace = [Errlog.make_trace_element 0 loc description []] in
-  log_issue ~issue_log ~loc ~trace ~severity issue_type description
+  log_issue ~issue_log ~loc ~trace ~severity ~nullsafe_extra issue_type description
 
 
 (* Optimization - if issues are disabled, don't bother analyzing them *)
diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp
