[issues] make sure the immutable parts of issues are always the same

Summary:
We allow some fields of issues to be defined dynamically, more precisely
when loading AL files. We don't want this to be abused and in particular
we don't want to miss an issue being declared once for a checker and
another time for another as this would be hard to debug.

Also, only register unknown issue types as coming from AL if they
haven't been registered already.

Reviewed By: skcho

Differential Revision: D21685879

fbshipit-source-id: 9e9438a75

infer/src/base/Config.ml

@@ -1184,7 +1184,14 @@ and () =
     let (_ : string list ref) =
       CLOpt.mk_string_list ?deprecated ~long
         ~f:(fun issue_id ->
-          let issue = IssueType.register_from_string ~id:issue_id Linters in
+          let issue =
+            match IssueType.find_from_string ~id:issue_id with
+            | Some issue ->
+                issue
+            | None ->
+                (* unknown issue type: assume it will be defined in AL *)
+                IssueType.register_from_string ~id:issue_id Linters
+          in
           IssueType.set_enabled issue b ;
           issue_id )
         ?default ~meta:"issue_type"

infer/src/base/IssueType.ml

@@ -7,6 +7,7 @@
 
 open! IStd
 module F = Format
+module L = Die
 
 (* Make sure we cannot create new issue types other than by calling [register_from_string]. This is because
      we want to keep track of the list of all the issues ever declared. *)
@@ -24,6 +25,8 @@ module Unsafe : sig
 
   val pp : F.formatter -> t -> unit
 
+  val find_from_string : id:string -> t option
+
   val register_from_string :
        ?enabled:bool
     -> ?hum:string
@@ -77,6 +80,8 @@ end = struct
 
   let set_enabled issue b = issue.enabled <- b
 
+  let find_from_string ~id:unique_id = IssueSet.find_rank !all_issues unique_id
+
   (** Avoid creating new issue types. The idea is that there are three types of issue types:
 
       + Statically pre-defined issue types, namely the ones in this module
@@ -90,10 +95,21 @@ end = struct
         of the issue type, eg in AL. *)
   let register_from_string ?(enabled = true) ?hum:hum0 ?doc_url ?linters_def_file ~id:unique_id
       checker =
-    match IssueSet.find_rank !all_issues unique_id with
+    match find_from_string ~id:unique_id with
-    | Some issue ->
+    | ((Some
-        (* update human-readable string in case it was supplied this time, but keep the previous
+         ( { unique_id= _ (* we know it has to be the same *)
-           value of enabled (see doc comment) *)
+           ; checker= checker_old
+           ; enabled= _ (* not touching this one since [Config] will have set it *)
+           ; hum= _ (* mutable field to update *)
+           ; doc_url= _ (* mutable field to update *)
+           ; linters_def_file= _ (* mutable field to update *) } as issue ))[@warning "+9"]) ->
+        (* update fields that were supplied this time around, but keep the previous values of others
+           and assert that the immutable fields are the same (see doc comment) *)
+        if not (Checker.equal checker checker_old) then
+          L.die InternalError
+            "Checker definition for issue \"%s\" doesn't match: found new checker \"%s\" but \
+             checker \"%s\" was already registered for this issue type"
+            unique_id (Checker.get_name checker) (Checker.get_name checker_old) ;
         Option.iter hum0 ~f:(fun hum -> issue.hum <- hum) ;
         if Option.is_some doc_url then issue.doc_url <- doc_url ;
         if Option.is_some linters_def_file then issue.linters_def_file <- linters_def_file ;

infer/src/base/IssueType.mli

@@ -24,6 +24,9 @@ val all_issues : unit -> t list
 val pp : Format.formatter -> t -> unit
 (** pretty print a localised string *)
 
+val find_from_string : id:string -> t option
+(** return the issue type if it was previously registered *)
+
 val register_from_string :
      ?enabled:bool
   -> ?hum:string