minor readability improvement in IssueType.ml

Summary:
- avoid creating issues just to look up their `unique_id` in the set
- avoid `let _ =` since it can hide partial applications
- delete outdated comment

Reviewed By: skcho

Differential Revision: D21663959

fbshipit-source-id: e50d02447

infer/src/base/IssueType.ml

@@ -4,7 +4,9 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *)
+
 open! IStd
+module F = Format
 
 (* 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. *)
@@ -20,6 +22,8 @@ module Unsafe : sig
 
   val equal : t -> t -> bool
 
+  val pp : F.formatter -> t -> unit
+
   val register_from_string :
        ?enabled:bool
     -> ?hum:string
@@ -33,7 +37,7 @@ module Unsafe : sig
        ?enabled:bool
     -> ?is_on_ui_thread:bool
     -> kind:CostKind.t
-    -> (string -> string, Format.formatter, unit, string) format4
+    -> (string -> string, F.formatter, unit, string) format4
     -> t
 
   val all_issues : unit -> t list
@@ -52,10 +56,16 @@ end = struct
     let compare {unique_id= id1} {unique_id= id2} = String.compare id1 id2
 
     let equal = [%compare.equal: t]
+
+    type rank = string
+
+    let to_rank {unique_id} = unique_id
+
+    let pp fmt t = F.pp_print_string fmt t.unique_id
   end
 
   include T
-  module IssueSet = Caml.Set.Make (T)
+  module IssueSet = PrettyPrintable.MakePPUniqRankSet (String) (T)
 
   (** keep track of the list of all declared issue types *)
   let all_issues = ref IssueSet.empty
@@ -80,25 +90,25 @@ 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 =
-    let hum = match hum0 with Some str -> str | _ -> prettify unique_id in
-    let issue = {unique_id; checker; enabled; hum; doc_url; linters_def_file} in
-    try
-      let old = IssueSet.find issue !all_issues in
-      (* update human-readable string in case it was supplied this time, but keep the previous
-         value of enabled (see doc comment) *)
-      if Option.is_some hum0 then old.hum <- hum ;
-      if Option.is_some doc_url then old.doc_url <- doc_url ;
-      if Option.is_some linters_def_file then old.linters_def_file <- linters_def_file ;
-      old
-    with Caml.Not_found ->
-      all_issues := IssueSet.add issue !all_issues ;
-      issue
+    match IssueSet.find_rank !all_issues unique_id with
+    | Some issue ->
+        (* update human-readable string in case it was supplied this time, but keep the previous
+           value of enabled (see doc comment) *)
+        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 ;
+        issue
+    | None ->
+        let hum = match hum0 with Some str -> str | _ -> prettify unique_id in
+        let issue = {unique_id; checker; enabled; hum; doc_url; linters_def_file} in
+        all_issues := IssueSet.add !all_issues issue ;
+        issue
 
 
   (** cost issues are already registered below.*)
   let register_from_cost_string ?(enabled = true) ?(is_on_ui_thread = false) ~(kind : CostKind.t) s
       =
-    let issue_type_base = Format.asprintf s (CostKind.to_issue_string kind) in
+    let issue_type_base = F.asprintf s (CostKind.to_issue_string kind) in
     let issue_type = if is_on_ui_thread then issue_type_base ^ "_UI_THREAD" else issue_type_base in
     register_from_string ~enabled ~id:issue_type Cost
 
@@ -108,9 +118,6 @@ end
 
 include Unsafe
 
-(** pretty print a localised string *)
-let pp fmt t = Format.pp_print_string fmt t.unique_id
-
 let checker_can_report reporting_checker {checker= allowed_checker} =
   Checker.equal reporting_checker allowed_checker
 
@@ -627,8 +634,8 @@ let unreachable_cost_call ~kind =
 let () =
   List.iter CostKind.enabled_cost_kinds ~f:(fun CostKind.{kind} ->
       List.iter [true; false] ~f:(fun is_on_ui_thread ->
-          let _ = unreachable_cost_call ~kind in
-          let _ = expensive_cost_call ~kind ~is_on_ui_thread in
-          let _ = infinite_cost_call ~kind in
-          let _ = complexity_increase ~kind ~is_on_ui_thread in
+          ignore (unreachable_cost_call ~kind) ;
+          ignore (expensive_cost_call ~kind ~is_on_ui_thread) ;
+          ignore (infinite_cost_call ~kind) ;
+          ignore (complexity_increase ~kind ~is_on_ui_thread) ;
           () ) )

infer/src/istd/PrettyPrintable.ml

@@ -28,6 +28,12 @@ module type PrintableOrderedType = sig
   include PrintableType with type t := t
 end
 
+module type PrintableEquatableOrderedType = sig
+  include Caml.Set.OrderedType
+
+  include PrintableEquatableType with type t := t
+end
+
 module type PPSet = sig
   include Caml.Set.S
 
@@ -209,12 +215,16 @@ module type PrintableRankedType = sig
 
   val equal : t -> t -> bool
 
-  val to_rank : t -> int
+  type rank
+
+  val to_rank : t -> rank
 end
 
 module type PPUniqRankSet = sig
   type t
 
+  type rank
+
   type elt
 
   val add : t -> elt -> t
@@ -223,7 +233,7 @@ module type PPUniqRankSet = sig
 
   val equal : t -> t -> bool
 
-  val find_rank : t -> int -> elt option
+  val find_rank : t -> rank -> elt option
 
   val fold : t -> init:'accum -> f:('accum -> elt -> 'accum) -> 'accum
 
@@ -239,6 +249,8 @@ module type PPUniqRankSet = sig
 
   val singleton : elt -> t
 
+  val elements : t -> elt list
+
   val remove : elt -> t -> t
 
   val union_prefer_left : t -> t -> t
@@ -246,11 +258,16 @@ module type PPUniqRankSet = sig
   val pp : ?print_rank:bool -> F.formatter -> t -> unit
 end
 
-module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type elt = Val.t = struct
-  module Map = MakePPMonoMap (Int) (Val)
+module MakePPUniqRankSet
+    (Rank : PrintableEquatableOrderedType)
+    (Val : PrintableRankedType with type rank = Rank.t) :
+  PPUniqRankSet with type elt = Val.t and type rank = Rank.t = struct
+  module Map = MakePPMonoMap (Rank) (Val)
 
   type t = Map.t
 
+  type rank = Rank.t
+
   type elt = Val.t
 
   let add map value = Map.add (Val.to_rank value) value map
@@ -278,7 +295,7 @@ module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type e
     Map.mapi
       (fun rank value ->
         let value' = f value in
-        assert (Int.equal rank (Val.to_rank value')) ;
+        assert (Rank.equal rank (Val.to_rank value')) ;
         value' )
       m
 
@@ -294,9 +311,10 @@ module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type e
     (!accum, m')
 
 
+  let elements map = Map.bindings map |> List.map ~f:snd
+
   let pp ?(print_rank = false) fmt map =
-    if print_rank then Map.pp fmt map
-    else pp_collection ~pp_item:Val.pp fmt (Map.bindings map |> List.map ~f:snd)
+    if print_rank then Map.pp fmt map else pp_collection ~pp_item:Val.pp fmt (elements map)
 
 
   let remove value map = Map.remove (Val.to_rank value) map

infer/src/istd/PrettyPrintable.mli

@@ -30,6 +30,12 @@ module type PrintableOrderedType = sig
   include PrintableType with type t := t
 end
 
+module type PrintableEquatableOrderedType = sig
+  include Caml.Set.OrderedType
+
+  include PrintableEquatableType with type t := t
+end
+
 module type PPSet = sig
   include Caml.Set.S
 
@@ -159,13 +165,17 @@ module type PrintableRankedType = sig
 
   val equal : t -> t -> bool
 
-  val to_rank : t -> int
+  type rank
+
+  val to_rank : t -> rank
 end
 
 (** set where at most one element of a given rank can be present *)
 module type PPUniqRankSet = sig
   type t
 
+  type rank
+
   type elt
 
   val add : t -> elt -> t
@@ -174,7 +184,7 @@ module type PPUniqRankSet = sig
 
   val equal : t -> t -> bool
 
-  val find_rank : t -> int -> elt option
+  val find_rank : t -> rank -> elt option
 
   val fold : t -> init:'accum -> f:('accum -> elt -> 'accum) -> 'accum
 
@@ -190,6 +200,8 @@ module type PPUniqRankSet = sig
 
   val singleton : elt -> t
 
+  val elements : t -> elt list
+
   val remove : elt -> t -> t
 
   val union_prefer_left : t -> t -> t
@@ -199,4 +211,7 @@ module type PPUniqRankSet = sig
   val pp : ?print_rank:bool -> F.formatter -> t -> unit
 end
 
-module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type elt = Val.t
+module MakePPUniqRankSet
+    (Rank : PrintableEquatableOrderedType)
+    (Val : PrintableRankedType with type rank = Rank.t) :
+  PPUniqRankSet with type elt = Val.t and type rank = Rank.t

infer/src/pulse/PulseAttribute.ml

@@ -37,6 +37,8 @@ module Attribute = struct
 
   let equal = [%compare.equal: t]
 
+  type rank = int
+
   let to_rank = Variants.to_rank
 
   let dummy_trace = Trace.Immediate {location= Location.dummy; history= []}
@@ -91,7 +93,7 @@ module Attribute = struct
 end
 
 module Attributes = struct
-  module Set = PrettyPrintable.MakePPUniqRankSet (Attribute)
+  module Set = PrettyPrintable.MakePPUniqRankSet (Int) (Attribute)
 
   let get_invalid attrs =
     Set.find_rank attrs Attribute.invalid_rank