From 5572484eeacab02d88990e92a965ad3ea4b9c91e Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Tue, 26 May 2020 03:46:44 -0700 Subject: [PATCH] [AL] monomorphise CIssue.t Summary: That was an interesting way to do things. But let's not. Also the logic to fill in the CIssue.t should probably do something to check that each field was provided instead of just filling them with default values but that's a separate concern. Reviewed By: ngorogiannis Differential Revision: D21664619 fbshipit-source-id: d49b74458 --- infer/src/al/ALIssues.ml | 55 +++++++++++++++++++---------------- infer/src/al/ALIssues.mli | 2 +- infer/src/al/ComponentKit.mli | 14 ++++----- infer/src/al/cIssue.ml | 28 +++++++++--------- infer/src/al/cIssue.mli | 12 ++++---- 5 files changed, 56 insertions(+), 55 deletions(-) diff --git a/infer/src/al/ALIssues.ml b/infer/src/al/ALIssues.ml index b5fd3c934..22aa9f474 100644 --- a/infer/src/al/ALIssues.ml +++ b/infer/src/al/ALIssues.ml @@ -12,7 +12,7 @@ module MF = MarkupFormatter type linter = { condition: CTLTypes.t - ; issue_desc: CIssue.issue_desc + ; issue_desc: CIssue.t ; whitelist_paths: ALVar.t list ; blacklist_paths: ALVar.t list } @@ -200,25 +200,31 @@ let string_to_issue_mode m = L.die InternalError "Mode %s does not exist. Please specify ON/OFF" s -type parsed_issue_type = - { name: string option +(** building a {!CIssue.t} piece by piece *) +type issue_in_construction = + { issue_type_doc_url: string option + ; issue_type_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 *) - ; doc_url: string option } + ; description: string + ; mode: CIssue.mode + ; loc: Location.t + ; severity: Exceptions.severity + ; suggestion: string option } (** Convert a parsed checker in list of linters *) let create_parsed_linters linters_def_file checkers : linter list = - let open CIssue in let open CTL in L.(debug Linters Medium) "@\nConverting checkers in (condition, issue) pairs@\n" ; let do_one_checker checker : linter = - let dummy_issue = - { issue_type= {name= None; doc_url= None} + let dummy_issue : issue_in_construction = + { issue_type_doc_url= None + ; issue_type_name= None ; description= "" ; suggestion= None ; loc= Location.dummy - ; severity= Exceptions.Warning - ; mode= CIssue.On } + ; severity= Warning + ; mode= On } in let issue_desc, condition, whitelist_paths, blacklist_paths = let process_linter_definitions (issue, cond, wl_paths, bl_paths) description = @@ -234,15 +240,9 @@ let create_parsed_linters linters_def_file checkers : linter list = | CDesc (av, m) when ALVar.is_mode_keyword av -> ({issue with mode= string_to_issue_mode m}, cond, wl_paths, bl_paths) | CDesc (av, doc) when ALVar.is_doc_url_keyword av -> - ( {issue with issue_type= {issue.issue_type with doc_url= Some doc}} - , cond - , wl_paths - , bl_paths ) + ({issue with issue_type_doc_url= Some doc}, cond, wl_paths, bl_paths) | CDesc (av, name) when ALVar.is_name_keyword av -> - ( {issue with issue_type= {issue.issue_type with name= Some name}} - , cond - , wl_paths - , bl_paths ) + ({issue with issue_type_name= Some name}, cond, wl_paths, bl_paths) | CPath (`WhitelistPath, paths) -> (issue, cond, paths, bl_paths) | CPath (`BlacklistPath, paths) -> @@ -260,13 +260,20 @@ let create_parsed_linters linters_def_file checkers : linter list = let doc_url = Option.first_some (Config.get_linter_doc_url ~linter_id:checker.id) - issue_desc.issue_type.doc_url + issue_desc.issue_type_doc_url in - IssueType.register_from_string ~id:checker.id ?hum:issue_desc.issue_type.name ?doc_url + IssueType.register_from_string ~id:checker.id ?hum:issue_desc.issue_type_name ?doc_url ~linters_def_file Linters in - let issue_desc = {issue_desc with issue_type} in - L.(debug Linters Medium) "@\nIssue_desc = %a@\n" CIssue.pp_issue issue_desc ; + let issue_desc = + { CIssue.issue_type + ; description= issue_desc.description + ; mode= issue_desc.mode + ; loc= issue_desc.loc + ; severity= issue_desc.severity + ; suggestion= issue_desc.suggestion } + in + L.debug Linters Medium "@\nIssue_desc = %a@\n" CIssue.pp issue_desc ; {condition; issue_desc; whitelist_paths; blacklist_paths} in List.map ~f:do_one_checker checkers @@ -473,8 +480,7 @@ let expand_checkers macro_map path_map checkers = let issue_log = ref IssueLog.empty (** Add a frontend warning with a description desc at location loc to the errlog of a proc desc *) -let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) - (issue_desc : CIssue.issue_desc) = +let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) (issue_desc : CIssue.t) = let procname = match method_decl_opt with | Some method_decl -> @@ -501,8 +507,7 @@ let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) ~node_key -let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc : CIssue.issue_desc) loc - = +let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc : CIssue.t) loc = let process_message message = remove_new_lines_and_whitespace (expand_message_string context message current_node) in diff --git a/infer/src/al/ALIssues.mli b/infer/src/al/ALIssues.mli index 08a152071..e37c010a0 100644 --- a/infer/src/al/ALIssues.mli +++ b/infer/src/al/ALIssues.mli @@ -11,7 +11,7 @@ val issue_log : IssueLog.t ref type linter = { condition: CTLTypes.t - ; issue_desc: CIssue.issue_desc + ; issue_desc: CIssue.t ; whitelist_paths: ALVar.t list ; blacklist_paths: ALVar.t list } diff --git a/infer/src/al/ComponentKit.mli b/infer/src/al/ComponentKit.mli index e014500ff..87b290eef 100644 --- a/infer/src/al/ComponentKit.mli +++ b/infer/src/al/ComponentKit.mli @@ -14,22 +14,22 @@ val contains_ck_impl : Clang_ast_t.decl list -> bool Does not recurse into hierarchy. *) val mutable_local_vars_advice : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc option + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t option val component_factory_function_advice : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc option + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t option val component_with_unconventional_superclass_advice : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc option + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t option val component_with_multiple_factory_methods_advice : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc list + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t list val component_initializer_with_side_effects_advice : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc option + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t option val component_file_line_count_info : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc list + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t list val component_file_cyclomatic_complexity_info : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc option + CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.t option diff --git a/infer/src/al/cIssue.ml b/infer/src/al/cIssue.ml index 754d5a6e9..3c4e74976 100644 --- a/infer/src/al/cIssue.ml +++ b/infer/src/al/cIssue.ml @@ -9,19 +9,25 @@ open! IStd type mode = On | Off -type 'issue_type issue_desc0 = - { issue_type: 'issue_type (** issue type *) +let string_of_mode m = match m with On -> "On" | Off -> "Off" + +let should_run_check mode = + match mode with + | On -> + true + | Off -> + Config.debug_mode || Config.debug_exceptions || not Config.filtering + + +type t = + { issue_type: IssueType.t ; description: string (** Description in the error message *) ; mode: mode ; loc: Location.t (** location in the code *) ; severity: Exceptions.severity ; suggestion: string option (** an optional suggestion or correction *) } -type issue_desc = IssueType.t issue_desc0 - -let string_of_mode m = match m with On -> "On" | Off -> "Off" - -let pp_issue fmt issue = +let pp fmt issue = Format.fprintf fmt "{@\n Id = %s@\n" issue.issue_type.IssueType.unique_id ; Format.fprintf fmt "{ Name = %s@\n" issue.issue_type.IssueType.hum ; Format.fprintf fmt " Severity = %s@\n" (Exceptions.severity_string issue.severity) ; @@ -32,11 +38,3 @@ let pp_issue fmt issue = (Option.value ~default:"" issue.issue_type.IssueType.doc_url) ; Format.fprintf fmt " Loc = %s@\n" (Location.to_string issue.loc) ; Format.fprintf fmt "}@\n" - - -let should_run_check mode = - match mode with - | On -> - true - | Off -> - Config.debug_mode || Config.debug_exceptions || not Config.filtering diff --git a/infer/src/al/cIssue.mli b/infer/src/al/cIssue.mli index 0cbebbcc2..2b387cceb 100644 --- a/infer/src/al/cIssue.mli +++ b/infer/src/al/cIssue.mli @@ -9,16 +9,14 @@ open! IStd type mode = On | Off -type 'issue_type issue_desc0 = - { issue_type: 'issue_type (** issue type *) +val should_run_check : mode -> bool + +type t = + { issue_type: IssueType.t ; description: string (** Description in the error message *) ; mode: mode ; loc: Location.t (** location in the code *) ; severity: Exceptions.severity ; suggestion: string option (** an optional suggestion or correction *) } -type issue_desc = IssueType.t issue_desc0 - -val pp_issue : Format.formatter -> issue_desc -> unit - -val should_run_check : mode -> bool +val pp : Format.formatter -> t -> unit