[reportdiff] add filtered out bugs to preexisting

Summary:
Once the fixed/preexisting/introduced sets have been computed, they endure
further filtering which may decide that more of them are equal. These bugs just
get dropped on the floor. Put these into preexisting as well instead, at least
in the case of the "skip_duplicated_types_on_filenames" filter.

Reviewed By: martinoluca

Differential Revision: D5274248

fbshipit-source-id: 99b3f3d

infer/src/Makefile

@@ -138,7 +138,7 @@ EXTRA_DEPS = opensource
 endif
 
 DEPENDENCIES = \
-  absint backend base bufferoverrun checkers eradicate harness integration IR labs quandary \
+  absint backend base bufferoverrun checkers eradicate harness integration IR labs quandary unit \
   $(EXTRA_DEPS)
 
 # ocamlbuild command with options common to all build targets

infer/src/backend/DifferentialFilters.ml

@@ -55,28 +55,30 @@ module FileRenamings = struct
   end
 end
 
-(* Remove duplicates between two lists whenever pred is true for such element *)
+(** Returns a triple [(l1', dups, l2')] where [dups] is the set of elements of that are in the
+    intersection of [l1] and [l2] according to [cmd] and additionally satisfy [pred], and [lN'] is
+    [lN] minus [dups]. [dups] contains only one witness for each removed issue, taken from [l1]. *)
 let relative_complements ~cmp ?(pred=(fun _ -> true)) l1 l2 =
-  let rec aux last_dup ((out_l1, out_l2) as out) in_l1 in_l2 =
-    let is_last_seen_dup v = match last_dup with
-      | Some ld -> Int.equal (cmp ld v) 0
-      | None -> false in
+  let rec aux ((out_l1, dups, out_l2) as out) in_l1 in_l2 =
+    let is_last_seen_dup v = match dups with
+      | ld::_ -> Int.equal (cmp ld v) 0
+      | [] -> false in
     match in_l1, in_l2 with
     | i::is, f::fs when Int.equal (cmp i f) 0 -> (* i = f *)
-        if pred i then aux (Some i) (out_l1, out_l2) is fs
-        else aux None (i::out_l1, f::out_l2) is fs
+        if pred i then aux (out_l1, i::dups, out_l2) is fs
+        else aux (i::out_l1, dups, f::out_l2) is fs
     | i::is, f::_ when cmp i f < 0 -> (* i < f *)
         let out_l1' = if is_last_seen_dup i then out_l1 else i::out_l1 in
-        aux last_dup (out_l1', out_l2) is in_l2
+        aux (out_l1', dups, out_l2) is in_l2
     | _::_, f::fs ->  (* i > f *)
         let out_l2' = if is_last_seen_dup f then out_l2 else f::out_l2 in
-        aux last_dup (out_l1, out_l2') in_l1 fs
-    | i::is, [] when is_last_seen_dup i -> aux last_dup out is in_l2
-    | [], f::fs when is_last_seen_dup f -> aux last_dup out in_l1 fs
-    | _, _ -> List.rev_append in_l1 out_l1, List.rev_append in_l2 out_l2 in
+        aux (out_l1, dups, out_l2') in_l1 fs
+    | i::is, [] when is_last_seen_dup i -> aux out is in_l2
+    | [], f::fs when is_last_seen_dup f -> aux out in_l1 fs
+    | _, _ -> List.rev_append in_l1 out_l1, dups, List.rev_append in_l2 out_l2 in
   let l1_sorted = List.sort ~cmp l1 in
   let l2_sorted = List.sort ~cmp l2 in
-  aux None ([], []) l1_sorted l2_sorted
+  aux ([], [], []) l1_sorted l2_sorted
 
 type issue_file_with_renaming = Jsonbug_t.jsonbug * (string option)
 
@@ -92,7 +94,7 @@ let skip_duplicated_types_on_filenames
     [%compare : string * issue_file_with_renaming]
       (issue1.Jsonbug_t.bug_type, issue_with_previous_file1)
       (issue2.Jsonbug_t.bug_type, issue_with_previous_file2) in
-  let introduced, fixed =
+  let introduced, preexisting, fixed =
     (* All comparisons will be made against filenames *before* renamings.
        This way, all introduced and fixed issues can be sorted independently
        over the same domain. *)
@@ -100,10 +102,13 @@ let skip_duplicated_types_on_filenames
       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 introduced_normalized', fixed_normalized' =
+    let introduced_normalized', preexisting', fixed_normalized' =
       relative_complements ~cmp introduced_normalized fixed_normalized in
-    List.map ~f:fst introduced_normalized', List.map ~f:fst fixed_normalized' in
-  {introduced; fixed; preexisting = diff.preexisting}
+    let list_map_fst = List.map ~f:fst in
+    list_map_fst introduced_normalized',
+    (list_map_fst preexisting') @ diff.preexisting,
+    list_map_fst fixed_normalized' in
+  {introduced; fixed; preexisting}
 
 let java_anon_class_pattern = Str.regexp "\\$[0-9]+"
 
@@ -168,8 +173,8 @@ let skip_anonymous_class_renamings (diff: Differential.t) : Differential.t =
         true
       with Not_found -> false in
     is_java_file () && has_anonymous_class_token () in
-  let introduced, fixed = relative_complements ~cmp ~pred diff.introduced diff.fixed in
-  {introduced; fixed; preexisting = diff.preexisting}
+  let introduced, preexisting, fixed = relative_complements ~cmp ~pred diff.introduced diff.fixed in
+  {introduced; fixed; preexisting = preexisting @ diff.preexisting}
 
 (* Filter out null dereferences reported by infer if file has eradicate
    enabled, to avoid double reporting. *)

infer/src/backend/DifferentialFilters.mli

@@ -33,7 +33,7 @@ val do_filter : Differential.t -> FileRenamings.t -> skip_duplicated_types:bool
 
 module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY : sig
   val relative_complements :
-    cmp:('a -> 'a -> int) -> ?pred:('a -> bool) -> 'a list -> 'a list -> 'a list * 'a list
+    cmp:('a -> 'a -> int) -> ?pred:('a -> bool) -> 'a list -> 'a list -> 'a list * 'a list * 'a list
   val skip_duplicated_types_on_filenames : FileRenamings.t -> Differential.t -> Differential.t
   val java_anon_class_pattern : Str.regexp
   val value_of_qualifier_tag : Jsonbug_t.tag_value_record list -> string -> string option

infer/src/unit/DifferentialFiltersTests.ml

@@ -115,94 +115,96 @@ let test_file_renamings_find_previous =
         name >:: create_test test_input expected_output)
 
 let test_relative_complements =
-  let create_test pred (l1, l2) (expected_l1, expected_l2) _ =
+  let create_test pred (l1, l2) (expected_l1, expected_l2, expected_l3) _ =
     let cmp = Int.compare in
-    let output_l1, output_l2 =
+    let output_l1, output_l2, output_l3 =
       DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.relative_complements
         ~cmp ~pred l1 l2 in
     let list_equal l1 l2 = List.equal ~equal:(fun v1 v2 -> Int.equal (cmp v1 v2) 0) l1 l2 in
     assert_equal
       ~pp_diff:(pp_diff_of_int_list "First list") ~cmp:list_equal expected_l1 output_l1;
     assert_equal
-      ~pp_diff:(pp_diff_of_int_list "Second list") ~cmp:list_equal expected_l2 output_l2 in
+      ~pp_diff:(pp_diff_of_int_list "Second list") ~cmp:list_equal expected_l2 output_l2;
+    assert_equal
+      ~pp_diff:(pp_diff_of_int_list "Third list") ~cmp:list_equal expected_l3 output_l3 in
   [
     (
       "test_relative_complements_with_always_true_pred",
       (fun _ -> true),
       ([0;1;2;3;4;5], [5;3;7;1;1;2]),
-      ([4;0], [7])
+      ([4;0], [5;3;2;1], [7])
     );
     (
       "test_relative_complements_with_even_numbers_pred",
       (fun i -> Int.equal (i mod 2) 0), (* skip when even, keep odd *)
       ([0;1;2;3;4;5], [5;3;7;1;1;2]),
-      ([5;4;3;1;0], [7;5;3;1;1])
+      ([5;4;3;1;0], [2], [7;5;3;1;1])
     );
     (
       "test_relative_complements_with_even_numbers_pred_2",
       (fun i -> Int.equal (i mod 2) 0), (* skip when even, keep odd *)
       ([0;1;2;3;5;5], [1;1;2;3;4;7]),
-      ([5;5;3;1;0], [7;4;3;1;1])
+      ([5;5;3;1;0], [2], [7;4;3;1;1])
     );
     (
       "test_relative_complements_with_always_true_pred_and_disjoint_lists_of_different_length",
       (fun _ -> true),
       ([0;3;2;3;5], [9;7;6;8;4;6;9]),
-      ([5;3;3;2;0], [9;9;8;7;6;6;4])
+      ([5;3;3;2;0], [], [9;9;8;7;6;6;4])
     );
     (
       "test_relative_complements_with_always_true_pred_and_lists_of_different_length",
       (fun _ -> true),
       ([0;3;2;3], [9;7;3;8;0;6;9;4]),
-      ([2], [9;9;8;7;6;4])
+      ([2], [3;0], [9;9;8;7;6;4])
     );
     (
       "test_relative_complements_with_odd_numbers_on_lists_of_different_length",
       (fun i -> Int.equal (i mod 2) 1), (* skip when odd, keep even *)
       ([0;3;2;3], [9;7;3;8;0;6;9;4]),
-      ([2;0], [9;9;8;7;6;4;0])
+      ([2;0], [3], [9;9;8;7;6;4;0])
     );
     (
       "test_relative_complements_with_singleton_lists1",
       (fun _ -> true),
       ([0], [0;1;0;0]),
-      ([], [1])
+      ([], [0], [1])
     );
     (
       "test_relative_complements_with_singleton_lists2",
       (fun _ -> true),
       ([0;1;0;0], [0]),
-      ([1], [])
+      ([1], [0], [])
     );
     (
       "test_relative_complements_with_singleton_lists3",
       (fun _ -> true),
       ([0], [0]),
-      ([], [])
+      ([], [0], [])
     );
     (
       "test_relative_complements_with_singleton_lists4",
       (fun _ -> true),
       ([0], [1]),
-      ([0], [1])
+      ([0], [], [1])
     );
     (
       "test_relative_complements_with_empty_lists1",
       (fun _ -> true),
       ([], [0;1;0;0]),
-      ([], [1;0;0;0])
+      ([], [], [1;0;0;0])
     );
     (
       "test_relative_complements_with_empty_lists2",
       (fun _ -> true),
       ([0;1;0;0], []),
-      ([1;0;0;0], [])
+      ([1;0;0;0], [], [])
     );
     (
       "test_relative_complements_with_empty_lists3",
       (fun _ -> true),
       ([], []),
-      ([], [])
+      ([], [], [])
     );
   ]
   |> List.map
@@ -243,7 +245,7 @@ let test_skip_duplicated_types_on_filenames =
       [3] (sorted_hashes_of_issues diff'.fixed);
     assert_equal
       ~pp_diff:(pp_diff_of_int_list "Hashes of preexisting")
-      [222] (sorted_hashes_of_issues diff'.preexisting) in
+      [22; 55; 111; 222] (sorted_hashes_of_issues diff'.preexisting) in
   "test_skip_duplicated_types_on_filenames" >:: do_assert
 
 let test_value_of_qualifier_tag =
@@ -340,7 +342,7 @@ let test_skip_anonymous_class_renamings =
            ~key:2
            ~hash:2 ();
        ],
-     ([4;5], [2], []));
+     ([4;5], [2], [3]));
     ("test_skip_anonymous_class_renamings_with_empty_qualifier_tags",
      Differential.of_reports
        ~current_report:[
@@ -372,7 +374,7 @@ let test_skip_anonymous_class_renamings =
            ~key:1
            ~hash:4 ();
        ],
-     ([1], [2], []));
+     ([1], [2], [3]));
     ("test_skip_anonymous_class_renamings_with_matching_non_anonymous_procedure_ids",
      Differential.of_reports
        ~current_report:[
@@ -436,7 +438,7 @@ let test_skip_anonymous_class_renamings =
            ~key:1
            ~hash:4 ();
        ],
-     ([3], [4], []));
+     ([3], [4], [1]));
     ("test_skip_anonymous_class_renamings_with_different_call_procedure_qualifier_tags",
      Differential.of_reports
        ~current_report:[

infer/tests/build_systems/differential_skip_anonymous_class_renamings/preexisting.exp

@@ -0,0 +1 @@
+NULL_DEREFERENCE, src/DiffExample.java, int DiffExample$4.aaa(), 0, DiffExample$4.aaa():int.a7bd5e834e7a8fb4284c75f621544e3e, DiffExample$4.aaa():int

infer/tests/build_systems/differential_skip_duplicated_types_on_filenames/preexisting.exp

@@ -0,0 +1 @@
+NULL_DEREFERENCE, src/DiffExample.java, int DiffExample.triggerNpeRenamed(), 1, DiffExample.triggerNpeRenamed():int.3e4070b89fdefd2c64fd437530b8dda9, DiffExample.triggerNpeRenamed():int

infer/tests/build_systems/differential_skip_duplicated_types_on_filenames_with_renamings/preexisting.exp

@@ -0,0 +1,2 @@
+NULL_DEREFERENCE, src/DiffExampleRenamed.java, int DiffExampleRenamed.triggerNpe(), 1, DiffExampleRenamed.triggerNpe():int.708d86c0dc5dc4e5dbbbcd38276ce86f, DiffExampleRenamed.triggerNpe():int
+RESOURCE_LEAK, src/DiffExampleRenamed.java, void DiffExampleRenamed.openResource(), 5, DiffExampleRenamed.openResource():void.715df29eeb5b62c29d4081c6dc9d407a, DiffExampleRenamed.openResource():void