[infer] remove ad hoc treatment of anonymous class renaming

Summary: This code is no longer necessary because the bug hash does not depend on the name of the anonymous classes

Reviewed By: mbouaziz

Differential Revision: D9176205

fbshipit-source-id: 9a8e9c9f8
master
Jeremy Dubreil 7 years ago committed by Facebook Github Bot
parent 974e134061
commit d5a5e7da10

@ -148,74 +148,10 @@ let skip_duplicated_types_on_filenames renamings (diff: Differential.t) : Differ
{introduced; fixed; preexisting; costs_summary= diff.costs_summary} {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 file_extension = string [@@deriving compare]
type weak_hash = string * string * string * Caml.Digest.t [@@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 *) (* Strip issues whose paths are not among those we're interested in *)
let interesting_paths_filter (interesting_paths: SourceFile.t list option) = let interesting_paths_filter (interesting_paths: SourceFile.t list option) =
match interesting_paths with match interesting_paths with
@ -245,9 +181,7 @@ let do_filter (diff: Differential.t) (renamings: FileRenamings.t) ~(skip_duplica
else issues else issues
in in
let diff' = 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 diff else diff
|> (if skip_duplicated_types then skip_duplicated_types_on_filenames renamings else Fn.id)
|> Fn.id
in in
{ introduced= apply_paths_filter_if_needed `Introduced diff'.introduced { introduced= apply_paths_filter_if_needed `Introduced diff'.introduced
; fixed= apply_paths_filter_if_needed `Fixed diff'.fixed ; 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_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 let interesting_paths_filter = interesting_paths_filter
end end

@ -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_duplicated_types_on_filenames : FileRenamings.t -> Differential.t -> Differential.t
val skip_anonymous_class_renamings : Differential.t -> Differential.t
val interesting_paths_filter : val interesting_paths_filter :
SourceFile.t list option -> Jsonbug_t.jsonbug list -> Jsonbug_t.jsonbug list SourceFile.t list option -> Jsonbug_t.jsonbug list -> Jsonbug_t.jsonbug list
end end

@ -188,121 +188,6 @@ let test_skip_duplicated_types_on_filenames =
"test_skip_duplicated_types_on_filenames" >:: do_assert "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 test_interesting_paths_filter =
let report = let report =
[ create_fake_jsonbug ~bug_type:"bug_type_1" ~file:"file_1.java" ~hash:"1" () [ 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 = let tests =
"differential_filters_suite" "differential_filters_suite"
>::: test_file_renamings_from_json @ test_file_renamings_find_previous >::: test_file_renamings_from_json @ test_file_renamings_find_previous
@ test_relative_complements @ test_skip_anonymous_class_renamings @ test_relative_complements @ test_interesting_paths_filter
@ test_interesting_paths_filter @ [test_skip_duplicated_types_on_filenames] @ [test_skip_duplicated_types_on_filenames]

@ -3,7 +3,7 @@
# This source code is licensed under the MIT license found in the # This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree. # 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 = ../.. TESTS_DIR = ../..
SOURCES = src/DiffExample.java.current src/DiffExample.java.previous SOURCES = src/DiffExample.java.current src/DiffExample.java.previous

@ -5,7 +5,8 @@
* LICENSE file in the root directory of this source tree. * 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 { class DiffExample {
private int checkAnonymousClasses() { private int checkAnonymousClasses() {
SimpleInterfaceExample sie1 = new SimpleInterfaceExample() { SimpleInterfaceExample sie1 = new SimpleInterfaceExample() {

@ -5,7 +5,8 @@
* LICENSE file in the root directory of this source tree. * 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 { class DiffExample {
private int checkAnonymousClasses() { private int checkAnonymousClasses() {
SimpleInterfaceExample sie1 = new SimpleInterfaceExample() { SimpleInterfaceExample sie1 = new SimpleInterfaceExample() {

@ -5,7 +5,8 @@
* LICENSE file in the root directory of this source tree. * 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 interface SimpleInterfaceExample {
public String getString(); public String getString();
} }

@ -5,7 +5,8 @@
* LICENSE file in the root directory of this source tree. * 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 interface SimpleNestedInterface {
public void doSomething(); public void doSomething();
} }

Loading…
Cancel
Save