diff --git a/infer/src/backend/DifferentialFilters.ml b/infer/src/backend/DifferentialFilters.ml index b2443fb24..6550db0dd 100644 --- a/infer/src/backend/DifferentialFilters.ml +++ b/infer/src/backend/DifferentialFilters.ml @@ -148,74 +148,10 @@ let skip_duplicated_types_on_filenames renamings (diff: Differential.t) : Differ {introduced; fixed; preexisting; costs_summary= diff.costs_summary} -let java_anon_class_pattern = Str.regexp "\\$[0-9]+" - -type procedure_id = string - -let compare_procedure_id pid1 pid2 = - (* THIS COMPARISON FUNCTION IS INTENDED FOR JAVA ONLY *) - let normalize_procedure_id pid = - let anon_token = "$ANON" in - Str.global_replace java_anon_class_pattern anon_token pid - in - let pid1_norm = normalize_procedure_id pid1 in - let pid2_norm = normalize_procedure_id pid2 in - (* NOTE: The CRC may swallow some extra chars if the anon class has more - * digits (e.g. ...$9.abcde():int.A1B2 and ...$10.abcde():in.C1FF), and this - * makes the 2 strings different. - * Cut the length to the min_length to match the 2 strings *) - let pid1_norm_trimmed, pid2_norm_trimmed = - let min_length = min (String.length pid1_norm) (String.length pid2_norm) in - (String.sub pid1_norm ~pos:0 ~len:min_length, String.sub pid2_norm ~pos:0 ~len:min_length) - in - String.compare pid1_norm_trimmed pid2_norm_trimmed - - type file_extension = string [@@deriving compare] type weak_hash = string * string * string * Caml.Digest.t [@@deriving compare] -let skip_anonymous_class_renamings (diff: Differential.t) : Differential.t = - (* - * THIS HEURISTIC IS INTENDED FOR JAVA ONLY. - * Two issues are similar (for the purpose of anonymous class renamings detection) - * when all of the following apply: - * 1) they are Java files - * 2) their weak hashes match - * 3) their anonymous procedure ids match - *) - let string_of_procedure_id issue = DB.strip_crc issue.Jsonbug_t.procedure_id in - let extension fname = snd (Filename.split_extension fname) in - let compare (i1: Jsonbug_t.jsonbug) (i2: Jsonbug_t.jsonbug) = - [%compare : file_extension option * weak_hash * procedure_id] - (extension i1.file, (i1.kind, i1.bug_type, i1.file, i1.key), string_of_procedure_id i1) - (extension i2.file, (i2.kind, i2.bug_type, i2.file, i2.key), string_of_procedure_id i2) - in - let pred (issue: Jsonbug_t.jsonbug) = - let is_java_file () = - match extension issue.file with - | Some ext -> - String.equal (String.lowercase ext) "java" - | None -> - false - in - let has_anonymous_class_token () = - try - ignore (Str.search_forward java_anon_class_pattern issue.procedure_id 0) ; - true - with Caml.Not_found -> false - in - is_java_file () && has_anonymous_class_token () - in - let introduced, preexisting, fixed = - relative_complements ~compare ~pred diff.introduced diff.fixed - in - { introduced - ; fixed - ; preexisting= preexisting @ diff.preexisting - ; costs_summary= diff.costs_summary } - - (* Strip issues whose paths are not among those we're interested in *) let interesting_paths_filter (interesting_paths: SourceFile.t list option) = match interesting_paths with @@ -245,9 +181,7 @@ let do_filter (diff: Differential.t) (renamings: FileRenamings.t) ~(skip_duplica else issues in let diff' = - diff |> (if Config.biabduction then skip_anonymous_class_renamings else Fn.id) - |> (if skip_duplicated_types then skip_duplicated_types_on_filenames renamings else Fn.id) - |> Fn.id + if skip_duplicated_types then skip_duplicated_types_on_filenames renamings diff else diff in { introduced= apply_paths_filter_if_needed `Introduced diff'.introduced ; fixed= apply_paths_filter_if_needed `Fixed diff'.fixed @@ -260,7 +194,5 @@ module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY = struct let skip_duplicated_types_on_filenames = skip_duplicated_types_on_filenames - let skip_anonymous_class_renamings = skip_anonymous_class_renamings - let interesting_paths_filter = interesting_paths_filter end diff --git a/infer/src/backend/DifferentialFilters.mli b/infer/src/backend/DifferentialFilters.mli index b737fa501..6c9b9f664 100644 --- a/infer/src/backend/DifferentialFilters.mli +++ b/infer/src/backend/DifferentialFilters.mli @@ -40,8 +40,6 @@ module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY : sig val skip_duplicated_types_on_filenames : FileRenamings.t -> Differential.t -> Differential.t - val skip_anonymous_class_renamings : Differential.t -> Differential.t - val interesting_paths_filter : SourceFile.t list option -> Jsonbug_t.jsonbug list -> Jsonbug_t.jsonbug list end diff --git a/infer/src/unit/DifferentialFiltersTests.ml b/infer/src/unit/DifferentialFiltersTests.ml index cc2d2d63e..4ed85a3f4 100644 --- a/infer/src/unit/DifferentialFiltersTests.ml +++ b/infer/src/unit/DifferentialFiltersTests.ml @@ -188,121 +188,6 @@ let test_skip_duplicated_types_on_filenames = "test_skip_duplicated_types_on_filenames" >:: do_assert -let test_skip_anonymous_class_renamings = - let create_test input_diff (exp_introduced, exp_fixed, exp_preexisting) _ = - let diff' = - DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.skip_anonymous_class_renamings - input_diff - in - assert_equal - ~pp_diff:(pp_diff_of_string_list "Hashes of introduced") - exp_introduced - (sorted_hashes_of_issues diff'.introduced) ; - assert_equal - ~pp_diff:(pp_diff_of_string_list "Hashes of fixed") - exp_fixed - (sorted_hashes_of_issues diff'.fixed) ; - assert_equal - ~pp_diff:(pp_diff_of_string_list "Hashes of preexisting") - exp_preexisting - (sorted_hashes_of_issues diff'.preexisting) - in - (* [(test_name, diff, expected hashes); ...] *) - [ ( "test_skip_anonymous_class_renamings_with_long_procedure_ids" - , Differential.of_reports - ~current_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - ( "com.whatever.package00.abcd." - ^ "ABasicExampleFragment$83.onMenuItemActionExpand(android.view.MenuItem):b." - ^ "5ab5e18cae498c35d887ce88f3d5fa82" ) - ~file:"a.java" ~key:"1" ~hash:"3" () - ; create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - ( "com.whatever.package00.abcd." - ^ "ABasicExampleFragment$83$7.onMenuItemActionExpand(android.view.MenuItem)." - ^ "522cc747174466169781c9d2fc980dbc" ) - ~file:"a.java" ~key:"1" ~hash:"4" () - ; create_fake_jsonbug ~bug_type:"bug_type_2" - ~procedure_id:"procid5.c854fd4a98113d9ab5b82deb3545de89" ~file:"b.java" ~key:"5" - ~hash:"5" () ] - ~previous_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - ( "com.whatever.package00.abcd." - ^ "ABasicExampleFragment$9.onMenuItemActionExpand(android.view.MenuItem):bo." - ^ "ba1776155fba2899542401da5bc779a5" ) - ~file:"a.java" ~key:"1" ~hash:"1" () - ; create_fake_jsonbug ~bug_type:"bug_type_2" - ~procedure_id:"procid2.92095aee3f1884c37e96feae031f4931" ~file:"b.java" ~key:"2" - ~hash:"2" () ] - , (["4"; "5"], ["2"], ["3"]) ) - ; ( "test_skip_anonymous_class_renamings_with_empty_qualifier_tags" - , Differential.of_reports - ~current_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class$1.foo():bool.bf13089cf4c47ff8ff089a1a4767324f" - ~file:"a.java" ~key:"1" ~hash:"1" () - ; create_fake_jsonbug ~bug_type:"bug_type_2" - ~procedure_id: - "com.whatever.package.Class$1.foo():bool.bf13089cf4c47ff8ff089a1a4767324f" - ~file:"a.java" ~key:"1" ~hash:"3" () ] - ~previous_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class$21$1.foo():bool.db89561ad9dab28587c8c04833f09b03" - ~file:"a.java" ~key:"1" ~hash:"2" () - ; create_fake_jsonbug ~bug_type:"bug_type_2" - ~procedure_id: - "com.whatever.package.Class$8.foo():bool.cffd4e941668063eb802183dbd3e856d" - ~file:"a.java" ~key:"1" ~hash:"4" () ] - , (["1"], ["2"], ["3"]) ) - ; ( "test_skip_anonymous_class_renamings_with_matching_non_anonymous_procedure_ids" - , Differential.of_reports - ~current_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class.foo():bool.919f37fd0993058a01f438210ba8a247" - ~file:"a.java" ~key:"1" ~hash:"1" () - ; create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class.foo():bool.919f37fd0993058a01f438210ba8a247" - ~file:"a.java" ~key:"1" ~hash:"3" () ] - ~previous_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class.foo():bool.919f37fd0993058a01f438210ba8a247" - ~file:"a.java" ~key:"1" ~hash:"2" () - ; create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class.foo():bool.919f37fd0993058a01f438210ba8a247" - ~file:"a.java" ~key:"1" ~hash:"4" () ] - , (["1"; "3"], ["2"; "4"], []) ) - ; ( "test_skip_anonymous_class_renamings_with_non_java_files" - , Differential.of_reports - ~current_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class$3$1.foo():bool.9ff39eb5c53c81da9f6a7ade324345b6" - ~file:"a.java" ~key:"1" ~hash:"1" () - ; create_fake_jsonbug ~bug_type:"bug_type_2" - ~procedure_id: - "com.whatever.package.Class$1.foo():bool.bf13089cf4c47ff8ff089a1a4767324f" - ~file:"a.mm" ~key:"1" ~hash:"3" () ] - ~previous_report: - [ create_fake_jsonbug ~bug_type:"bug_type_1" - ~procedure_id: - "com.whatever.package.Class$21$1.foo():bool.db89561ad9dab28587c8c04833f09b03" - ~file:"a.java" ~key:"1" ~hash:"2" () - ; create_fake_jsonbug ~bug_type:"bug_type_2" - ~procedure_id: - "com.whatever.package.Class$8.foo():bool.cffd4e941668063eb802183dbd3e856d" - ~file:"a.mm" ~key:"1" ~hash:"4" () ] - , (["3"], ["4"], ["1"]) ) ] - |> List.map ~f:(fun (name, diff, expected_output) -> name >:: create_test diff expected_output) - - let test_interesting_paths_filter = let report = [ create_fake_jsonbug ~bug_type:"bug_type_1" ~file:"file_1.java" ~hash:"1" () @@ -339,5 +224,5 @@ let test_interesting_paths_filter = let tests = "differential_filters_suite" >::: test_file_renamings_from_json @ test_file_renamings_find_previous - @ test_relative_complements @ test_skip_anonymous_class_renamings - @ test_interesting_paths_filter @ [test_skip_duplicated_types_on_filenames] + @ test_relative_complements @ test_interesting_paths_filter + @ [test_skip_duplicated_types_on_filenames] diff --git a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/Makefile b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/Makefile index 5e88e3f3f..7405af23c 100644 --- a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/Makefile +++ b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/Makefile @@ -3,7 +3,7 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -# E2E test involving the skip_anonymous_class_renamings filter +# E2E test for anonymous class renaming TESTS_DIR = ../.. SOURCES = src/DiffExample.java.current src/DiffExample.java.previous diff --git a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.current b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.current index 803418fde..2235989e8 100644 --- a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.current +++ b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.current @@ -5,7 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -// This example tests the skip_anonymous_class_renamings filter +// This example tests that anonymous class renaming does not affect the +// computation of the pre-existing warnings class DiffExample { private int checkAnonymousClasses() { SimpleInterfaceExample sie1 = new SimpleInterfaceExample() { diff --git a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.previous b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.previous index 19b667812..00fc8a4e7 100644 --- a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.previous +++ b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/DiffExample.java.previous @@ -5,7 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -// This example tests the skip_anonymous_class_renamings filter +// This example tests that anonymous class renaming does not affect the +// computation of the pre-existing warnings class DiffExample { private int checkAnonymousClasses() { SimpleInterfaceExample sie1 = new SimpleInterfaceExample() { diff --git a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleInterfaceExample.java b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleInterfaceExample.java index 76e04566b..4f2e38ae6 100644 --- a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleInterfaceExample.java +++ b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleInterfaceExample.java @@ -5,7 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -// This example tests the skip_anonymous_class_renamings filter +// This example tests that anonymous class renaming does not affect the +// computation of the pre-existing warnings public interface SimpleInterfaceExample { public String getString(); } diff --git a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleNestedInterface.java b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleNestedInterface.java index 7e314953a..7e105f26d 100644 --- a/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleNestedInterface.java +++ b/infer/tests/build_systems/differential_skip_anonymous_class_renamings/src/SimpleNestedInterface.java @@ -5,7 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -// This example tests the skip_anonymous_class_renamings filter +// This example tests that anonymous class renaming does not affect the +// computation of the pre-existing warnings public interface SimpleNestedInterface { public void doSomething(); }