Reporting cleanup 2: Errlog.log_issue

Summary:
- made arguments of `Errlog.log_issue` mandatory
- pushed some arguments of `log_issue_from_errlog` higher in the stack, the goal is to make sure `State` is only used in analyses that update it (biabduction and eradicate, if I'm correct)

Reviewed By: jvillard

Differential Revision: D9332773

fbshipit-source-id: ce79df21c

infer/src/IR/Errlog.ml

@@ -213,8 +213,8 @@ let update errlog_old errlog_new =
   ErrLogHash.iter (fun err_key l -> ignore (add_issue errlog_old err_key l)) errlog_new
 
 
-let log_issue procname ?clang_method_kind severity err_log loc (node_id, node_key) session ltr
-    ?linters_def_file ?doc_url ?access ?extras exn =
+let log_issue procname ~clang_method_kind severity err_log ~loc ~node_id:(node_id, node_key)
+    ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn =
   let lang = Typ.Procname.get_language procname in
   let error = Exceptions.recognize_exception exn in
   let severity = Option.value error.severity ~default:severity in

infer/src/IR/Errlog.mli

@@ -93,6 +93,6 @@ val update : t -> t -> unit
 (** Update an old error log with a new one *)
 
 val log_issue :
-  Typ.Procname.t -> ?clang_method_kind:string -> Exceptions.severity -> t -> Location.t
-  -> int * Caml.Digest.t -> int -> loc_trace -> ?linters_def_file:string -> ?doc_url:string
-  -> ?access:string -> ?extras:Jsonbug_t.extra -> exn -> unit
+  Typ.Procname.t -> clang_method_kind:string option -> Exceptions.severity -> t -> loc:Location.t
+  -> node_id:int * Caml.Digest.t -> session:int -> ltr:loc_trace -> linters_def_file:string option
+  -> doc_url:string option -> access:string option -> extras:Jsonbug_t.extra option -> exn -> unit

infer/src/backend/reporting.ml

@@ -13,36 +13,25 @@ type log_t =
   -> ?linters_def_file:string -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra
   -> exn -> unit
 
-type log_issue_from_errlog = Errlog.t -> log_t
-
-let log_issue_from_errlog procname ?clang_method_kind severity err_log ?loc ?node_id ?session ?ltr
-    ?linters_def_file ?doc_url ?access ?extras exn =
-  let clang_method_kind =
-    Option.map clang_method_kind ~f:ProcAttributes.string_of_clang_method_kind
-  in
+let log_issue_from_errlog_internal procname ~clang_method_kind severity err_log ~loc ~node_id
+    ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn =
   let issue_type = (Exceptions.recognize_exception exn).name in
   if not Config.filtering (* no-filtering takes priority *) || issue_type.IssueType.enabled then
-    let loc = match loc with None -> State.get_loc () | Some loc -> loc in
-    let node_id =
-      match node_id with
-      | None ->
-          (State.get_node_id_key () :> int * Caml.Digest.t)
-      | Some node_id ->
-          node_id
-    in
-    let session =
-      match session with None -> (State.get_session () :> int) | Some session -> session
-    in
-    let ltr = match ltr with None -> State.get_loc_trace () | Some ltr -> ltr in
-    Errlog.log_issue procname ?clang_method_kind severity err_log loc node_id session ltr
-      ?linters_def_file ?doc_url ?access ?extras exn
+    Errlog.log_issue procname ~clang_method_kind severity err_log ~loc ~node_id ~session ~ltr
+      ~linters_def_file ~doc_url ~access ~extras exn
+
+
+let log_issue_from_errlog procname severity errlog ~loc ~node_id ~ltr ~linters_def_file ~doc_url
+    exn =
+  let session = (State.get_session () :> int) in
+  log_issue_from_errlog_internal procname ~clang_method_kind:None severity errlog ~loc ~node_id
+    ~session ~ltr ~linters_def_file ~doc_url ~access:None ~extras:None exn
 
 
 let log_issue_from_summary severity summary ?loc ?node_id ?session ?ltr ?linters_def_file ?doc_url
     ?access ?extras exn =
   let attrs = Summary.get_attributes summary in
   let procname = attrs.proc_name in
-  let clang_method_kind = attrs.clang_method_kind in
   let is_java_generated_method =
     match procname with
     | Typ.Procname.Java java_pname ->
@@ -58,8 +47,23 @@ let log_issue_from_summary severity summary ?loc ?node_id ?session ?ltr ?linters
   if should_suppress_lint || is_java_generated_method then () (* Skip the reporting *)
   else
     let err_log = Summary.get_err_log summary in
-    log_issue_from_errlog procname ~clang_method_kind severity err_log ?loc ?node_id ?session ?ltr
-      ?linters_def_file ?doc_url ?access ?extras exn
+    let clang_method_kind =
+      Some (ProcAttributes.string_of_clang_method_kind attrs.clang_method_kind)
+    in
+    let loc = match loc with None -> State.get_loc () | Some loc -> loc in
+    let node_id =
+      match node_id with
+      | None ->
+          (State.get_node_id_key () :> int * Caml.Digest.t)
+      | Some node_id ->
+          node_id
+    in
+    let session =
+      match session with None -> (State.get_session () :> int) | Some session -> session
+    in
+    let ltr = match ltr with None -> State.get_loc_trace () | Some ltr -> ltr in
+    log_issue_from_errlog_internal procname ~clang_method_kind severity err_log ~loc ~node_id
+      ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn
 
 
 let log_issue_deprecated severity proc_name ?loc ?node_id ?session ?ltr ?linters_def_file ?doc_url
@@ -79,11 +83,12 @@ let log_error = log_issue_from_summary Exceptions.Error
 
 let log_warning = log_issue_from_summary Exceptions.Warning
 
-let log_issue_external procname ?clang_method_kind severity ?loc ?node_id ?session ?ltr
-    ?linters_def_file ?doc_url ?access ?extras exn =
+let log_issue_external procname severity ~loc ~ltr ?access exn =
   let errlog = IssueLog.get_errlog procname in
-  log_issue_from_errlog procname ?clang_method_kind severity errlog ?loc ?node_id ?session ?ltr
-    ?linters_def_file ?doc_url ?access ?extras exn
+  let node_id = (State.get_node_id_key () :> int * Caml.Digest.t) in
+  let session = (State.get_session () :> int) in
+  log_issue_from_errlog_internal procname ~clang_method_kind:None severity errlog ~loc ~node_id
+    ~session ~ltr ~linters_def_file:None ~doc_url:None ~access ~extras:None exn
 
 
 let is_suppressed ?(field_name= None) tenv proc_desc kind =

infer/src/backend/reporting.mli

@@ -14,16 +14,15 @@ type log_t =
   -> ?linters_def_file:string -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra
   -> exn -> unit
 
-type log_issue_from_errlog = Errlog.t -> log_t
-
 val log_issue_deprecated : Exceptions.severity -> Typ.Procname.t -> log_t
 (** Report an issue in the given procedure.
     DEPRECATED as it can create race conditions between checkers.
     Use log_error/warning instead *)
 
 val log_issue_from_errlog :
-  Typ.Procname.t -> ?clang_method_kind:ProcAttributes.clang_method_kind -> Exceptions.severity
-  -> log_issue_from_errlog
+  Typ.Procname.t -> Exceptions.severity -> Errlog.t -> loc:Location.t
+  -> node_id:int * Caml.Digest.t -> ltr:Errlog.loc_trace -> linters_def_file:string option
+  -> doc_url:string option -> exn -> unit
 (** Report an issue of a given kind  in the given error log. *)
 
 val log_error : Summary.t -> log_t
@@ -33,8 +32,8 @@ val log_warning : Summary.t -> log_t
 (** Add an warning to the given summary. *)
 
 val log_issue_external :
-  Typ.Procname.t -> ?clang_method_kind:ProcAttributes.clang_method_kind -> Exceptions.severity
-  -> log_t
+  Typ.Procname.t -> Exceptions.severity -> loc:Location.t -> ltr:Errlog.loc_trace -> ?access:string
+  -> exn -> unit
 (** Log an issue to the error log in [IssueLog] associated with the given procname. *)
 
 val is_suppressed :

infer/src/clang/cFrontend_errors.ml

@@ -458,7 +458,7 @@ let log_frontend_issue method_decl_opt (node: Ctl_parser_types.ast_node)
   in
   let key = Utils.better_hash key_str in
   Reporting.log_issue_from_errlog procname issue_desc.severity errlog exn ~loc:issue_desc.loc
-    ~ltr:trace ~node_id:(0, key) ?linters_def_file ?doc_url:issue_desc.doc_url
+    ~ltr:trace ~node_id:(0, key) ~linters_def_file ~doc_url:issue_desc.doc_url
 
 
 let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc: CIssue.issue_desc)