From a4078b43eec9d773d5c120b7ed770bc853da2dd0 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 8 Feb 2018 09:09:34 -0800 Subject: [PATCH] [cleanup] Remove Tags from JSON Summary: There's a lot of code for building up and moving around `Tags`. When working on cleaning up some of the `Errlog` code, I noticed that `Tags` were included in the JSON and wondered why. The answer is suprisingly just one thing: only the line tags get used, and even then they are only used to decide what frame to select as the start frame for the trace (i.e., the one that is highlighted first). That seems like overkill; starting on trace on the actual line where the error occurs, starting at the beginning of the procedure where the error occurs, or starting at the first line of the trace all seem equally reasonable. If we are happy with any of these alternatives, we can kill `Tags` altogether and potentially save a decent amount space in our JSON artifacts. Reviewed By: mbouaziz Differential Revision: D6876752 fbshipit-source-id: 1580127 --- infer/src/IR/Io_infer.ml | 4 -- infer/src/IR/Io_infer.mli | 4 -- infer/src/IR/Localise.ml | 18 --------- infer/src/IR/Localise.mli | 8 ---- infer/src/atd/jsonbug.atd | 7 ---- infer/src/backend/DifferentialFilters.ml | 28 ++----------- infer/src/backend/DifferentialFilters.mli | 2 - infer/src/backend/InferPrint.ml | 26 +----------- infer/src/backend/InferPrint.mli | 2 - infer/src/unit/DifferentialFiltersTests.ml | 46 +++------------------- infer/src/unit/DifferentialTestsUtils.ml | 3 +- 11 files changed, 10 insertions(+), 138 deletions(-) diff --git a/infer/src/IR/Io_infer.ml b/infer/src/IR/Io_infer.ml index f54c7b6c5..bd5aea49b 100644 --- a/infer/src/IR/Io_infer.ml +++ b/infer/src/IR/Io_infer.ml @@ -210,16 +210,12 @@ end (** Create and print xml trees *) module Xml = struct - let tag_branch = "branch" - let tag_err = "err" let tag_file = "file" let tag_in_calls = "in_calls" - let tag_kind = "kind" - let tag_line = "line" let tag_loc = "loc" diff --git a/infer/src/IR/Io_infer.mli b/infer/src/IR/Io_infer.mli index dceeeffa2..868cc9075 100644 --- a/infer/src/IR/Io_infer.mli +++ b/infer/src/IR/Io_infer.mli @@ -62,16 +62,12 @@ end (** Create and print xml trees *) module Xml : sig - val tag_branch : string - val tag_err : string val tag_file : string val tag_in_calls : string - val tag_kind : string - val tag_line : string val tag_loc : string diff --git a/infer/src/IR/Localise.ml b/infer/src/IR/Localise.ml index ac672c582..218326940 100644 --- a/infer/src/IR/Localise.ml +++ b/infer/src/IR/Localise.ml @@ -98,24 +98,6 @@ module Tags = struct let update tags tag value = tags := add !tags tag value let get tags tag = List.Assoc.find ~equal:String.equal tags tag - - let tag_value_records_of_tags tags = - List.map ~f:(fun (tag, value) -> {Jsonbug_t.tag; value}) tags - - - let tags_of_tag_value_records (tag_value_records: Jsonbug_t.tag_value_record list) = - List.map ~f:(fun {Jsonbug_t.tag; value} -> (tag, value)) tag_value_records - - - let lines_of_tags (tags: t) = - let line_tags = - String.Set.of_list - [dereferenced_line; call_line; assigned_line; alloc_line; accessed_line; dealloc_line] - in - List.filter_map - ~f:(fun (tag, value) -> - if String.Set.mem line_tags tag then Some (int_of_string value) else None ) - tags end type error_desc = diff --git a/infer/src/IR/Localise.mli b/infer/src/IR/Localise.mli index 0422c93a6..08f0e127d 100644 --- a/infer/src/IR/Localise.mli +++ b/infer/src/IR/Localise.mli @@ -15,14 +15,6 @@ open! IStd module Tags : sig type t - val tag_value_records_of_tags : t -> Jsonbug_t.tag_value_record list - (** convert error description's tags to atd-serializable format *) - - val tags_of_tag_value_records : Jsonbug_t.tag_value_record list -> t - (** convert atd-serializable format to error description's tags *) - - val lines_of_tags : t -> int list - (** collect all lines from tags *) end (** description field of error messages *) diff --git a/infer/src/atd/jsonbug.atd b/infer/src/atd/jsonbug.atd index 7da3026ba..a5ae04b71 100644 --- a/infer/src/atd/jsonbug.atd +++ b/infer/src/atd/jsonbug.atd @@ -1,15 +1,9 @@ -type tag_value_record = { - tag : string; - value : string; -} - type json_trace_item = { level : int; filename : string; line_number : int; column_number : int; description : string; - node_tags : tag_value_record list; } type loc = { @@ -35,7 +29,6 @@ type jsonbug = { file : string; bug_trace : json_trace_item list; key : string; - qualifier_tags : tag_value_record list; hash : string; ?dotty : string option; ?infer_source_loc: loc option; diff --git a/infer/src/backend/DifferentialFilters.ml b/infer/src/backend/DifferentialFilters.ml index 509c719d2..014b63669 100644 --- a/infer/src/backend/DifferentialFilters.ml +++ b/infer/src/backend/DifferentialFilters.ml @@ -170,17 +170,9 @@ let compare_procedure_id pid1 pid2 = String.compare pid1_norm_trimmed pid2_norm_trimmed -let value_of_qualifier_tag qts tag = - match List.find ~f:(fun elem -> String.equal elem.Jsonbug_t.tag tag) qts with - | Some qt -> - Some qt.Jsonbug_t.value - | None -> - None - - type file_extension = string [@@deriving compare] -type weak_hash = string * string * string * Caml.Digest.t * string option [@@deriving compare] +type weak_hash = string * string * string * Caml.Digest.t [@@deriving compare] let skip_anonymous_class_renamings (diff: Differential.t) : Differential.t = (* @@ -195,20 +187,8 @@ let skip_anonymous_class_renamings (diff: Differential.t) : Differential.t = let extension fname = snd (Filename.split_extension fname) in let cmp (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 - , value_of_qualifier_tag i1.qualifier_tags "call_procedure" ) - , string_of_procedure_id i1 ) - ( extension i2.file - , ( i2.kind - , i2.bug_type - , i2.file - , i2.key - , value_of_qualifier_tag i2.qualifier_tags "call_procedure" ) - , string_of_procedure_id i2 ) + (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 () = @@ -275,8 +255,6 @@ module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY = struct let skip_duplicated_types_on_filenames = skip_duplicated_types_on_filenames - let value_of_qualifier_tag = value_of_qualifier_tag - let skip_anonymous_class_renamings = skip_anonymous_class_renamings let interesting_paths_filter = interesting_paths_filter diff --git a/infer/src/backend/DifferentialFilters.mli b/infer/src/backend/DifferentialFilters.mli index 0a324a1ed..2fdc48ecc 100644 --- a/infer/src/backend/DifferentialFilters.mli +++ b/infer/src/backend/DifferentialFilters.mli @@ -42,8 +42,6 @@ module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY : sig val skip_duplicated_types_on_filenames : FileRenamings.t -> Differential.t -> Differential.t - val value_of_qualifier_tag : Jsonbug_t.tag_value_record list -> string -> string option - val skip_anonymous_class_renamings : Differential.t -> Differential.t val interesting_paths_filter : diff --git a/infer/src/backend/InferPrint.ml b/infer/src/backend/InferPrint.ml index 9441e7296..a498f0bf5 100644 --- a/infer/src/backend/InferPrint.ml +++ b/infer/src/backend/InferPrint.ml @@ -59,40 +59,17 @@ let compute_hash (kind: string) (type_str: string) (proc_name: Typ.Procname.t) ( |> Caml.Digest.to_hex -let exception_value = "exception" - let loc_trace_to_jsonbug_record trace_list ekind = match ekind with | Exceptions.Kinfo -> [] | _ -> - let tag_value_records_of_node_tag nt = - match nt with - | Errlog.Condition cond -> - [ {Jsonbug_j.tag= Io_infer.Xml.tag_kind; value= "condition"} - ; {Jsonbug_j.tag= Io_infer.Xml.tag_branch; value= Printf.sprintf "%B" cond} ] - | Errlog.Exception exn_name -> - let res = [{Jsonbug_j.tag= Io_infer.Xml.tag_kind; value= exception_value}] in - let exn_str = Typ.Name.name exn_name in - if String.is_empty exn_str then res - else {Jsonbug_j.tag= Io_infer.Xml.tag_name; value= exn_str} :: res - | Errlog.Procedure_start pname -> - [ {Jsonbug_j.tag= Io_infer.Xml.tag_kind; value= "procedure_start"} - ; {Jsonbug_j.tag= Io_infer.Xml.tag_name; value= Typ.Procname.to_string pname} - ; {Jsonbug_j.tag= Io_infer.Xml.tag_name_id; value= Typ.Procname.to_filename pname} ] - | Errlog.Procedure_end pname -> - [ {Jsonbug_j.tag= Io_infer.Xml.tag_kind; value= "procedure_end"} - ; {Jsonbug_j.tag= Io_infer.Xml.tag_name; value= Typ.Procname.to_string pname} - ; {Jsonbug_j.tag= Io_infer.Xml.tag_name_id; value= Typ.Procname.to_filename pname} ] - in let trace_item_to_record trace_item = { Jsonbug_j.level= trace_item.Errlog.lt_level ; filename= SourceFile.to_string trace_item.Errlog.lt_loc.Location.file ; line_number= trace_item.Errlog.lt_loc.Location.line ; column_number= trace_item.Errlog.lt_loc.Location.col - ; description= trace_item.Errlog.lt_description - ; node_tags= - List.concat_map ~f:tag_value_records_of_node_tag trace_item.Errlog.lt_node_tags } + ; description= trace_item.Errlog.lt_description } in let record_list = List.rev (List.rev_map ~f:trace_item_to_record trace_list) in record_list @@ -290,7 +267,6 @@ module IssuesJson = struct ; file ; bug_trace= loc_trace_to_jsonbug_record err_data.loc_trace key.err_kind ; key= err_data.node_id_key.node_key |> Caml.Digest.to_hex - ; qualifier_tags= Localise.Tags.tag_value_records_of_tags key.err_desc.tags ; hash= compute_hash kind bug_type procname file qualifier ; dotty= error_desc_to_dotty_string key.err_desc ; infer_source_loc= json_ml_loc diff --git a/infer/src/backend/InferPrint.mli b/infer/src/backend/InferPrint.mli index b74981de6..a91a0114c 100644 --- a/infer/src/backend/InferPrint.mli +++ b/infer/src/backend/InferPrint.mli @@ -9,6 +9,4 @@ open! IStd -val exception_value : string - val main : report_json:string option -> unit diff --git a/infer/src/unit/DifferentialFiltersTests.ml b/infer/src/unit/DifferentialFiltersTests.ml index 8676d6fdb..722424b68 100644 --- a/infer/src/unit/DifferentialFiltersTests.ml +++ b/infer/src/unit/DifferentialFiltersTests.ml @@ -190,29 +190,7 @@ let test_skip_duplicated_types_on_filenames = "test_skip_duplicated_types_on_filenames" >:: do_assert -let test_value_of_qualifier_tag = - let qts = [{Jsonbug_t.tag= "tag1"; value= "value1"}; {Jsonbug_t.tag= "tag2"; value= "value2"}] in - let pp_diff fmt (expected, actual) = - let to_str v = Option.value v ~default:"NONE" in - Format.fprintf fmt "Expected: %s Found: %s" (to_str expected) (to_str actual) - in - let do_assert _ = - assert_equal ~cmp:(Option.equal String.equal) ~pp_diff (Some "value2") - (DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.value_of_qualifier_tag qts - "tag2") ; - assert_equal ~cmp:(Option.equal String.equal) ~pp_diff None - (DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.value_of_qualifier_tag qts - "tag3") ; - assert_equal ~cmp:(Option.equal String.equal) ~pp_diff (Some "value1") - (DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.value_of_qualifier_tag qts - "tag1") - in - "test_value_of_qualifier_tag" >:: do_assert - - let test_skip_anonymous_class_renamings = - let qt1 = [{Jsonbug_t.tag= "call_procedure"; value= "aValue1"}] in - let qt2 = [{Jsonbug_t.tag= "call_procedure"; value= "aValue2"}] in 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 @@ -240,13 +218,13 @@ let test_skip_anonymous_class_renamings = ( "com.whatever.package00.abcd." ^ "ABasicExampleFragment$83.onMenuItemActionExpand(android.view.MenuItem):b." ^ "5ab5e18cae498c35d887ce88f3d5fa82" ) - ~file:"a.java" ~key:"1" ~qualifier_tags:qt1 ~hash:"3" () + ~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" ~qualifier_tags:qt1 ~hash:"4" () + ~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" () ] @@ -256,7 +234,7 @@ let test_skip_anonymous_class_renamings = ( "com.whatever.package00.abcd." ^ "ABasicExampleFragment$9.onMenuItemActionExpand(android.view.MenuItem):bo." ^ "ba1776155fba2899542401da5bc779a5" ) - ~file:"a.java" ~key:"1" ~qualifier_tags:qt1 ~hash:"1" () + ~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" () ] @@ -323,20 +301,7 @@ let test_skip_anonymous_class_renamings = ~procedure_id: "com.whatever.package.Class$8.foo():bool.cffd4e941668063eb802183dbd3e856d" ~file:"a.mm" ~key:"1" ~hash:"4" () ] - , (["3"], ["4"], ["1"]) ) - ; ( "test_skip_anonymous_class_renamings_with_different_call_procedure_qualifier_tags" - , 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" ~qualifier_tags:qt1 ~hash:"1" () ] - ~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" ~qualifier_tags:qt2 ~hash:"2" () ] - , (["1"], ["2"], []) ) ] + , (["3"], ["4"], ["1"]) ) ] |> List.map ~f:(fun (name, diff, expected_output) -> name >:: create_test diff expected_output) @@ -377,5 +342,4 @@ 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_value_of_qualifier_tag] + @ test_interesting_paths_filter @ [test_skip_duplicated_types_on_filenames] diff --git a/infer/src/unit/DifferentialTestsUtils.ml b/infer/src/unit/DifferentialTestsUtils.ml index a11429b28..3a2b388ba 100644 --- a/infer/src/unit/DifferentialTestsUtils.ml +++ b/infer/src/unit/DifferentialTestsUtils.ml @@ -13,7 +13,7 @@ let create_fake_jsonbug ?(bug_class= "bug_class") ?(kind= "kind") ?(bug_type= "b ?(qualifier= "qualifier") ?(severity= "severity") ?(visibility= "visibility") ?(line= 1) ?(column= 1) ?(procedure= "procedure") ?(procedure_id= "procedure_id") ?(procedure_start_line= 1) ?(file= "file/at/a/certain/path.java") ?(bug_trace= []) - ?(key= "1234") ?(qualifier_tags= []) ?(hash= "1") ?(dotty= None) ?(infer_source_loc= None) + ?(key= "1234") ?(hash= "1") ?(dotty= None) ?(infer_source_loc= None) ?(linters_def_file= Some "file/at/certain/path.al") ?doc_url () : Jsonbug_t.jsonbug = { bug_class ; kind @@ -29,7 +29,6 @@ let create_fake_jsonbug ?(bug_class= "bug_class") ?(kind= "kind") ?(bug_type= "b ; file ; bug_trace ; key - ; qualifier_tags ; hash ; dotty ; infer_source_loc