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
5 years ago · 7310726155
parent e86ea5ca7f
commit 7310726155
5 changed files with 54 additions and 41 deletions
--- 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 *)
--- 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
--- 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
--- 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] *)
--- 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)