From 731072615571545480031ec78d2f75ce6b85b9bf Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 11 Jun 2020 06:43:00 -0700 Subject: [PATCH] Checker.name -> Checker.id Summary: The checker names are only used in debug information but I need them to be more useful so users can do queries about each checker. Turn them into an "id" instead of a "name", with some constraints to avoid crazy IDs. In the next diff these IDs will be used on the command lin. Reviewed By: dulmarod Differential Revision: D21934373 fbshipit-source-id: 847a4958d --- infer/src/absint/Errlog.ml | 6 +-- infer/src/backend/registerCheckers.ml | 4 +- infer/src/base/Checker.ml | 73 ++++++++++++++++----------- infer/src/base/Checker.mli | 8 +-- infer/src/base/IssueType.ml | 4 +- 5 files changed, 54 insertions(+), 41 deletions(-) diff --git a/infer/src/absint/Errlog.ml b/infer/src/absint/Errlog.ml index 4d6829145..15708fecd 100644 --- a/infer/src/absint/Errlog.ml +++ b/infer/src/absint/Errlog.ml @@ -203,9 +203,9 @@ let log_issue ?severity_override err_log ~loc ~node ~session ~ltr ~linters_def_f "Issue type \"%s\" cannot be reported by the checker \"%s\". The only checker that is \ allowed to report this issue type is \"%s\". If this is incorrect please either update the \ issue in IssueType or create a new issue type for \"%s\"." - error.issue_type.unique_id (Checker.get_name checker) - (Checker.get_name error.issue_type.checker) - (Checker.get_name checker) ; + error.issue_type.unique_id (Checker.get_id checker) + (Checker.get_id error.issue_type.checker) + (Checker.get_id checker) ; let severity = Option.value severity_override ~default:error.issue_type.default_severity in let hide_java_loc_zero = (* hide java errors at location zero unless in -developer_mode *) diff --git a/infer/src/backend/registerCheckers.ml b/infer/src/backend/registerCheckers.ml index b6607910f..302d9169d 100644 --- a/infer/src/backend/registerCheckers.ml +++ b/infer/src/backend/registerCheckers.ml @@ -198,7 +198,7 @@ let get_active_checkers () = let register checkers = let register_one {checker; callbacks} = - let name = (Checker.config checker).name in + let name = Checker.get_id checker in let register_callback (callback, language) = match callback with | Procedure procedure_cb -> @@ -222,6 +222,6 @@ let pp_checker fmt {checker; callbacks} = LanguageSet.add lang langs ) |> LanguageSet.elements in - F.fprintf fmt "%s (%a)" (Checker.config checker).name + F.fprintf fmt "%s (%a)" (Checker.get_id checker) (Pp.seq ~sep:", " (Pp.of_string ~f:Language.to_string)) langs_of_callbacks diff --git a/infer/src/base/Checker.ml b/infer/src/base/Checker.ml index 2ce55bd27..97bb9b483 100644 --- a/infer/src/base/Checker.ml +++ b/infer/src/base/Checker.ml @@ -6,6 +6,7 @@ *) open! IStd +module L = Die type t = | AnnotationReachability @@ -42,7 +43,7 @@ type support = NoSupport | Support | ExperimentalSupport | ToySupport type cli_flags = {long: string; deprecated: string list; show_in_help: bool} type config = - { name: string + { id: string ; support: Language.t -> support ; short_documentation: string ; cli_flags: cli_flags option @@ -52,7 +53,7 @@ type config = (* support for languages should be consistent with the corresponding callbacks registered. Or maybe with the issues reported in link with each analysis. Some runtime check probably needed. *) -let config checker = +let config_unsafe checker = let supports_clang_and_java _ = Support in let supports_clang_and_java_experimental _ = ExperimentalSupport in let supports_clang (language : Language.t) = @@ -66,7 +67,7 @@ let config checker = in match checker with | AnnotationReachability -> - { name= "annotation reachability" + { id= "annotation-reachability" ; support= supports_clang_and_java ; short_documentation= "the annotation reachability checker. Given a pair of source and sink annotation, e.g. \ @@ -77,7 +78,7 @@ let config checker = ; enabled_by_default= false ; activates= [] } | Biabduction -> - { name= "biabduction" + { id= "biabduction" ; support= supports_clang_and_java ; short_documentation= "the separation logic based bi-abduction analysis using the checkers framework" @@ -85,7 +86,7 @@ let config checker = ; enabled_by_default= true ; activates= [] } | BufferOverrunAnalysis -> - { name= "buffer overrun analysis" + { id= "buffer-overrun-analysis" ; support= supports_clang_and_java ; short_documentation= "internal part of the buffer overrun analysis that computes values at each program \ @@ -94,35 +95,35 @@ let config checker = ; enabled_by_default= false ; activates= [] } | BufferOverrunChecker -> - { name= "buffer overrun checker" + { id= "buffer-overrun-checker" ; support= supports_clang_and_java ; short_documentation= "the buffer overrun analysis" ; cli_flags= Some {long= "bufferoverrun"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [BufferOverrunAnalysis] } | ClassLoads -> - { name= "Class loading analysis" + { id= "class-loading-analysis" ; support= supports_java ; short_documentation= "Java class loading analysis" ; cli_flags= Some {long= "class-loads"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [] } | Cost -> - { name= "cost analysis" + { id= "cost-analysis" ; support= supports_clang_and_java ; short_documentation= "checker for performance cost analysis" ; cli_flags= Some {long= "cost"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [BufferOverrunAnalysis] } | Eradicate -> - { name= "eradicate" + { id= "eradicate" ; support= supports_java ; short_documentation= "the eradicate @Nullable checker for Java annotations" ; cli_flags= Some {long= "eradicate"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [] } | FragmentRetainsView -> - { name= "fragment retains view" + { id= "fragment-retains-view" ; support= supports_java ; short_documentation= "detects when Android fragments are not explicitly nullified before becoming unreabable" @@ -130,7 +131,7 @@ let config checker = ; enabled_by_default= true ; activates= [] } | ImmutableCast -> - { name= "immutable cast" + { id= "immutable-cast" ; support= supports_java ; short_documentation= "the detection of object cast from immutable type to mutable type. For instance, it will \ @@ -139,14 +140,14 @@ let config checker = ; enabled_by_default= false ; activates= [] } | Impurity -> - { name= "impurity" + { id= "impurity" ; support= supports_clang_and_java_experimental ; short_documentation= "[EXPERIMENTAL] Impurity analysis" ; cli_flags= Some {long= "impurity"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [Pulse] } | InefficientKeysetIterator -> - { name= "inefficient keyset iterator" + { id= "inefficient-keyset-iterator" ; support= supports_java ; short_documentation= "Check for inefficient uses of keySet iterator that access both the key and the value." @@ -154,35 +155,35 @@ let config checker = ; enabled_by_default= true ; activates= [] } | Linters -> - { name= "AST Language (AL) linters" + { id= "al-linters" ; support= supports_clang ; short_documentation= "syntactic linters" ; cli_flags= Some {long= "linters"; deprecated= []; show_in_help= true} ; enabled_by_default= true ; activates= [] } | LithoRequiredProps -> - { name= "litho-required-props" + { id= "litho-required-props" ; support= supports_java_experimental ; short_documentation= "[EXPERIMENTAL] Required Prop check for Litho" ; cli_flags= Some {long= "litho-required-props"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [] } | Liveness -> - { name= "liveness" + { id= "liveness" ; support= supports_clang ; short_documentation= "the detection of dead stores and unused variables" ; cli_flags= Some {long= "liveness"; deprecated= []; show_in_help= true} ; enabled_by_default= true ; activates= [] } | LoopHoisting -> - { name= "loop hoisting" + { id= "loop-hoisting" ; support= supports_clang_and_java ; short_documentation= "checker for loop-hoisting" ; cli_flags= Some {long= "loop-hoisting"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [BufferOverrunAnalysis; Purity] } | NullsafeDeprecated -> - { name= "nullsafe" + { id= "nullsafe" ; support= (fun _ -> NoSupport) ; short_documentation= "[RESERVED] Reserved for nullsafe typechecker, use --eradicate for now" ; cli_flags= @@ -193,7 +194,7 @@ let config checker = ; enabled_by_default= false ; activates= [] } | PrintfArgs -> - { name= "printf args" + { id= "printf-args" ; support= supports_java ; short_documentation= "the detection of mismatch between the Java printf format strings and the argument types \ @@ -203,49 +204,49 @@ let config checker = ; enabled_by_default= false ; activates= [] } | Pulse -> - { name= "pulse" + { id= "pulse" ; support= supports_clang_and_java_experimental ; short_documentation= "[EXPERIMENTAL] memory and lifetime analysis" ; cli_flags= Some {long= "pulse"; deprecated= ["-ownership"]; show_in_help= true} ; enabled_by_default= false ; activates= [] } | Purity -> - { name= "purity" + { id= "purity" ; support= supports_clang_and_java_experimental ; short_documentation= "[EXPERIMENTAL] Purity analysis" ; cli_flags= Some {long= "purity"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [BufferOverrunAnalysis] } | Quandary -> - { name= "quandary" + { id= "quandary" ; support= supports_clang_and_java ; short_documentation= "the quandary taint analysis" ; cli_flags= Some {long= "quandary"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [] } | RacerD -> - { name= "RacerD" + { id= "RacerD" ; support= supports_clang_and_java ; short_documentation= "the RacerD thread safety analysis" ; cli_flags= Some {long= "racerd"; deprecated= ["-threadsafety"]; show_in_help= true} ; enabled_by_default= true ; activates= [] } | ResourceLeakLabExercise -> - { name= "resource leak lab exercise" + { id= "resource-leak-lab" ; support= (fun _ -> ToySupport) ; short_documentation= "" ; cli_flags= Some {long= "resource-leak"; deprecated= []; show_in_help= false} ; enabled_by_default= false ; activates= [] } | SIOF -> - { name= "SIOF" + { id= "SIOF" ; support= supports_clang ; short_documentation= "the Static Initialization Order Fiasco analysis (C++ only)" ; cli_flags= Some {long= "siof"; deprecated= []; show_in_help= true} ; enabled_by_default= true ; activates= [] } | SelfInBlock -> - { name= "Self captured in block checker" + { id= "self-in-block" ; support= supports_clang ; short_documentation= "checker to flag incorrect uses of when Objective-C blocks capture self" @@ -253,21 +254,21 @@ let config checker = ; enabled_by_default= true ; activates= [] } | Starvation -> - { name= "Starvation analysis" + { id= "starvation" ; support= supports_clang_and_java ; short_documentation= "starvation analysis" ; cli_flags= Some {long= "starvation"; deprecated= []; show_in_help= true} ; enabled_by_default= true ; activates= [] } | TOPL -> - { name= "TOPL" + { id= "TOPL" ; support= supports_clang_and_java_experimental ; short_documentation= "TOPL" ; cli_flags= Some {long= "topl"; deprecated= []; show_in_help= true} ; enabled_by_default= false ; activates= [Biabduction] } | Uninit -> - { name= "uninitialized variables" + { id= "uninit" ; support= supports_clang ; short_documentation= "checker for use of uninitialized values" ; cli_flags= Some {long= "uninit"; deprecated= []; show_in_help= true} @@ -275,4 +276,16 @@ let config checker = ; activates= [] } -let get_name c = (config c).name +let config c = + let config = config_unsafe c in + let is_illegal_id_char c = match c with 'a' .. 'z' | 'A' .. 'Z' | '-' -> false | _ -> true in + String.find config.id ~f:is_illegal_id_char + |> Option.iter ~f:(fun c -> + L.die InternalError + "Illegal character '%c' in id: '%s'. Checker ids must be easy to pass on the command \ + line." + c config.id ) ; + config + + +let get_id c = (config c).id diff --git a/infer/src/base/Checker.mli b/infer/src/base/Checker.mli index 09edf6c42..723027c8a 100644 --- a/infer/src/base/Checker.mli +++ b/infer/src/base/Checker.mli @@ -55,15 +55,15 @@ type cli_flags = ; show_in_help: bool } type config = - { name: string + { id: string ; support: Language.t -> support ; short_documentation: string ; cli_flags: cli_flags option (** If [None] then the checker cannot be enabled/disabled from the command line. *) ; enabled_by_default: bool - ; activates: t list (** TODO doc *) } + ; activates: t list (** list of checkers that get enabled when this checker is enabled *) } val config : t -> config -val get_name : t -> string -(** [get_name c] is [(config c).name] *) +val get_id : t -> string +(** [get_id c] is [(config c).id] *) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index d2daafc67..c806399e3 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -139,8 +139,8 @@ end = struct (String.capitalize what) unique_id what new_ what old in if not (Checker.equal checker checker_old) then - die_of_mismatch ~what:"checker" ~old:(Checker.get_name checker_old) - ~new_:(Checker.get_name checker) ; + die_of_mismatch ~what:"checker" ~old:(Checker.get_id checker_old) + ~new_:(Checker.get_id checker) ; if not (equal_visibility visibility visibility_old) then die_of_mismatch ~what:"visibility" ~old:(string_of_visibility visibility_old)