From b158f5cd3006dcd33865fc65abce470408038570 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 22 Jun 2017 05:05:26 -0700 Subject: [PATCH] [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 | 2 +- infer/src/backend/DifferentialFilters.ml | 43 +++++++++++-------- infer/src/backend/DifferentialFilters.mli | 2 +- infer/src/unit/DifferentialFiltersTests.ml | 42 +++++++++--------- .../preexisting.exp | 1 + .../preexisting.exp | 1 + .../preexisting.exp | 2 + 7 files changed, 52 insertions(+), 41 deletions(-) diff --git a/infer/src/Makefile b/infer/src/Makefile index c25be2951..7e4bb24e9 100644 --- a/infer/src/Makefile +++ b/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 diff --git a/infer/src/backend/DifferentialFilters.ml b/infer/src/backend/DifferentialFilters.ml index 68272156c..799039e8b 100644 --- a/infer/src/backend/DifferentialFilters.ml +++ b/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. *) diff --git a/infer/src/backend/DifferentialFilters.mli b/infer/src/backend/DifferentialFilters.mli index 98d28abd5..00514ff30 100644 --- a/infer/src/backend/DifferentialFilters.mli +++ b/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 diff --git a/infer/src/unit/DifferentialFiltersTests.ml b/infer/src/unit/DifferentialFiltersTests.ml index 709d52b61..4d8a6ff45 100644 --- a/infer/src/unit/DifferentialFiltersTests.ml +++ b/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:[ diff --git a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/preexisting.exp b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/preexisting.exp index e69de29bb..600008d3d 100644 --- a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/preexisting.exp +++ b/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 diff --git a/infer/tests/build_systems/differential_skip_duplicated_types_on_filenames/preexisting.exp b/infer/tests/build_systems/differential_skip_duplicated_types_on_filenames/preexisting.exp index e69de29bb..fb9470b49 100644 --- a/infer/tests/build_systems/differential_skip_duplicated_types_on_filenames/preexisting.exp +++ b/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 diff --git a/infer/tests/build_systems/differential_skip_duplicated_types_on_filenames_with_renamings/preexisting.exp b/infer/tests/build_systems/differential_skip_duplicated_types_on_filenames_with_renamings/preexisting.exp index e69de29bb..430f67ac8 100644 --- a/infer/tests/build_systems/differential_skip_duplicated_types_on_filenames_with_renamings/preexisting.exp +++ b/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