[linters] Be able to specify human readable bug type in the rules

Reviewed By: jvillard

Differential Revision: D5375731

fbshipit-source-id: 82d5e48
master
Dulma Churchill 8 years ago committed by Facebook Github Bot
parent 7c41d3ee48
commit 3aa68e23d9

@ -218,7 +218,7 @@ DEFINE-CHECKER CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK = {
SET message = SET message =
"%decl_ref_or_selector_name% is not available in the required iOS SDK version "%decl_ref_or_selector_name% is not available in the required iOS SDK version
%iphoneos_target_sdk_version% (only available from version %available_ios_sdk%)"; %iphoneos_target_sdk_version% (only available from version %available_ios_sdk%)";
SET name = "Unavailable API In Supported iOS SDK";
SET suggestion = "This could cause a crash."; SET suggestion = "This could cause a crash.";
SET severity = "ERROR"; SET severity = "ERROR";
}; };

@ -69,7 +69,7 @@ exception Double_lock of Localise.error_desc * L.ml_loc
exception Empty_vector_access 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 string * Localise.error_desc
exception Field_not_null_checked of Localise.error_desc * L.ml_loc exception Field_not_null_checked of Localise.error_desc * L.ml_loc
exception Frontend_warning of string * 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 string * Localise.error_desc
exception Inherently_dangerous_function of Localise.error_desc exception Inherently_dangerous_function of Localise.error_desc
exception Internal_error of Localise.error_desc exception Internal_error of Localise.error_desc
@ -188,8 +188,8 @@ let recognize_exception exn =
| Field_not_null_checked (desc, ml_loc) -> | Field_not_null_checked (desc, ml_loc) ->
(Localise.field_not_null_checked, (Localise.field_not_null_checked,
desc, Some ml_loc, Exn_user, Medium, Some Kwarning, Nocat) desc, Some ml_loc, Exn_user, Medium, Some Kwarning, Nocat)
| Frontend_warning (name, desc, ml_loc) -> | Frontend_warning ((name, hum), desc, ml_loc) ->
(Localise.from_string name, (Localise.from_string name ?hum,
desc, Some ml_loc, Exn_user, Medium, None, Linters) desc, Some ml_loc, Exn_user, Medium, None, Linters)
| Checkers (kind_s, desc) -> | Checkers (kind_s, desc) ->
(Localise.from_string kind_s, (Localise.from_string kind_s,

@ -65,7 +65,7 @@ exception Field_not_null_checked of Localise.error_desc * Logging.ml_loc
exception Empty_vector_access 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 string * Localise.error_desc
exception Checkers of string * Localise.error_desc exception Checkers of string * Localise.error_desc
exception Frontend_warning of string * Localise.error_desc * Logging.ml_loc exception Frontend_warning of (string * string option) * Localise.error_desc * Logging.ml_loc
exception Inherently_dangerous_function of Localise.error_desc exception Inherently_dangerous_function of Localise.error_desc
exception Internal_error of Localise.error_desc exception Internal_error of Localise.error_desc
exception Java_runtime_exception of Typ.Name.t * string * Localise.error_desc exception Java_runtime_exception of Typ.Name.t * string * Localise.error_desc

@ -38,7 +38,7 @@ let log_issue_from_errlog err_kind err_log ?loc ?node_id ?session ?ltr ?linters_
| None -> State.get_loc_trace () | None -> State.get_loc_trace ()
| Some ltr -> ltr in | Some ltr -> ltr in
let err_name = match exn with let err_name = match exn with
| Exceptions.Frontend_warning (err_name, _, _) -> err_name | Exceptions.Frontend_warning ((err_name, _), _, _) -> err_name
| _ -> let err_name, _, _, _, _, _, _ = Exceptions.recognize_exception exn in | _ -> let err_name, _, _, _, _, _, _ = Exceptions.recognize_exception exn in
(Localise.to_issue_id err_name) in (Localise.to_issue_id err_name) in
if (Inferconfig.is_checker_enabled err_name) then if (Inferconfig.is_checker_enabled err_name) then

@ -11,12 +11,13 @@ open! IStd
module L = Logging module L = Logging
type keyword = type keyword =
| Report_when | Doc_url
| Message | Message
| Suggestion
| Severity
| Mode | Mode
| Doc_url | Name
| Report_when
| Severity
| Suggestion
type formula_id = Formula_id of string[@@deriving compare] type formula_id = Formula_id of string[@@deriving compare]
@ -44,12 +45,13 @@ let alexp_to_string e =
let keyword_to_string k = let keyword_to_string k =
match k with match k with
| Report_when -> "report_when" | Doc_url -> "doc_url"
| Message -> "message" | Message -> "message"
| Suggestion -> "suggestion"
| Severity -> "severity"
| Mode -> "mode" | Mode -> "mode"
| Doc_url -> "doc_url" | Name -> "name_hum_readable"
| Report_when -> "report_when"
| Severity -> "severity"
| Suggestion -> "suggestion"
let is_report_when_keyword k = let is_report_when_keyword k =
match k with match k with
@ -81,6 +83,11 @@ let is_doc_url_keyword k =
| Doc_url -> true | Doc_url -> true
| _ -> false | _ -> false
let is_name_keyword k =
match k with
| Name -> true
| _ -> false
(* true if and only if a substring of container matches the regular (* true if and only if a substring of container matches the regular
expression defined by contained expression defined by contained
*) *)

@ -9,12 +9,13 @@
open! IStd open! IStd
type keyword = type keyword =
| Report_when | Doc_url
| Message | Message
| Suggestion
| Severity
| Mode | Mode
| Doc_url | Name
| Report_when
| Severity
| Suggestion
type formula_id = Formula_id of string type formula_id = Formula_id of string
@ -46,6 +47,8 @@ val is_mode_keyword : keyword -> bool
val is_doc_url_keyword : keyword -> bool val is_doc_url_keyword : keyword -> bool
val is_name_keyword : keyword -> bool
val str_match_regex : string -> string -> bool val str_match_regex : string -> string -> bool
val compare_str_with_alexp : string -> alexp -> bool val compare_str_with_alexp : string -> alexp -> bool

@ -125,7 +125,8 @@ let mutable_local_vars_advice context an =
&& not decl_info.di_is_implicit in && not decl_info.di_is_implicit in
if condition then if condition then
Some { Some {
CIssue.name = "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE"; CIssue.id = "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE";
name = None;
severity = Exceptions.Kadvice; severity = Exceptions.Kadvice;
mode = CIssue.On; mode = CIssue.On;
description = description =
@ -155,7 +156,8 @@ let component_factory_function_advice context an =
is_ck_context context an && is_component_if objc_interface in is_ck_context context an && is_component_if objc_interface in
if condition then if condition then
Some { Some {
CIssue.name = "COMPONENT_FACTORY_FUNCTION"; CIssue.id = "COMPONENT_FACTORY_FUNCTION";
name = None;
severity = Exceptions.Kadvice; severity = Exceptions.Kadvice;
mode = CIssue.Off; mode = CIssue.Off;
description = "Break out composite components"; description = "Break out composite components";
@ -200,7 +202,8 @@ let component_with_unconventional_superclass_advice context an =
&& not has_conventional_superclass in && not has_conventional_superclass in
if condition then if condition then
Some { Some {
CIssue.name = "COMPONENT_WITH_UNCONVENTIONAL_SUPERCLASS"; CIssue.id = "COMPONENT_WITH_UNCONVENTIONAL_SUPERCLASS";
name = None;
severity = Exceptions.Kadvice; severity = Exceptions.Kadvice;
mode = CIssue.On; mode = CIssue.On;
description = "Never Subclass Components"; description = "Never Subclass Components";
@ -255,7 +258,8 @@ let component_with_multiple_factory_methods_advice context an =
| Clang_ast_t.ObjCInterfaceDecl (_, _, decls, _, _) -> | Clang_ast_t.ObjCInterfaceDecl (_, _, decls, _, _) ->
let factory_methods = List.filter ~f:(is_available_factory_method if_decl) decls in let factory_methods = List.filter ~f:(is_available_factory_method if_decl) decls in
List.map ~f:(fun meth_decl -> { List.map ~f:(fun meth_decl -> {
CIssue.name = "COMPONENT_WITH_MULTIPLE_FACTORY_METHODS"; CIssue.id = "COMPONENT_WITH_MULTIPLE_FACTORY_METHODS";
name = None;
severity = Exceptions.Kadvice; severity = Exceptions.Kadvice;
mode = CIssue.On; mode = CIssue.On;
description = "Avoid Overrides"; description = "Avoid Overrides";
@ -309,7 +313,8 @@ let rec _component_initializer_with_side_effects_advice
| Some "dispatch_async" | Some "dispatch_async"
| Some "dispatch_sync" -> | Some "dispatch_sync" ->
Some { Some {
CIssue.name = "COMPONENT_INITIALIZER_WITH_SIDE_EFFECTS"; CIssue.id = "COMPONENT_INITIALIZER_WITH_SIDE_EFFECTS";
name = None;
severity = Exceptions.Kadvice; severity = Exceptions.Kadvice;
mode = CIssue.On; mode = CIssue.On;
description = "No Side-effects"; description = "No Side-effects";
@ -344,7 +349,8 @@ let component_file_line_count_info (context: CLintersContext.context) dec =
context.translation_unit_context.CFrontend_config.source_file in context.translation_unit_context.CFrontend_config.source_file in
let line_count = SourceFile.line_count source_file in let line_count = SourceFile.line_count source_file in
List.map ~f:(fun i -> { List.map ~f:(fun i -> {
CIssue.name = "COMPONENT_FILE_LINE_COUNT"; CIssue.id = "COMPONENT_FILE_LINE_COUNT";
name = None;
severity = Exceptions.Kinfo; severity = Exceptions.Kinfo;
mode = CIssue.Off; mode = CIssue.Off;
description = "Line count analytics"; description = "Line count analytics";
@ -391,7 +397,8 @@ let component_file_cyclomatic_complexity_info (context: CLintersContext.context)
match cyclo_loc_opt an with match cyclo_loc_opt an with
| Some loc -> | Some loc ->
Some { Some {
CIssue.name = "COMPONENT_FILE_CYCLOMATIC_COMPLEXITY"; CIssue.id = "COMPONENT_FILE_CYCLOMATIC_COMPLEXITY";
name = None;
severity = Exceptions.Kinfo; severity = Exceptions.Kinfo;
mode = CIssue.Off; mode = CIssue.Off;
description = "Cyclomatic Complexity Incremental Marker"; description = "Cyclomatic Complexity Incremental Marker";

@ -32,7 +32,7 @@ let filter_parsed_linters_developer parsed_linters =
--linter <name> to specify the linter you want to debug."; --linter <name> to specify the linter you want to debug.";
| Some lint -> | Some lint ->
List.filter ~f:( List.filter ~f:(
fun (rule : linter) -> String.equal rule.issue_desc.name lint fun (rule : linter) -> String.equal rule.issue_desc.id lint
) parsed_linters ) parsed_linters
else parsed_linters else parsed_linters
@ -57,8 +57,8 @@ let filter_parsed_linters parsed_linters source_file =
else filter_parsed_linters_by_path linters source_file else filter_parsed_linters_by_path linters source_file
let pp_linters fmt linters = let pp_linters fmt linters =
let pp_linter fmt {issue_desc={name}} = let pp_linter fmt {issue_desc={id}} =
F.fprintf fmt "%s@\n" name in F.fprintf fmt "%s@\n" id in
List.iter ~f:(pp_linter fmt) linters List.iter ~f:(pp_linter fmt) linters
(* Map a formula id to a triple (visited, parameters, definition). (* Map a formula id to a triple (visited, parameters, definition).
@ -166,7 +166,8 @@ let create_parsed_linters linters_def_file checkers : linter list =
L.(debug Linters Medium) "@\nConverting checkers in (condition, issue) pairs@\n"; L.(debug Linters Medium) "@\nConverting checkers in (condition, issue) pairs@\n";
let do_one_checker checker : linter = let do_one_checker checker : linter =
let dummy_issue = { let dummy_issue = {
name = checker.name; id = checker.id;
name = None;
description = ""; description = "";
suggestion = None; suggestion = None;
loc = Location.dummy; loc = Location.dummy;
@ -189,6 +190,8 @@ let create_parsed_linters linters_def_file checkers : linter list =
{issue with mode = string_to_issue_mode m }, cond, wl_paths, bl_paths {issue with mode = string_to_issue_mode m }, cond, wl_paths, bl_paths
| CDesc (av, doc) when ALVar.is_doc_url_keyword av -> | CDesc (av, doc) when ALVar.is_doc_url_keyword av ->
{issue with doc_url = Some doc }, cond, wl_paths, bl_paths {issue with doc_url = Some doc }, cond, wl_paths, bl_paths
| CDesc (av, name) when ALVar.is_name_keyword av ->
{issue with name = Some name }, cond, wl_paths, bl_paths
| CPath (`WhitelistPath, paths) -> | CPath (`WhitelistPath, paths) ->
issue, cond, paths, bl_paths issue, cond, paths, bl_paths
| CPath (`BlacklistPath, paths) -> | CPath (`BlacklistPath, paths) ->
@ -198,7 +201,7 @@ let create_parsed_linters linters_def_file checkers : linter list =
~f:process_linter_definitions ~f:process_linter_definitions
~init:(dummy_issue, CTL.False, [], []) ~init:(dummy_issue, CTL.False, [], [])
checker.definitions in checker.definitions in
L.(debug Linters Medium) "@\nMaking condition and issue desc for checker '%s'@\n" checker.name; L.(debug Linters Medium) "@\nMaking condition and issue desc for checker '%s'@\n" checker.id;
L.(debug Linters Medium) "@\nCondition =@\n %a@\n" CTL.Debug.pp_formula condition; L.(debug Linters Medium) "@\nCondition =@\n %a@\n" CTL.Debug.pp_formula condition;
L.(debug Linters Medium) "@\nIssue_desc = %a@\n" CIssue.pp_issue issue_desc; L.(debug Linters Medium) "@\nIssue_desc = %a@\n" CIssue.pp_issue issue_desc;
{condition; issue_desc; def_file = Some linters_def_file; whitelist_paths; blacklist_paths;} in {condition; issue_desc; def_file = Some linters_def_file; whitelist_paths; blacklist_paths;} in
@ -331,7 +334,7 @@ let build_paths_map paths =
let expand_checkers macro_map path_map checkers = let expand_checkers macro_map path_map checkers =
let open CTL in let open CTL in
let expand_one_checker c = let expand_one_checker c =
L.(debug Linters Medium) " +Start expanding %s@\n" c.name; L.(debug Linters Medium) " +Start expanding %s@\n" c.id;
let map = _build_macros_map c.definitions macro_map in let map = _build_macros_map c.definitions macro_map in
let exp_defs = List.fold ~f:(fun defs clause -> let exp_defs = List.fold ~f:(fun defs clause ->
match clause with match clause with
@ -352,20 +355,20 @@ let get_err_log translation_unit_context method_decl_opt =
LintIssues.get_err_log procname LintIssues.get_err_log procname
(* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *) (* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *)
let log_frontend_issue translation_unit_context method_decl_opt key issue_desc linters_def_file = let log_frontend_issue translation_unit_context method_decl_opt key (issue_desc : CIssue.issue_desc)
let name = issue_desc.CIssue.name in linters_def_file =
let loc = issue_desc.CIssue.loc in
let errlog = get_err_log translation_unit_context method_decl_opt in let errlog = get_err_log translation_unit_context method_decl_opt in
let err_desc = Errdesc.explain_frontend_warning issue_desc.CIssue.description let err_desc = Errdesc.explain_frontend_warning issue_desc.description
issue_desc.CIssue.suggestion loc in issue_desc.suggestion issue_desc.loc in
let exn = Exceptions.Frontend_warning (name, err_desc, __POS__) in let exn =
let trace = [ Errlog.make_trace_element 0 issue_desc.CIssue.loc "" [] ] in Exceptions.Frontend_warning ((issue_desc.id, issue_desc.name), err_desc, __POS__) in
let err_kind = issue_desc.CIssue.severity in let trace = [ Errlog.make_trace_element 0 issue_desc.loc "" [] ] in
let err_kind = issue_desc.severity in
let method_name = CAst_utils.full_name_of_decl_opt method_decl_opt let method_name = CAst_utils.full_name_of_decl_opt method_decl_opt
|> QualifiedCppName.to_qual_string in |> QualifiedCppName.to_qual_string in
let key = Hashtbl.hash (key ^ method_name) in let key = Hashtbl.hash (key ^ method_name) in
Reporting.log_issue_from_errlog err_kind errlog exn ~loc ~ltr:trace Reporting.log_issue_from_errlog err_kind errlog exn ~loc:issue_desc.loc ~ltr:trace
~node_id:(0, key) ?linters_def_file ?doc_url:issue_desc.CIssue.doc_url ~node_id:(0, key) ?linters_def_file ?doc_url:issue_desc.doc_url
let get_current_method context (an : Ctl_parser_types.ast_node) = let get_current_method context (an : Ctl_parser_types.ast_node) =
match an with match an with

@ -12,13 +12,15 @@ open! IStd
type mode = On | Off type mode = On | Off
type issue_desc = { type issue_desc = {
name : string; (* issue name *) id : string; (* issue id *)
severity : Exceptions.err_kind;
mode : mode;
description : string; (* Description in the error message *) description : string; (* Description in the error message *)
suggestion : string option; (* an optional suggestion or correction *)
doc_url : string option; doc_url : string option;
mode : mode;
name : string option; (* issue name, if no name is given name will be a readable version of id,
by removing underscores and capitalizing first letters of words *)
loc : Location.t; (* location in the code *) loc : Location.t; (* location in the code *)
severity : Exceptions.err_kind;
suggestion : string option; (* an optional suggestion or correction *)
} }
let string_of_mode m = let string_of_mode m =
@ -27,15 +29,15 @@ let string_of_mode m =
| Off -> "Off" | Off -> "Off"
let pp_issue fmt issue = let pp_issue fmt issue =
Format.fprintf fmt "{@\n Name = %s@\n" (issue.name); Format.fprintf fmt "{@\n Id = %s@\n" (issue.id);
Format.fprintf fmt " Severity = %s@\n" (Exceptions.err_kind_string issue.severity); Format.fprintf fmt "{ Name = %s@\n" (Option.value ~default:"" issue.name);
Format.fprintf fmt " Mode = %s@\n" (string_of_mode issue.mode); Format.fprintf fmt " Severity = %s@\n" (Exceptions.err_kind_string issue.severity);
Format.fprintf fmt " Descrption = %s@\n" issue.description; Format.fprintf fmt " Mode = %s@\n" (string_of_mode issue.mode);
(match issue.suggestion with Format.fprintf fmt " Description = %s@\n" issue.description;
| Some s -> Format.fprintf fmt " Suggestion = %s@\n" s Format.fprintf fmt " Suggestion = %s@\n" (Option.value ~default:"" issue.suggestion);
| _ -> ()); Format.fprintf fmt " Docs URL = %s@\n" (Option.value ~default:"" issue.doc_url);
Format.fprintf fmt " Loc = %s@\n" (Location.to_string issue.loc); Format.fprintf fmt " Loc = %s@\n" (Location.to_string issue.loc);
Format.fprintf fmt "}@\n" Format.fprintf fmt "}@\n"
let should_run_check mode = let should_run_check mode =
match mode with match mode with

@ -12,13 +12,15 @@ open! IStd
type mode = On | Off type mode = On | Off
type issue_desc = { type issue_desc = {
name : string; (* issue name *) id : string; (* issue id *)
severity : Exceptions.err_kind;
mode : mode;
description : string; (* Description in the error message *) description : string; (* Description in the error message *)
suggestion : string option; (* an optional suggestion or correction *)
doc_url : string option; doc_url : string option;
mode : mode;
name : string option; (* issue name, if no name is given name will be a readable version of id,
by removing underscores and capitalizing first letters of words *)
loc : Location.t; (* location in the code *) loc : Location.t; (* location in the code *)
severity : Exceptions.err_kind;
suggestion : string option; (* an optional suggestion or correction *)
} }
val string_of_mode : mode -> string val string_of_mode : mode -> string

@ -86,7 +86,7 @@ type clause =
| CPath of [ `WhitelistPath | `BlacklistPath ] * ALVar.t list | CPath of [ `WhitelistPath | `BlacklistPath ] * ALVar.t list
type ctl_checker = { type ctl_checker = {
name : string; (* Checker's name *) id : string; (* Checker's id *)
definitions : clause list (* A list of let/set definitions *) definitions : clause list (* A list of let/set definitions *)
} }
@ -434,7 +434,7 @@ end
let print_checker c = let print_checker c =
L.(debug Linters Medium) "@\n-------------------- @\n"; L.(debug Linters Medium) "@\n-------------------- @\n";
L.(debug Linters Medium) "@\nChecker name: %s@\n" c.name; L.(debug Linters Medium) "@\nChecker name: %s@\n" c.id;
List.iter ~f:(fun d -> (match d with List.iter ~f:(fun d -> (match d with
| CSet (keyword, phi) -> | CSet (keyword, phi) ->
let cn_str = ALVar.keyword_to_string keyword in let cn_str = ALVar.keyword_to_string keyword in

@ -84,7 +84,7 @@ type clause =
| CPath of [ `WhitelistPath | `BlacklistPath ] * ALVar.t list | CPath of [ `WhitelistPath | `BlacklistPath ] * ALVar.t list
type ctl_checker = { type ctl_checker = {
name : string; (* Checker's name *) id : string; (* Checker's id *)
definitions : clause list (* A list of let/set definitions *) definitions : clause list (* A list of let/set definitions *)
} }

@ -138,7 +138,7 @@ checker:
DEFINE_CHECKER identifier ASSIGNMENT LEFT_BRACE clause_list RIGHT_BRACE DEFINE_CHECKER identifier ASSIGNMENT LEFT_BRACE clause_list RIGHT_BRACE
{ {
L.(debug Linters Verbose) "@\nParsed checker definition@\n"; L.(debug Linters Verbose) "@\nParsed checker definition@\n";
let c = { CTL.name = $2; CTL.definitions = $5 } in let c = { CTL.id = $2; CTL.definitions = $5 } in
CTL.print_checker c; CTL.print_checker c;
c c
} }
@ -179,9 +179,10 @@ clause:
| "severity" -> ALVar.Severity | "severity" -> ALVar.Severity
| "mode" -> ALVar.Mode | "mode" -> ALVar.Mode
| "doc_url" -> ALVar.Doc_url | "doc_url" -> ALVar.Doc_url
| "name" -> ALVar.Name
| _ -> failwithf "string '%s' cannot be set in a SET clause. \ | _ -> failwithf "string '%s' cannot be set in a SET clause. \
Use either of: \ Use either of: \
'doc_url', 'message', 'mode', 'severity' or 'suggestion'" $2 in 'doc_url', 'message', 'mode', 'name', 'severity' or 'suggestion'" $2 in
CTL.CDesc (alvar, $4) } CTL.CDesc (alvar, $4) }
| let_clause { $1 } | let_clause { $1 }
; ;

Loading…
Cancel
Save