[infer] simplify the API to report errors

Summary: There was a back and forth conversion between `string` and `IssueType.t` which was not necessary.

Reviewed By: sblackshear

Differential Revision: D6562747

fbshipit-source-id: 70b57a2

infer/src/IR/Exceptions.ml

@@ -84,13 +84,13 @@ exception Double_lock of Localise.error_desc * L.ml_loc
 
 exception Empty_vector_access of Localise.error_desc * L.ml_loc
 
-exception Eradicate of string * Localise.error_desc
+exception Eradicate of IssueType.t * Localise.error_desc
 
 exception Field_not_null_checked of Localise.error_desc * L.ml_loc
 
 exception Frontend_warning of (string * string option) * Localise.error_desc * L.ml_loc
 
-exception Checkers of string * Localise.error_desc
+exception Checkers of IssueType.t * Localise.error_desc
 
 exception Inherently_dangerous_function of Localise.error_desc
 
@@ -337,8 +337,8 @@ let recognize_exception exn =
       ; severity= High
       ; kind= Some Kerror
       ; category= Prover }
-  | Eradicate (kind_s, desc) ->
+  | Eradicate (kind, desc) ->
-      { name= IssueType.from_string kind_s
+      { name= kind
       ; description= desc
       ; ml_loc= None
       ; visibility= Exn_user
@@ -369,8 +369,8 @@ let recognize_exception exn =
       ; severity= Medium
       ; kind= None
       ; category= Linters }
-  | Checkers (kind_s, desc) ->
+  | Checkers (kind, desc) ->
-      { name= IssueType.from_string kind_s
+      { name= kind
       ; description= desc
       ; ml_loc= None
       ; visibility= Exn_user
@@ -695,4 +695,3 @@ let pp_err ~node_key loc ekind ex_name desc ml_loc_opt fmt () =
 let handle_exception exn =
   let error = recognize_exception exn in
   equal_visibility error.visibility Exn_user || equal_visibility error.visibility Exn_developer
-

infer/src/IR/Exceptions.mli

@@ -84,9 +84,9 @@ exception Field_not_null_checked of Localise.error_desc * Logging.ml_loc
 
 exception Empty_vector_access of Localise.error_desc * Logging.ml_loc
 
-exception Eradicate of string * Localise.error_desc
+exception Eradicate of IssueType.t * Localise.error_desc
 
-exception Checkers of string * Localise.error_desc
+exception Checkers of IssueType.t * Localise.error_desc
 
 exception Frontend_warning of (string * string option) * Localise.error_desc * Logging.ml_loc
 

infer/src/absint/Checkers.ml

@@ -43,7 +43,7 @@ module ST = struct
       Localise.custom_desc_with_advice description (Option.value ~default:"" advice)
         [("always_report", string_of_bool always_report)]
     in
-    let exn = exception_kind kind.IssueType.unique_id localized_description in
+    let exn = exception_kind kind localized_description in
     let proc_attributes = Specs.pdesc_resolve_attributes proc_desc in
     (* Errors can be suppressed with annotations. An error of kind CHECKER_ERROR_NAME can be
        suppressed with the following annotations:

infer/src/absint/Checkers.mli

@@ -16,7 +16,7 @@ module ST : sig
   val report_error :
     Tenv.t -> Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t -> ?advice:string option
     -> ?field_name:Typ.Fieldname.t option -> ?origin_loc:Location.t option
-    -> ?exception_kind:(string -> Localise.error_desc -> exn) -> ?always_report:bool -> string
+    -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn) -> ?always_report:bool -> string
     -> unit
   (** Report an error. *)
 end

infer/src/base/IssueType.ml

@@ -108,6 +108,18 @@ let cannot_star = from_string "Cannot_star"
 
 let checkers_access_global = from_string "CHECKERS_ACCESS_GLOBAL"
 
+let checkers_allocates_memory = from_string "CHECKERS_ALLOCATES_MEMORY"
+
+let checkers_annotation_reachability_error = from_string "CHECKERS_ANNOTATION_REACHABILITY_ERROR"
+
+let checkers_calls_expensive_method = from_string "CHECKERS_CALLS_EXPENSIVE_METHOD"
+
+let checkers_expensive_overrides_unexpensive =
+  from_string "CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED"
+
+
+let checkers_fragment_retain_view = from_string "CHECKERS_FRAGMENT_RETAINS_VIEW"
+
 let checkers_immutable_cast = from_string "CHECKERS_IMMUTABLE_CAST"
 
 let checkers_print_c_call = from_string "CHECKERS_PRINT_C_CALL"

infer/src/base/IssueType.mli

@@ -59,6 +59,22 @@ val cannot_star : t
 
 val checkers_access_global : t
 
+val checkers_allocates_memory : t
+(** Warning name when a performance critical method directly or indirectly
+    calls a method allocating memory *)
+
+val checkers_annotation_reachability_error : t
+
+val checkers_calls_expensive_method : t
+(** Warning name when a performance critical method directly or indirectly
+    calls a method annotatd as expensive *)
+
+val checkers_expensive_overrides_unexpensive : t
+(** Warning name for the subtyping rule: method not annotated as expensive cannot be overridden
+    by a method annotated as expensive *)
+
+val checkers_fragment_retain_view : t
+
 val checkers_immutable_cast : t
 
 val checkers_print_c_call : t

infer/src/bufferoverrun/bufferOverrunChecker.ml

@@ -500,7 +500,7 @@ module Report = struct
         if Typ.Procname.equal pname caller_pname then
           let description = PO.description cond trace in
           let error_desc = Localise.desc_buffer_overrun description in
-          let exn = Exceptions.Checkers (issue_type.IssueType.unique_id, error_desc) in
+          let exn = Exceptions.Checkers (issue_type, error_desc) in
           let trace =
             match TraceSet.choose_shortest trace.PO.ConditionTrace.val_traces with
             | trace ->

infer/src/checkers/NullabilityCheck.ml

@@ -63,7 +63,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
   let report_nullable_dereference ap call_sites {ProcData.pdesc; extras} loc =
     let pname = Procdesc.get_proc_name pdesc in
     let annotation = Localise.nullable_annotation_name pname in
-    let issue_kind = IssueType.nullable_dereference.unique_id in
     let call_site =
       try CallSites.min_elt call_sites with Not_found ->
         L.(die InternalError)
@@ -94,7 +93,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
           (MF.wrap_monospaced AccessPath.pp)
           ap MF.pp_monospaced annotation (MF.wrap_monospaced CallSite.pp) call_site Location.pp loc
     in
-    let exn = Exceptions.Checkers (issue_kind, Localise.verbatim_desc message) in
+    let exn =
+      Exceptions.Checkers (IssueType.nullable_dereference, Localise.verbatim_desc message)
+    in
     let summary = extras in
     let trace =
       let with_origin_site =

infer/src/checkers/NullabilitySuggest.ml

@@ -171,7 +171,6 @@ let checker {Callbacks.summary; proc_desc; tenv} =
   let annotation = Localise.nullable_annotation_name (Procdesc.get_proc_name proc_desc) in
   let report astate (proc_data: extras ProcData.t) =
     let report_access_path ap udchain =
-      let issue_kind = IssueType.field_should_be_nullable.unique_id in
       match AccessPath.get_field_and_annotation ap proc_data.tenv with
       | Some (field_name, _) when is_outside_codebase proc_desc tenv field_name ->
           (* Skip reporting when the field is outside the analyzed codebase *)
@@ -186,7 +185,9 @@ let checker {Callbacks.summary; proc_desc; tenv} =
               (pretty_field_name proc_data field_name)
               MF.pp_monospaced annotation
           in
-          let exn = Exceptions.Checkers (issue_kind, Localise.verbatim_desc message) in
+          let exn =
+            Exceptions.Checkers (IssueType.field_should_be_nullable, Localise.verbatim_desc message)
+          in
           match make_error_trace astate ap udchain with
           | Some (loc, ltr) ->
               Reporting.log_warning summary ~loc ~ltr exn

infer/src/checkers/ShouldUpdate.ml

@@ -69,8 +69,7 @@ let report receiver call_chain summary =
   let call_strings = List.map ~f:(Typ.Procname.to_simplified_string ~withclass:false) call_chain in
   let call_string = String.concat ~sep:"." call_strings in
   let message = F.asprintf "GraphQL getter chain %a.%s" AccessPath.pp receiver call_string in
-  let issue_id = IssueType.graphql_field_access.unique_id in
+  let exn = Exceptions.Checkers (IssueType.graphql_field_access, Localise.verbatim_desc message) in
-  let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in
   let loc = Specs.get_loc summary in
   Reporting.log_error summary ~loc exn
 
@@ -116,4 +115,3 @@ let checker {Callbacks.summary; proc_desc; tenv} =
       summary
   | None ->
       summary
-

infer/src/checkers/Siof.ml

@@ -207,8 +207,10 @@ 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 msg = IssueType.static_initialization_order_fiasco.unique_id in
+    let exn =
-    let exn = Exceptions.Checkers (msg, Localise.verbatim_desc description) in
+      Exceptions.Checkers
+        (IssueType.static_initialization_order_fiasco, Localise.verbatim_desc description)
+    in
     Reporting.log_error summary ~loc ~ltr exn
   in
   let reportable_paths = SiofTrace.get_reportable_sink_paths trace ~trace_of_pname in

infer/src/checkers/annotationReachability.ml

@@ -72,20 +72,6 @@ module Summary = Summary.Make (struct
   let read_payload (summary: Specs.summary) = summary.payload.annot_map
 end)
 
-(* Warning name when a performance critical method directly or indirectly
-   calls a method annotatd as expensive *)
-let calls_expensive_method = "CHECKERS_CALLS_EXPENSIVE_METHOD"
-
-(* Warning name when a performance critical method directly or indirectly
-   calls a method allocating memory *)
-let allocates_memory = "CHECKERS_ALLOCATES_MEMORY"
-
-(* Warning name for the subtyping rule: method not annotated as expensive cannot be overridden
-   by a method annotated as expensive *)
-let expensive_overrides_unexpensive = "CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED"
-
-let annotation_reachability_error = "CHECKERS_ANNOTATION_REACHABILITY_ERROR"
-
 let is_modeled_expensive tenv = function
   | Typ.Procname.Java proc_name_java as proc_name ->
       not (BuiltinDecl.is_declared proc_name)
@@ -160,7 +146,9 @@ 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 (allocates_memory, Localise.verbatim_desc description) 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
 
 
@@ -179,8 +167,9 @@ let report_annotation_stack src_annot snk_annot src_summary loc trace stack_str
         MF.pp_monospaced exp_pname_str MF.pp_monospaced ("@" ^ snk_annot)
     in
     let msg =
-      if String.equal src_annot Annotations.performance_critical then calls_expensive_method
+      if String.equal src_annot Annotations.performance_critical then
-      else annotation_reachability_error
+        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
@@ -319,7 +308,8 @@ module ExpensiveAnnotationSpec = struct
           MF.pp_monospaced ("@" ^ Annotations.expensive)
       in
       let exn =
-        Exceptions.Checkers (expensive_overrides_unexpensive, Localise.verbatim_desc description)
+        Exceptions.Checkers
+          (IssueType.checkers_expensive_overrides_unexpensive, Localise.verbatim_desc description)
       in
       Reporting.log_error summary ~loc exn
 

infer/src/checkers/fragmentRetainsViewChecker.ml

@@ -15,9 +15,8 @@ module P = Printf
 
 let report_error fragment_typ fld fld_typ summary pdesc =
   let pname = Procdesc.get_proc_name pdesc in
-  let retained_view = "CHECKERS_FRAGMENT_RETAINS_VIEW" in
   let description = Localise.desc_fragment_retains_view fragment_typ fld fld_typ pname in
-  let exn = Exceptions.Checkers (retained_view, description) in
+  let exn = Exceptions.Checkers (IssueType.checkers_fragment_retain_view, description) in
   let loc = Procdesc.get_loc pdesc in
   Reporting.log_error summary ~loc exn
 
@@ -64,4 +63,3 @@ let callback_fragment_retains_view ({Callbacks.summary} as args) : Specs.summary
   | _ ->
       () ) ;
   summary
-

infer/src/checkers/liveness.ml

@@ -147,13 +147,12 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary =
       || is_scope_guard typ || Procdesc.has_modify_in_block_attr proc_desc pvar )
   in
   let log_report pvar typ loc =
-    let issue_id = IssueType.dead_store.unique_id in
     let message =
       F.asprintf "The value written to %a (type %a) is never used" (Pvar.pp Pp.text) pvar
         (Typ.pp_full Pp.text) typ
     in
     let ltr = [Errlog.make_trace_element 0 loc "Write of unused value" []] in
-    let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in
+    let exn = Exceptions.Checkers (IssueType.dead_store, Localise.verbatim_desc message) in
     Reporting.log_error summary ~loc ~ltr exn
   in
   let report_dead_store live_vars = function
@@ -181,4 +180,3 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary =
   in
   List.iter (CFG.nodes cfg) ~f:report_on_node ;
   summary
-

infer/src/checkers/printfArgs.ml

@@ -119,7 +119,9 @@ 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 (description, Localise.verbatim_desc description) in
+          let exn =
+            Exceptions.Checkers (IssueType.checkers_printf_args, Localise.verbatim_desc description)
+          in
           Reporting.log_error summary ~loc:instr_loc exn
         else check_type_names instr_loc (n_arg + 1) instr_proc_name fs gs
     | [], [] ->
@@ -129,7 +131,9 @@ 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 (description, Localise.verbatim_desc description) in
+        let exn =
+          Exceptions.Checkers (IssueType.checkers_printf_args, Localise.verbatim_desc description)
+        in
         Reporting.log_error summary ~loc:instr_loc exn
   in
   (* Get the array ivar for a given nvar *)

infer/src/checkers/uninit.ml

@@ -55,9 +55,10 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
 
   let report ap loc summary =
     let message = F.asprintf "The value read from %a was never initialized" AccessPath.pp ap in
-    let issue_id = IssueType.uninitialized_value.unique_id in
     let ltr = [Errlog.make_trace_element 0 loc "" []] in
-    let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in
+    let exn =
+      Exceptions.Checkers (IssueType.uninitialized_value, Localise.verbatim_desc message)
+    in
     Reporting.log_error summary ~loc ~ltr exn
 
 

infer/src/concurrency/RacerD.ml

@@ -1197,9 +1197,7 @@ let report_thread_safety_violation tenv pdesc ~make_description ~report_kind acc
       (* why we are reporting it *)
       let issue_type, explanation = get_reporting_explanation report_kind tenv pname thread in
       let error_message = F.sprintf "%s%s" description explanation in
-      let exn =
+      let exn = Exceptions.Checkers (issue_type, Localise.verbatim_desc error_message) in
-        Exceptions.Checkers (issue_type.IssueType.unique_id, Localise.verbatim_desc error_message)
-      in
       let end_locs = Option.to_list original_end @ Option.to_list conflict_end in
       let access = IssueAuxData.encode (pname, access, end_locs) in
       Reporting.log_error_deprecated ~store_summary:true pname ~loc ~ltr ~access exn

infer/src/eradicate/typeErr.ml

@@ -273,7 +273,7 @@ end
 type st_report_error =
   Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t -> ?advice:string option
   -> ?field_name:Typ.Fieldname.t option -> ?origin_loc:Location.t option
-  -> ?exception_kind:(string -> Localise.error_desc -> exn) -> ?always_report:bool -> string
+  -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn) -> ?always_report:bool -> string
   -> unit
 
 (** Report an error right now. *)
@@ -507,4 +507,3 @@ let report_forall_checks_and_reset tenv st_report_error proc_desc =
         ()
   in
   H.iter iter err_tbl ; reset ()
-

infer/src/eradicate/typeErr.mli

@@ -75,7 +75,7 @@ val node_reset_forall : Procdesc.Node.t -> unit
 type st_report_error =
   Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t -> ?advice:string option
   -> ?field_name:Typ.Fieldname.t option -> ?origin_loc:Location.t option
-  -> ?exception_kind:(string -> Localise.error_desc -> exn) -> ?always_report:bool -> string
+  -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn) -> ?always_report:bool -> string
   -> unit
 
 val report_error :

infer/src/labs/ResourceLeaks.ml

@@ -106,9 +106,8 @@ let checker {Callbacks.summary; proc_desc; tenv} : Specs.summary =
   let report leak_count (proc_data: extras ProcData.t) =
     if leak_count > 0 (* 2(a) *) then
       let last_loc = Procdesc.Node.get_loc (Procdesc.get_exit_node proc_data.pdesc) in
-      let issue_kind = IssueType.resource_leak.unique_id in
       let message = F.asprintf "Leaked %d resource(s)" leak_count in
-      let exn = Exceptions.Checkers (issue_kind, Localise.verbatim_desc message) in
+      let exn = Exceptions.Checkers (IssueType.resource_leak, Localise.verbatim_desc message) in
       Reporting.log_error summary ~loc:last_loc exn
   in
   (* Convert the abstract state to a summary. for now, just the identity function *)
@@ -127,4 +126,3 @@ let checker {Callbacks.summary; proc_desc; tenv} : Specs.summary =
       L.(die InternalError)
         "Analyzer failed to compute post for %a" Typ.Procname.pp
         (Procdesc.get_proc_name proc_data.pdesc)
-

infer/src/quandary/TaintAnalysis.ml

@@ -296,7 +296,7 @@ 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.unique_id, Localise.verbatim_desc trace_str) 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
       in
       List.iter ~f:report_one (TraceDomain.get_reports ~cur_site trace)

infer/tests/codetoanalyze/java/checkers/issues.exp

@@ -45,9 +45,9 @@ codetoanalyze/java/checkers/NullableViolation.java, void NullableViolation.deref
 codetoanalyze/java/checkers/NullableViolation.java, void NullableViolation.dereferenceNullableMethodNotAlwaysCheckedForNullBad(), 2, NULLABLE_DEREFERENCE, [dereferencing the return of NullableViolation.returnsNullable(),definition of returnsNullable]
 codetoanalyze/java/checkers/NullableViolation.java, void NullableViolation.dereferenceNullableReturnValueBad(), 2, NULLABLE_DEREFERENCE, [dereference of &t,assignment of the nullable value,definition of returnsNullable]
 codetoanalyze/java/checkers/PrintfArgsChecker.java, void PrintfArgsChecker.formatStringIsNotLiteral(PrintStream), 2, CHECKERS_PRINTF_ARGS, [Format string must be string literal]
-codetoanalyze/java/checkers/PrintfArgsChecker.java, void PrintfArgsChecker.stringInsteadOfInteger(PrintStream), 1, printf(...) at line 40: parameter 2 is expected to be of type java.lang.Integer but java.lang.String was given., []
+codetoanalyze/java/checkers/PrintfArgsChecker.java, void PrintfArgsChecker.stringInsteadOfInteger(PrintStream), 1, CHECKERS_PRINTF_ARGS, []
-codetoanalyze/java/checkers/PrintfArgsChecker.java, void PrintfArgsChecker.wrongNumberOfArguments(PrintStream), 1, format string arguments don't mach provided arguments in printf(...) at line 44, []
+codetoanalyze/java/checkers/PrintfArgsChecker.java, void PrintfArgsChecker.wrongNumberOfArguments(PrintStream), 1, CHECKERS_PRINTF_ARGS, []
-codetoanalyze/java/checkers/PrintfArgsChecker.java, void SuppressedPrintfArgsChecker.classSuppressed(PrintStream), 1, printf(...) at line 68: parameter 2 is expected to be of type java.lang.Integer but java.lang.String was given., []
+codetoanalyze/java/checkers/PrintfArgsChecker.java, void SuppressedPrintfArgsChecker.classSuppressed(PrintStream), 1, CHECKERS_PRINTF_ARGS, []
 codetoanalyze/java/checkers/TwoCheckersExample.java, List TwoCheckersExample.shouldRaiseImmutableCastError(), 0, CHECKERS_IMMUTABLE_CAST, [Method shouldRaiseImmutableCastError() returns class com.google.common.collect.ImmutableList but the return type is class java.util.List. Make sure that users of this method do not try to modify the collection.]
 codetoanalyze/java/checkers/TwoCheckersExample.java, List TwoCheckersExample.shouldRaisePerformanceCriticalError(), 1, CHECKERS_CALLS_EXPENSIVE_METHOD, []
 codetoanalyze/java/checkers/UiThreads.java, void UiThreads.callForNonUiThreadBad1(), 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, []