From b02d8e9b22b894301cc1e853eacedf11de797f18 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Tue, 26 May 2020 03:46:36 -0700 Subject: [PATCH] [UI] use human-readable issue type in console reporting Summary: Problem: issue types can be reported by several checkers. The current solution is to change the name of the issue, eg `BIABD_USE_AFTER_FREE`. This doesn't look great for the user. We already store a "human readable" name for each issue. These need not be unique. Use this instead in textual output. In order to keep the link between the text output and the true "issue type", eg to pass to `--disable-issue-type`, also print the `unique_id` part of the issue in the summary: ``` examples/hello.c:12: error: Null Dereference pointer `s` last assigned on line 11 could be null and is dereferenced at line 12, column 3. 10. void test() { 11. int* s = NULL; 12. *s = 42; ^ 13. } Found 1 issue Issue Type(ISSUED_TYPE_ID): # Null Dereference(NULL_DEREFERENCE): 1 ``` We could also print the issue id in each report but that looks worse. Other tools use numbers for issue ids, but these are not descriptive at all, eg `--disable-issue-type 86` is not very telling. Reviewed By: skcho Differential Revision: D21663957 fbshipit-source-id: 506b0fda9 --- infer/src/base/IssueType.ml | 24 +++++++++++------ infer/src/integration/TextReport.ml | 40 ++++++++++++++++++----------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 33c6ab91b..427256cf1 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -61,7 +61,7 @@ end = struct let to_rank {unique_id} = unique_id - let pp fmt t = F.pp_print_string fmt t.unique_id + let pp fmt t = F.pp_print_string fmt t.hum end include T @@ -147,19 +147,27 @@ let assert_failure = register_from_string ~id:"Assert_failure" Biabduction let bad_footprint = register_from_string ~id:"Bad_footprint" Biabduction let biabd_condition_always_false = - register_from_string ~enabled:false ~id:"BIABD_CONDITION_ALWAYS_FALSE" Biabduction + register_from_string ~enabled:false ~hum:"Condition Always False" + ~id:"BIABD_CONDITION_ALWAYS_FALSE" Biabduction let biabd_condition_always_true = - register_from_string ~enabled:false ~id:"BIABD_CONDITION_ALWAYS_TRUE" Biabduction + register_from_string ~enabled:false ~hum:"Condition Always True" ~id:"BIABD_CONDITION_ALWAYS_TRUE" + Biabduction let biabd_registered_observer_being_deallocated = - register_from_string ~id:"BIABD_REGISTERED_OBSERVER_BEING_DEALLOCATED" Biabduction + register_from_string ~hum:"Registered Observer Being Deallocated" + ~id:"BIABD_REGISTERED_OBSERVER_BEING_DEALLOCATED" Biabduction let biabd_stack_variable_address_escape = - register_from_string ~enabled:false ~id:"BIABD_STACK_VARIABLE_ADDRESS_ESCAPE" Biabduction + register_from_string ~enabled:false ~hum:"Stack Variable Address Escape" + ~id:"BIABD_STACK_VARIABLE_ADDRESS_ESCAPE" Biabduction + + +let biabd_use_after_free = + register_from_string ~hum:"Use After Free" ~id:"BIABD_USE_AFTER_FREE" Biabduction let buffer_overrun_l1 = register_from_string ~id:"BUFFER_OVERRUN_L1" BufferOverrunChecker @@ -520,7 +528,9 @@ let pulse_memory_leak = register_from_string ~enabled:false ~id:"PULSE_MEMORY_LE let pure_function = register_from_string ~id:"PURE_FUNCTION" Purity -let quandary_taint_error = register_from_string ~id:"QUANDARY_TAINT_ERROR" Quandary +let quandary_taint_error = + register_from_string ~hum:"Taint Error" ~id:"QUANDARY_TAINT_ERROR" Quandary + let resource_leak = register_from_string ~id:"RESOURCE_LEAK" Biabduction @@ -580,8 +590,6 @@ let use_after_delete = register_from_string ~id:"USE_AFTER_DELETE" Pulse let use_after_free = register_from_string ~id:"USE_AFTER_FREE" Pulse -let biabd_use_after_free = register_from_string ~id:"BIABD_USE_AFTER_FREE" Biabduction - let use_after_lifetime = register_from_string ~id:"USE_AFTER_LIFETIME" Pulse let user_controlled_sql_risk = register_from_string ~id:"USER_CONTROLLED_SQL_RISK" Quandary diff --git a/infer/src/integration/TextReport.ml b/infer/src/integration/TextReport.ml index 9383d282d..d901d39f0 100644 --- a/infer/src/integration/TextReport.ml +++ b/infer/src/integration/TextReport.ml @@ -21,44 +21,54 @@ let pp_n_spaces n fmt = module ReportSummary = struct - type t = {mutable n_issues: int; issue_type_counts: int IssueHash.t} + type t = {mutable n_issues: int; issue_type_counts: (int * string) IssueHash.t} let mk_empty () = {n_issues= 0; issue_type_counts= IssueHash.create 64} let pp fmt {n_issues= _; issue_type_counts} = + let string_of_issue ~issue_type ~issue_type_hum = + Printf.sprintf "%s(%s)" issue_type_hum issue_type + in + (* to document the format of the output to the user *) + let header_issue = string_of_issue ~issue_type:"ISSUED_TYPE_ID" ~issue_type_hum:"Issue Type" in let max_issue_length, issue_counts = IssueHash.to_seq issue_type_counts |> Seq.fold_left - (fun (max_issue_length, issue_counts) ((issue_type, _) as issue_with_count) -> - let l = String.length issue_type in - (Int.max max_issue_length l, issue_with_count :: issue_counts) ) - (0, []) + (fun (max_issue_length, issue_counts) ((issue_type, (_, issue_type_hum)) as binding) -> + let l = String.length (string_of_issue ~issue_type ~issue_type_hum) in + (Int.max max_issue_length l, binding :: issue_counts) ) + (String.length header_issue, []) in - List.sort issue_counts ~compare:(fun (issue_type1, count1) (issue_type2, count2) -> + pp_n_spaces (max_issue_length - String.length header_issue) fmt ; + F.fprintf fmt " %s: #@\n" header_issue ; + List.sort issue_counts ~compare:(fun (issue_type1, (count1, _)) (issue_type2, (count2, _)) -> (* reverse lexicographic order on (count * issue_type) *) if Int.equal count2 count1 then String.compare issue_type2 issue_type1 else Int.compare count2 count1 ) - |> List.iter ~f:(fun (bug_type, count) -> - pp_n_spaces (max_issue_length - String.length bug_type) fmt ; - F.fprintf fmt " %s: %d@\n" bug_type count ) + |> List.iter ~f:(fun (issue_type, (count, issue_type_hum)) -> + let issue_string = string_of_issue ~issue_type ~issue_type_hum in + pp_n_spaces (max_issue_length - String.length issue_string) fmt ; + F.fprintf fmt " %s: %d@\n" issue_string count ) let add_issue summary (jsonbug : Jsonbug_t.jsonbug) = let bug_count = - IssueHash.find_opt summary.issue_type_counts jsonbug.bug_type |> Option.value ~default:0 + IssueHash.find_opt summary.issue_type_counts jsonbug.bug_type + |> Option.value_map ~default:0 ~f:fst in - IssueHash.replace summary.issue_type_counts jsonbug.bug_type (bug_count + 1) ; + IssueHash.replace summary.issue_type_counts jsonbug.bug_type + (bug_count + 1, jsonbug.bug_type_hum) ; summary.n_issues <- summary.n_issues + 1 ; (* chain for convenience/pretending it's a functional data structure *) summary end -let pp_jsonbug fmt {Jsonbug_t.file; severity; line; bug_type; qualifier; _} = - F.fprintf fmt "%s:%d: %s: %s@\n %s" file line (String.lowercase severity) bug_type qualifier +let pp_jsonbug fmt {Jsonbug_t.file; severity; line; bug_type_hum; qualifier; _} = + F.fprintf fmt "%s:%d: %s: %s@\n %s" file line (String.lowercase severity) bug_type_hum qualifier -let pp_jsonbug_with_number fmt (i, {Jsonbug_t.file; severity; line; bug_type; qualifier; _}) = - F.fprintf fmt "#%d@\n%s:%d: %s: %s@\n %s" i file line (String.lowercase severity) bug_type +let pp_jsonbug_with_number fmt (i, {Jsonbug_t.file; severity; line; bug_type_hum; qualifier; _}) = + F.fprintf fmt "#%d@\n%s:%d: %s: %s@\n %s" i file line (String.lowercase severity) bug_type_hum qualifier