DifferentialFilters: simplify find_previous

Summary: Returns `string` rather than `string option`, simplifies comparison.

Reviewed By: martinoluca

Differential Revision: D9633523

fbshipit-source-id: 69d6cb305

infer/src/backend/DifferentialFilters.ml

@@ -61,7 +61,9 @@ module FileRenamings = struct
 
   let from_json_file file : t = from_json (In_channel.read_all file)
 
-  let find_previous (t : t) current = CurrentToPreviousMap.find_opt current t
+  let find_previous (t : t) current =
+    try CurrentToPreviousMap.find current t with Caml.Not_found -> current
+
 
   module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY = struct
     type nonrec renaming = renaming = {current: string; previous: string}
@@ -116,21 +118,11 @@ let relative_complements ~compare ?(pred = fun _ -> true) l1 l2 =
   aux ([], [], []) l1_sorted l2_sorted
 
 
-type issue_file_with_renaming = Jsonbug_t.jsonbug * string option
-
 let skip_duplicated_types_on_filenames renamings (diff : Differential.t) : Differential.t =
-  let compare_issue_file_with_renaming (issue1, previous_file1) (issue2, previous_file2) =
-    let f1, f2 =
-      ( Option.value previous_file1 ~default:issue1.Jsonbug_t.file
-      , Option.value previous_file2 ~default:issue2.Jsonbug_t.file )
-    in
-    String.compare f1 f2
-  in
-  let compare ((issue1, _) as issue_with_previous_file1) ((issue2, _) as issue_with_previous_file2)
-      =
-    [%compare: Caml.Digest.t * string * issue_file_with_renaming]
-      (issue1.Jsonbug_t.node_key, issue1.Jsonbug_t.bug_type, issue_with_previous_file1)
-      (issue2.Jsonbug_t.node_key, issue2.Jsonbug_t.bug_type, issue_with_previous_file2)
+  let compare (issue1, previous_file1) (issue2, previous_file2) =
+    [%compare: Caml.Digest.t * string * string]
+      (issue1.Jsonbug_t.node_key, issue1.Jsonbug_t.bug_type, previous_file1)
+      (issue2.Jsonbug_t.node_key, issue2.Jsonbug_t.bug_type, previous_file2)
   in
   let introduced, preexisting, fixed =
     (* All comparisons will be made against filenames *before* renamings.
@@ -140,7 +132,7 @@ let skip_duplicated_types_on_filenames renamings (diff : Differential.t) : Diffe
       List.map diff.introduced ~f:(fun i ->
           (i, FileRenamings.find_previous renamings i.Jsonbug_t.file) )
     in
-    let fixed_normalized = List.map diff.fixed ~f:(fun f -> (f, None)) in
+    let fixed_normalized = List.map diff.fixed ~f:(fun f -> (f, f.Jsonbug_t.file)) in
     let introduced_normalized', preexisting', fixed_normalized' =
       relative_complements ~compare introduced_normalized fixed_normalized
     in

infer/src/backend/DifferentialFilters.mli

@@ -23,7 +23,7 @@ module FileRenamings : sig
 
     val pp : Format.formatter -> t -> unit
 
-    val find_previous : t -> string -> string option
+    val find_previous : t -> string -> string
 
     val from_json : string -> t
   end

infer/src/unit/DifferentialFiltersTests.ml

@@ -68,22 +68,17 @@ let test_file_renamings_find_previous =
         ; {current= "ccc.java"; previous= "DDD.java"}
         ; {current= "eee.java"; previous= "FFF.java"} ])
   in
-  let cmp s1 s2 = [%compare.equal: string option] s1 s2 in
   let find_previous =
     DifferentialFilters.FileRenamings.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.find_previous
   in
   let pp_diff fmt (expected, actual) =
-    let pp_str_opt fmt str_opt =
-      let out = match str_opt with Some str -> "Some " ^ str | None -> "None" in
-      Format.pp_print_string fmt out
-    in
-    Format.fprintf fmt "Expected '%a' but got '%a'" pp_str_opt expected pp_str_opt actual
+    Format.fprintf fmt "Expected '%s' but got '%s'" expected actual
   in
   let create_test input expected_previous _ =
-    assert_equal ~cmp ~pp_diff expected_previous (find_previous renamings input)
+    assert_equal ~cmp:String.equal ~pp_diff expected_previous (find_previous renamings input)
   in
-  [ ("test_file_renamings_find_previous_with_existing_value", "ccc.java", Some "DDD.java")
-  ; ("test_file_renamings_find_previous_with_existing_value", "abc.java", None) ]
+  [ ("test_file_renamings_find_previous_with_existing_value", "ccc.java", "DDD.java")
+  ; ("test_file_renamings_find_previous_with_existing_value", "abc.java", "abc.java") ]
   |> List.map ~f:(fun (name, test_input, expected_output) ->
          name >:: create_test test_input expected_output )