Reporting cleanup 22: log_error/warning -> use IssueType rather than exception

Summary:
Callsites of `Reporting.log_error/warning` always use `Exceptions.Checkers`, let's simplify the API.
Under the hood it still creates an exception, but this can be cleaned up later.

Reviewed By: jeremydubreil

Differential Revision: D9799860

fbshipit-source-id: 6492a60b4

infer/src/IR/Localise.ml

@@ -160,29 +160,6 @@ let add_by_call_to_opt problem_str proc_name_opt =
       problem_str
 
 
-let rec format_typ typ =
-  match typ.Typ.desc with
-  | Typ.Tptr (t, _) when Language.curr_language_is Java ->
-      format_typ t
-  | Typ.Tstruct name ->
-      Typ.Name.name name
-  | _ ->
-      Typ.to_string typ
-
-
-let format_field f =
-  if Language.curr_language_is Java then Typ.Fieldname.Java.get_field f
-  else Typ.Fieldname.to_string f
-
-
-let format_method pname =
-  match pname with
-  | Typ.Procname.Java pname_java ->
-      Typ.Procname.Java.get_method pname_java
-  | _ ->
-      Typ.Procname.to_string pname
-
-
 let mem_dyn_allocated = "memory dynamically allocated"
 
 let lock_acquired = "lock acquired"
@@ -370,23 +347,6 @@ let desc_unsafe_guarded_by_access accessed_fld guarded_by_str loc =
   {no_desc with descriptions= [msg]}
 
 
-let desc_fragment_retains_view fragment_typ fieldname fld_typ pname : error_desc =
-  (* TODO: try advice *)
-  let problem =
-    Printf.sprintf "Fragment %s does not nullify View field %s (type %s) in %s."
-      (format_typ fragment_typ) (format_field fieldname) (format_typ fld_typ) (format_method pname)
-  in
-  let consequences =
-    "If this Fragment is placed on the back stack, a reference to this (probably dead) View will \
-     be retained."
-  in
-  let advice =
-    "In general, it is a good idea to initialize View's in onCreateView, then nullify them in \
-     onDestroyView."
-  in
-  {no_desc with descriptions= [problem; consequences; advice]}
-
-
 let desc_custom_error loc : error_desc =
   {no_desc with descriptions= ["detected"; at_line (Tags.create ()) loc]}
 
@@ -673,8 +633,6 @@ let desc_leak hpred_type_opt value_str_opt resource_opt resource_action_opt loc
     descriptions= (bucket_str :: xxx_allocated_to) @ by_call_to @ is_not_rxxx_after; tags= !tags }
 
 
-let desc_buffer_overrun desc = verbatim_desc desc
-
 (** kind of precondition not met *)
 type pnm_kind = Pnm_bounds | Pnm_dangling
 

infer/src/IR/Localise.mli

@@ -140,14 +140,10 @@ val desc_leak :
   -> string option
   -> error_desc
 
-val desc_buffer_overrun : string -> error_desc
-
 val desc_null_test_after_dereference : string -> int -> Location.t -> error_desc
 
 val java_unchecked_exn_desc : Typ.Procname.t -> Typ.Name.t -> string -> error_desc
 
-val desc_fragment_retains_view : Typ.t -> Typ.Fieldname.t -> Typ.t -> Typ.Procname.t -> error_desc
-
 val desc_custom_error : Location.t -> error_desc
 (** Create human-readable error description for assertion failures *)
 

infer/src/backend/reporting.ml

@@ -8,7 +8,7 @@
 open! IStd
 module L = Logging
 
-type log_t = ?ltr:Errlog.loc_trace -> ?extras:Jsonbug_t.extra -> exn -> unit
+type log_t = ?ltr:Errlog.loc_trace -> ?extras:Jsonbug_t.extra -> IssueType.t -> string -> unit
 
 let log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr
     ~access ~extras exn =
@@ -68,7 +68,13 @@ let log_issue_deprecated_using_state severity proc_name ?node ?loc ?ltr exn =
           Typ.Procname.pp proc_name Typ.Procname.pp proc_name
 
 
-let log_issue_from_summary_simplified severity summary ~loc ?(ltr = []) ?extras exn =
+let checker_exception issue_type error_message =
+  Exceptions.Checkers (issue_type, Localise.verbatim_desc error_message)
+
+
+let log_issue_from_summary_simplified severity summary ~loc ?(ltr = []) ?extras issue_type
+    error_message =
+  let exn = checker_exception issue_type error_message in
   log_issue_from_summary severity summary ~node:Errlog.UnknownNode ~session:0 ~loc ~ltr ?extras exn
 
 
@@ -77,7 +83,7 @@ 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 severity ~loc ~ltr ?access issue_type error_message =
-  let exn = Exceptions.Checkers (issue_type, Localise.verbatim_desc error_message) in
+  let exn = checker_exception issue_type error_message in
   let errlog = IssueLog.get_errlog procname in
   let node = Errlog.UnknownNode in
   log_issue_from_errlog procname ~clang_method_kind:None severity errlog ~loc ~node ~session:0 ~ltr

infer/src/backend/reporting.mli

@@ -9,7 +9,7 @@ open! IStd
 
 (** Type of functions to report issues to the error_log in a spec. *)
 
-type log_t = ?ltr:Errlog.loc_trace -> ?extras:Jsonbug_t.extra -> exn -> unit
+type log_t = ?ltr:Errlog.loc_trace -> ?extras:Jsonbug_t.extra -> IssueType.t -> string -> unit
 
 val log_issue_deprecated_using_state :
      Exceptions.severity

infer/src/bufferoverrun/bufferOverrunChecker.ml

@@ -423,8 +423,7 @@ module Report = struct
         let issue_type =
           if true_branch then IssueType.condition_always_false else IssueType.condition_always_true
         in
-        let exn = Exceptions.Checkers (issue_type, Localise.verbatim_desc desc) in
-        Reporting.log_warning summary ~loc:location exn
+        Reporting.log_warning summary ~loc:location issue_type desc
     (* special case for `exit` when we're at the end of a block / procedure *)
     | Sil.Call (_, Const (Cfun pname), _, _, _)
       when String.equal (Typ.Procname.get_method pname) "exit"
@@ -432,12 +431,8 @@ module Report = struct
         ()
     | _ ->
         let location = Sil.instr_get_loc instr in
-        let exn =
-          Exceptions.Checkers
-            ( IssueType.unreachable_code_after
-            , Localise.verbatim_desc "Unreachable code after statement" )
-        in
-        Reporting.log_error summary ~loc:location exn
+        Reporting.log_error summary ~loc:location IssueType.unreachable_code_after
+          "Unreachable code after statement"
 
 
   let check_binop_array_access :
@@ -670,8 +665,6 @@ module Report = struct
     let report cond trace issue_type =
       let location = PO.ConditionTrace.get_report_location trace in
       let description = PO.description cond trace in
-      let error_desc = Localise.desc_buffer_overrun description in
-      let exn = Exceptions.Checkers (issue_type, error_desc) in
       let trace =
         match TraceSet.choose_shortest (PO.ConditionTrace.get_val_traces trace) with
         | trace ->
@@ -679,7 +672,7 @@ module Report = struct
         | exception _ ->
             [Errlog.make_trace_element 0 location description []]
       in
-      Reporting.log_error summary ~loc:location ~ltr:trace exn
+      Reporting.log_error summary ~loc:location ~ltr:trace issue_type description
     in
     PO.ConditionSet.check_all ~report cond_set
 

infer/src/checkers/Litho.ml

@@ -85,12 +85,9 @@ module GraphQLGetters = struct
       in
       let call_string = String.concat ~sep:"." call_strings in
       let message = F.asprintf "%a.%s" AccessPath.pp access_path call_string in
-      let exn =
-        Exceptions.Checkers (IssueType.graphql_field_access, Localise.verbatim_desc message)
-      in
       let loc = Summary.get_loc summary in
       let ltr = [Errlog.make_trace_element 0 loc message []] in
-      Reporting.log_error summary ~loc ~ltr exn
+      Reporting.log_error summary ~loc ~ltr IssueType.graphql_field_access message
     in
     Domain.iter_call_chains ~f:report_graphql_getter astate
 end
@@ -121,11 +118,8 @@ module RequiredProps = struct
       F.asprintf "@Prop %s is required for component %s, but is not set before the call to build()"
         prop_string (Typ.Name.name parent_typename)
     in
-    let exn =
-      Exceptions.Checkers (IssueType.missing_required_prop, Localise.verbatim_desc message)
-    in
     let ltr = [Errlog.make_trace_element 0 loc message []] in
-    Reporting.log_error summary ~loc ~ltr exn
+    Reporting.log_error summary ~loc ~ltr IssueType.missing_required_prop message
 
 
   (* walk backward through [call_chain] and return the first type T <: Component that is not part of

infer/src/checkers/NullabilityCheck.ml

@@ -134,9 +134,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
             ap MF.pp_monospaced annotation (MF.wrap_monospaced CallSite.pp) call_site Location.pp
             loc
       in
-      let exn =
-        Exceptions.Checkers (IssueType.nullable_dereference, Localise.verbatim_desc message)
-      in
       let trace =
         let with_origin_site =
           let callee_pname = CallSite.pname call_site in
@@ -171,7 +168,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         in
         dereference_site :: with_assignment_site
       in
-      Reporting.log_error summary ~loc ~ltr:trace exn
+      Reporting.log_error summary ~loc ~ltr:trace IssueType.nullable_dereference message
 
 
   let add_nullable_ap ap call_sites (aps, pnames) = (NullableAP.add ap call_sites aps, pnames)

infer/src/checkers/NullabilitySuggest.ml

@@ -184,21 +184,20 @@ let checker {Callbacks.summary; proc_desc; tenv} =
       | Some (field_name, _) when Typ.Fieldname.Java.is_captured_parameter field_name ->
           (* Skip reporting when field comes from generated code *)
           ()
-      | Some (field_name, _) -> (
+      | Some (field_name, _) ->
           let message =
             F.asprintf "Field %a should be annotated with %a" MF.pp_monospaced
               (pretty_field_name proc_data field_name)
               MF.pp_monospaced annotation
           in
-          let exn =
-            Exceptions.Checkers (IssueType.field_should_be_nullable, Localise.verbatim_desc message)
-          in
+          let loc, ltr =
             match make_error_trace astate ap udchain with
             | Some (loc, ltr) ->
-              Reporting.log_warning summary ~loc ~ltr exn
+                (loc, Some ltr)
             | None ->
-              let loc = Procdesc.get_loc proc_desc in
-              Reporting.log_warning summary ~loc exn )
+                (Procdesc.get_loc proc_desc, None)
+          in
+          Reporting.log_warning summary ~loc ?ltr IssueType.field_should_be_nullable message
       | _ ->
           ()
     in

infer/src/checkers/Ownership.ml

@@ -93,8 +93,7 @@ module Domain = struct
   include AbstractDomain.Map (Base) (CapabilityDomain)
 
   let report message loc ltr summary =
-    let exn = Exceptions.Checkers (IssueType.use_after_lifetime, Localise.verbatim_desc message) in
-    Reporting.log_error summary ~loc ~ltr exn
+    Reporting.log_error summary ~loc ~ltr IssueType.use_after_lifetime message
 
 
   let report_return_stack_var (var, _) loc summary =

infer/src/checkers/Siof.ml

@@ -238,11 +238,7 @@ let report_siof summary trace pdesc gname loc =
             gname GlobalVar.pp (SiofTrace.Sink.kind final_sink)
     in
     let ltr = SiofTrace.trace_of_error loc gname trace in
-    let exn =
-      Exceptions.Checkers
-        (IssueType.static_initialization_order_fiasco, Localise.verbatim_desc description)
-    in
-    Reporting.log_error summary ~loc ~ltr exn
+    Reporting.log_error summary ~loc ~ltr IssueType.static_initialization_order_fiasco description
   in
   let reportable_paths = SiofTrace.get_reportable_sink_paths trace ~trace_of_pname in
   if Config.filtering then List.hd reportable_paths |> Option.iter ~f:report_one_path

infer/src/checkers/annotationReachability.ml

@@ -141,10 +141,8 @@ let report_allocation_stack src_annot summary fst_call_loc trace stack_str const
       MF.pp_monospaced ("@" ^ src_annot) MF.pp_monospaced constr_str MF.pp_monospaced
       (stack_str ^ "new " ^ constr_str)
   in
-  let exn =
-    Exceptions.Checkers (IssueType.checkers_allocates_memory, Localise.verbatim_desc description)
-  in
-  Reporting.log_error summary ~loc:fst_call_loc ~ltr:final_trace exn
+  Reporting.log_error summary ~loc:fst_call_loc ~ltr:final_trace
+    IssueType.checkers_allocates_memory description
 
 
 let report_annotation_stack src_annot snk_annot src_summary loc trace stack_str snk_pname call_loc
@@ -162,13 +160,12 @@ let report_annotation_stack src_annot snk_annot src_summary loc trace stack_str
         MF.pp_monospaced ("@" ^ src_annot) MF.pp_monospaced (stack_str ^ exp_pname_str)
         MF.pp_monospaced exp_pname_str MF.pp_monospaced ("@" ^ snk_annot)
     in
-    let msg =
+    let issue_type =
       if String.equal src_annot Annotations.performance_critical then
         IssueType.checkers_calls_expensive_method
       else IssueType.checkers_annotation_reachability_error
     in
-    let exn = Exceptions.Checkers (msg, Localise.verbatim_desc description) in
-    Reporting.log_error src_summary ~loc ~ltr:final_trace exn
+    Reporting.log_error src_summary ~loc ~ltr:final_trace issue_type description
 
 
 let report_call_stack summary end_of_stack lookup_next_calls report call_site sink_map =
@@ -299,11 +296,8 @@ module ExpensiveAnnotationSpec = struct
           (Typ.Procname.to_string overridden_pname)
           MF.pp_monospaced ("@" ^ Annotations.expensive)
       in
-      let exn =
-        Exceptions.Checkers
-          (IssueType.checkers_expensive_overrides_unexpensive, Localise.verbatim_desc description)
-      in
-      Reporting.log_error summary ~loc exn
+      Reporting.log_error summary ~loc IssueType.checkers_expensive_overrides_unexpensive
+        description
 
 
   let spec =

infer/src/checkers/cost.ml

@@ -644,16 +644,14 @@ module TransferFunctionsWCET = struct
       let cost_desc = F.asprintf "with estimated cost %a%s" BasicCost.pp cost degree_str in
       [Errlog.make_trace_element 0 loc cost_desc []]
     in
-    let exn =
     let message =
       F.asprintf
-          "The execution time from the beginning of the function up to this program point is \
-           likely above the acceptable threshold of %a (estimated cost %a%s)"
+        "The execution time from the beginning of the function up to this program point is likely \
+         above the acceptable threshold of %a (estimated cost %a%s)"
         BasicCost.pp expensive_threshold BasicCost.pp cost degree_str
     in
-      Exceptions.Checkers (IssueType.expensive_execution_time_call, Localise.verbatim_desc message)
-    in
-    Reporting.log_error summary ~loc ~ltr ~extras:(compute_errlog_extras cost) exn
+    Reporting.log_error summary ~loc ~ltr ~extras:(compute_errlog_extras cost)
+      IssueType.expensive_execution_time_call message
 
 
   (* get a list of nodes and check if we have already reported for at
@@ -723,23 +721,17 @@ end
 module AnalyzerWCET = AbstractInterpreter.MakeNoCFG (InstrCFGScheduler) (TransferFunctionsWCET)
 
 let check_and_report_top_and_bottom cost proc_desc summary =
+  let report issue suffix =
     let message =
       F.asprintf "The execution time of the function %a %s" Typ.Procname.pp
         (Procdesc.get_proc_name proc_desc)
+        suffix
     in
-  let exn_opt =
-    if BasicCost.is_top cost then
-      let msg = message "cannot be computed" in
-      Some
-        (Exceptions.Checkers (IssueType.infinite_execution_time_call, Localise.verbatim_desc msg))
-    else if BasicCost.is_zero cost then
-      let msg = message "is zero" in
-      Some (Exceptions.Checkers (IssueType.zero_execution_time_call, Localise.verbatim_desc msg))
-    else None
+    let loc = Procdesc.get_start_node proc_desc |> Procdesc.Node.get_loc in
+    Reporting.log_error ~loc ~extras:(compute_errlog_extras cost) summary issue message
   in
-  let loc () = Procdesc.get_start_node proc_desc |> Procdesc.Node.get_loc in
-  Option.iter exn_opt
-    ~f:(Reporting.log_error ~loc:(loc ()) ~extras:(compute_errlog_extras cost) summary)
+  if BasicCost.is_top cost then report IssueType.infinite_execution_time_call "cannot be computed"
+  else if BasicCost.is_zero cost then report IssueType.zero_execution_time_call "is zero"
 
 
 let checker ({Callbacks.tenv; proc_desc} as callback_args) : Summary.t =

infer/src/checkers/fragmentRetainsViewChecker.ml

@@ -9,12 +9,40 @@ open! IStd
 
 (** Make sure callbacks are always unregistered. drive the point home by reporting possible NPE's *)
 
+let rec format_typ typ =
+  match typ.Typ.desc with
+  | Typ.Tptr (t, _) when Language.curr_language_is Java ->
+      format_typ t
+  | Typ.Tstruct name ->
+      Typ.Name.name name
+  | _ ->
+      Typ.to_string typ
+
+
+let format_field f =
+  if Language.curr_language_is Java then Typ.Fieldname.Java.get_field f
+  else Typ.Fieldname.to_string f
+
+
+let format_method pname =
+  match pname with
+  | Typ.Procname.Java pname_java ->
+      Typ.Procname.Java.get_method pname_java
+  | _ ->
+      Typ.Procname.to_string pname
+
+
 let report_error fragment_typ fld fld_typ summary pdesc =
   let pname = Procdesc.get_proc_name pdesc in
-  let description = Localise.desc_fragment_retains_view fragment_typ fld fld_typ pname in
-  let exn = Exceptions.Checkers (IssueType.checkers_fragment_retain_view, description) in
+  let description =
+    Printf.sprintf
+      "Fragment %s does not nullify View field %s (type %s) in %s. If this Fragment is placed on \
+       the back stack, a reference to this (probably dead) View will be retained. In general, it \
+       is a good idea to initialize View's in onCreateView, then nullify them in onDestroyView."
+      (format_typ fragment_typ) (format_field fld) (format_typ fld_typ) (format_method pname)
+  in
   let loc = Procdesc.get_loc pdesc in
-  Reporting.log_error summary ~loc exn
+  Reporting.log_error summary ~loc IssueType.checkers_fragment_retain_view description
 
 
 let callback_fragment_retains_view_java pname_java {Callbacks.proc_desc; summary; tenv} =

infer/src/checkers/hoisting.ml

@@ -67,13 +67,10 @@ let do_report summary Call.({pname; loc}) loop_head_loc =
     F.asprintf "Loop-invariant call to %a at %a" Typ.Procname.pp pname Location.pp loc
   in
   let ltr = [Errlog.make_trace_element 0 loc exp_desc []] in
-  let exn =
   let message =
     F.asprintf "%s can be moved out of the loop at %a." exp_desc Location.pp loop_head_loc
   in
-    Exceptions.Checkers (IssueType.invariant_call, Localise.verbatim_desc message)
-  in
-  Reporting.log_error summary ~loc ~ltr exn
+  Reporting.log_error summary ~loc ~ltr IssueType.invariant_call message
 
 
 let checker {Callbacks.tenv; summary; proc_desc} : Summary.t =

infer/src/checkers/liveness.ml

@@ -173,8 +173,7 @@ let checker {Callbacks.tenv; summary; proc_desc} : Summary.t =
         (Typ.pp_full Pp.text) typ
     in
     let ltr = [Errlog.make_trace_element 0 loc "Write of unused value" []] in
-    let exn = Exceptions.Checkers (IssueType.dead_store, Localise.verbatim_desc message) in
-    Reporting.log_error summary ~loc ~ltr exn
+    Reporting.log_error summary ~loc ~ltr IssueType.dead_store message
   in
   let report_dead_store live_vars captured_by_ref_vars = function
     | Sil.Store (Lvar pvar, typ, rhs_exp, loc)

infer/src/checkers/printfArgs.ml

@@ -117,10 +117,7 @@ let check_printf_args_ok tenv (node : Procdesc.Node.t) (instr : Sil.instr)
               "%s at line %s: parameter %d is expected to be of type %s but %s was given."
               instr_name instr_line n_arg (default_format_type_name ft) gt
           in
-          let exn =
-            Exceptions.Checkers (IssueType.checkers_printf_args, Localise.verbatim_desc description)
-          in
-          Reporting.log_error summary ~loc:instr_loc exn
+          Reporting.log_error summary ~loc:instr_loc IssueType.checkers_printf_args description
         else check_type_names instr_loc (n_arg + 1) instr_proc_name fs gs
     | [], [] ->
         ()
@@ -129,10 +126,7 @@ let check_printf_args_ok tenv (node : Procdesc.Node.t) (instr : Sil.instr)
           Printf.sprintf "format string arguments don't mach provided arguments in %s at line %s"
             instr_name instr_line
         in
-        let exn =
-          Exceptions.Checkers (IssueType.checkers_printf_args, Localise.verbatim_desc description)
-        in
-        Reporting.log_error summary ~loc:instr_loc exn
+        Reporting.log_error summary ~loc:instr_loc IssueType.checkers_printf_args description
   in
   (* Get the array ivar for a given nvar *)
   let array_ivar instrs nvar =
@@ -183,12 +177,8 @@ let check_printf_args_ok tenv (node : Procdesc.Node.t) (instr : Sil.instr)
               (fixed_nvar_type_names @ vararg_ivar_type_names)
         | None ->
             if not (Reporting.is_suppressed tenv proc_desc IssueType.checkers_printf_args) then
-              let exn =
-                Exceptions.Checkers
-                  ( IssueType.checkers_printf_args
-                  , Localise.verbatim_desc "Format string must be string literal" )
-              in
-              Reporting.log_warning summary ~loc:cl exn
+              Reporting.log_warning summary ~loc:cl IssueType.checkers_printf_args
+                "Format string must be string literal"
       with e ->
         L.internal_error "%s Exception when analyzing %s: %s@."
           IssueType.checkers_printf_args.unique_id

infer/src/checkers/uninit.ml

@@ -49,10 +49,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
       F.asprintf "The value read from %a was never initialized" AccessExpression.pp access_expr
     in
     let ltr = [Errlog.make_trace_element 0 loc "" []] in
-    let exn =
-      Exceptions.Checkers (IssueType.uninitialized_value, Localise.verbatim_desc message)
-    in
-    Reporting.log_error summary ~loc ~ltr exn
+    Reporting.log_error summary ~loc ~ltr IssueType.uninitialized_value message
 
 
   type extras = FormalMap.t * Summary.t

infer/src/labs/ResourceLeaks.ml

@@ -106,8 +106,7 @@ let checker {Callbacks.summary; proc_desc; tenv} : Summary.t =
     if leak_count > 0 (* 2(a) *) then
       let last_loc = Procdesc.Node.get_loc (Procdesc.get_exit_node proc_data.pdesc) in
       let message = F.asprintf "Leaked %d resource(s)" leak_count in
-      let exn = Exceptions.Checkers (IssueType.resource_leak, Localise.verbatim_desc message) in
-      Reporting.log_error summary ~loc:last_loc exn
+      Reporting.log_error summary ~loc:last_loc IssueType.resource_leak message
   in
   (* Convert the abstract state to a summary. for now, just the identity function *)
   let convert_to_summary (post : Domain.astate) : Domain.summary =

infer/src/quandary/TaintAnalysis.ml

@@ -315,8 +315,8 @@ module Make (TaintSpecification : TaintSpec.S) = struct
           get_short_trace_string initial_source initial_source_caller final_sink final_sink_caller
         in
         let ltr = source_trace @ List.rev sink_trace in
-        let exn = Exceptions.Checkers (issue, Localise.verbatim_desc trace_str) in
-        Reporting.log_error proc_data.extras.summary ~loc:(CallSite.loc cur_site) ~ltr exn
+        Reporting.log_error proc_data.extras.summary ~loc:(CallSite.loc cur_site) ~ltr issue
+          trace_str
       in
       List.iter ~f:report_one (TraceDomain.get_reports ~cur_site trace)