From 249ef9d75be09fe889fb4ac5367a69707b5e437d Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Thu, 6 Sep 2018 12:55:28 -0700 Subject: [PATCH] Reporting cleanup 17: move doc_url from reporting to issue type Reviewed By: jeremydubreil Differential Revision: D9654073 fbshipit-source-id: 77f3fddd1 --- infer/src/IR/Exceptions.ml | 7 ++++--- infer/src/IR/Exceptions.mli | 4 +++- infer/src/backend/reporting.ml | 25 ++++++++++++------------- infer/src/backend/reporting.mli | 2 -- infer/src/base/IssueType.ml | 16 +++++++++++----- infer/src/base/IssueType.mli | 5 +++-- infer/src/clang/cFrontend_errors.ml | 7 +++++-- 7 files changed, 38 insertions(+), 28 deletions(-) diff --git a/infer/src/IR/Exceptions.ml b/infer/src/IR/Exceptions.ml index bd5c9c015..5f9272f89 100644 --- a/infer/src/IR/Exceptions.ml +++ b/infer/src/IR/Exceptions.ml @@ -78,7 +78,8 @@ exception Eradicate of IssueType.t * Localise.error_desc exception Field_not_null_checked of Localise.error_desc * L.ocaml_pos -exception Frontend_warning of (string * string option) * Localise.error_desc * L.ocaml_pos +exception + Frontend_warning of (string * string option * string option) * Localise.error_desc * L.ocaml_pos exception Checkers of IssueType.t * Localise.error_desc @@ -320,8 +321,8 @@ let recognize_exception exn = ; visibility= Exn_user ; severity= Some Warning ; category= Nocat } - | Frontend_warning ((name, hum), desc, ocaml_pos) -> - { name= IssueType.from_string name ?hum + | Frontend_warning ((name, hum, doc_url), desc, ocaml_pos) -> + { name= IssueType.from_string name ?hum ?doc_url ; description= desc ; ocaml_pos= Some ocaml_pos ; visibility= Exn_user diff --git a/infer/src/IR/Exceptions.mli b/infer/src/IR/Exceptions.mli index c26a11a2f..b407168e0 100644 --- a/infer/src/IR/Exceptions.mli +++ b/infer/src/IR/Exceptions.mli @@ -79,7 +79,9 @@ exception Eradicate of IssueType.t * Localise.error_desc exception Checkers of IssueType.t * Localise.error_desc -exception Frontend_warning of (string * string option) * Localise.error_desc * Logging.ocaml_pos +exception + Frontend_warning of + (string * string option * string option) * Localise.error_desc * Logging.ocaml_pos exception Inherently_dangerous_function of Localise.error_desc diff --git a/infer/src/backend/reporting.ml b/infer/src/backend/reporting.ml index 44dbb6cd1..b75326f68 100644 --- a/infer/src/backend/reporting.ml +++ b/infer/src/backend/reporting.ml @@ -11,29 +11,28 @@ module L = Logging type log_t = ?ltr:Errlog.loc_trace -> ?linters_def_file:string - -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra -> exn -> unit let log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr - ~linters_def_file ~doc_url ~access ~extras exn = + ~linters_def_file ~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 doc_url = issue_type.doc_url in Errlog.log_issue procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn -let log_frontend_issue procname severity errlog ~loc ~node_key ~ltr ~linters_def_file ~doc_url exn - = +let log_frontend_issue procname severity errlog ~loc ~node_key ~ltr ~linters_def_file exn = let node = Errlog.FrontendNode {node_key} in log_issue_from_errlog procname ~clang_method_kind:None severity errlog ~loc ~node ~session:0 ~ltr - ~linters_def_file ~doc_url ~access:None ~extras:None exn + ~linters_def_file ~access:None ~extras:None exn -let log_issue_from_summary severity summary ~node ~session ~loc ~ltr ?linters_def_file ?doc_url - ?access ?extras exn = +let log_issue_from_summary severity summary ~node ~session ~loc ~ltr ?linters_def_file ?access + ?extras exn = let attrs = Summary.get_attributes summary in let procname = attrs.proc_name in let is_java_generated_method = @@ -53,7 +52,7 @@ let log_issue_from_summary severity summary ~node ~session ~loc ~ltr ?linters_de let err_log = Summary.get_err_log summary in let clang_method_kind = Some attrs.clang_method_kind in log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr - ~linters_def_file ~doc_url ~access ~extras exn + ~linters_def_file ~access ~extras exn let log_issue_deprecated_using_state severity proc_name ?node ?loc ?ltr exn = @@ -74,10 +73,10 @@ 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 = []) ?linters_def_file ?doc_url - ?access ?extras exn = +let log_issue_from_summary_simplified severity summary ~loc ?(ltr = []) ?linters_def_file ?access + ?extras exn = log_issue_from_summary severity summary ~node:Errlog.UnknownNode ~session:0 ~loc ~ltr - ?linters_def_file ?doc_url ?access ?extras exn + ?linters_def_file ?access ?extras exn let log_error = log_issue_from_summary_simplified Exceptions.Error @@ -89,7 +88,7 @@ let log_issue_external procname severity ~loc ~ltr ?access issue_type error_mess 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 - ~linters_def_file:None ~doc_url:None ~access ~extras:None exn + ~linters_def_file:None ~access ~extras:None exn let log_error_using_state summary exn = @@ -98,7 +97,7 @@ let log_error_using_state summary exn = let loc = State.get_loc () in let ltr = State.get_loc_trace () in log_issue_from_summary Exceptions.Error summary ~node ~session ~loc ~ltr ?linters_def_file:None - ?doc_url:None ?access:None exn + ?access:None exn let is_suppressed ?(field_name = None) tenv proc_desc kind = diff --git a/infer/src/backend/reporting.mli b/infer/src/backend/reporting.mli index e1bd82b89..0dfffb3f9 100644 --- a/infer/src/backend/reporting.mli +++ b/infer/src/backend/reporting.mli @@ -12,7 +12,6 @@ open! IStd type log_t = ?ltr:Errlog.loc_trace -> ?linters_def_file:string - -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra -> exn @@ -38,7 +37,6 @@ val log_frontend_issue : -> node_key:Procdesc.NodeKey.t -> ltr:Errlog.loc_trace -> linters_def_file:string option - -> doc_url:string option -> exn -> unit (** Report a frontend issue of a given kind in the given error log. *) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 66a8ad9f3..5e28ee9ed 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -9,19 +9,24 @@ open! IStd (* Make sure we cannot create new issue types other than by calling [from_string]. This is because we want to keep track of the list of all the issues ever declared. *) module Unsafe : sig - type t = private {unique_id: string; mutable enabled: bool; mutable hum: string} + type t = private + {unique_id: string; mutable enabled: bool; mutable hum: string; mutable doc_url: string option} [@@deriving compare] val equal : t -> t -> bool - val from_string : ?enabled:bool -> ?hum:string -> string -> t + val from_string : ?enabled:bool -> ?hum:string -> ?doc_url:string -> string -> t val all_issues : unit -> t list val set_enabled : t -> bool -> unit end = struct module T = struct - type t = {unique_id: string; mutable enabled: bool; mutable hum: string} + type t = + { unique_id: string + ; mutable enabled: bool + ; mutable hum: string + ; mutable doc_url: string option } let compare {unique_id= id1} {unique_id= id2} = String.compare id1 id2 @@ -51,14 +56,15 @@ end = struct 2., but issues of type 2. have not yet been defined. Thus, we record only there [enabled] status definitely. The [hum]an-readable description can be updated when we encounter the definition of the issue type, eg in AL. *) - let from_string ?(enabled = true) ?hum:hum0 unique_id = + let from_string ?(enabled = true) ?hum:hum0 ?doc_url unique_id = let hum = match hum0 with Some str -> str | _ -> prettify unique_id in - let issue = {unique_id; enabled; hum} in + let issue = {unique_id; enabled; hum; doc_url} in try let old = IssueSet.find issue !all_issues in (* update human-readable string in case it was supplied this time, but keep the previous value of enabled (see doc comment) *) if Option.is_some hum0 then old.hum <- hum ; + if Option.is_some doc_url then old.doc_url <- doc_url ; old with Caml.Not_found -> all_issues := IssueSet.add issue !all_issues ; diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 028b7ca93..764af9985 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -8,7 +8,8 @@ open! IStd (** type of string used for localisation *) -type t = private {unique_id: string; mutable enabled: bool; mutable hum: string} +type t = private + {unique_id: string; mutable enabled: bool; mutable hum: string; mutable doc_url: string option} [@@deriving compare] val equal : t -> t -> bool @@ -19,7 +20,7 @@ val all_issues : unit -> t list val pp : Format.formatter -> t -> unit (** pretty print a localised string *) -val from_string : ?enabled:bool -> ?hum:string -> string -> t +val from_string : ?enabled:bool -> ?hum:string -> ?doc_url:string -> string -> t (** create from an ordinary string *) val set_enabled : t -> bool -> unit diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index dbec42050..d8730e1dc 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -444,7 +444,10 @@ let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) let err_desc = Errdesc.explain_frontend_warning issue_desc.description issue_desc.suggestion issue_desc.loc in - let exn = Exceptions.Frontend_warning ((issue_desc.id, issue_desc.name), err_desc, __POS__) in + let exn = + Exceptions.Frontend_warning + ((issue_desc.id, issue_desc.name, issue_desc.doc_url), err_desc, __POS__) + in let trace = [Errlog.make_trace_element 0 issue_desc.loc "" []] in let key_str = match node with @@ -455,7 +458,7 @@ let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) in let node_key = Procdesc.NodeKey.of_frontend_node_key key_str in Reporting.log_frontend_issue procname issue_desc.severity errlog exn ~loc:issue_desc.loc - ~ltr:trace ~node_key ~linters_def_file ~doc_url:issue_desc.doc_url + ~ltr:trace ~node_key ~linters_def_file let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc : CIssue.issue_desc)