[Infer] Emit potential exception lines into qualifier's message

Summary: Sometimes reports need traces to be fully understood, but sometimes reporting where the exception takes place can save time to developers.

Reviewed By: jvillard

Differential Revision: D4914037

fbshipit-source-id: 039ab63

Makefile

@@ -64,6 +64,7 @@ BUILD_SYSTEMS_TESTS += \
   differential_skip_duplicated_types_on_filenames_with_renamings \
   gradle \
   javac \
+  resource_leak_exception_lines \
 
 DIRECT_TESTS += \
   java_checkers java_eradicate java_infer java_tracing java_quandary java_threadsafety \

infer/src/IR/Errlog.ml

@@ -27,12 +27,33 @@ type loc_trace_elem = {
   lt_node_tags : node_tag list (** tags describing the node at the current location *)
 }
 
+let contains_exception loc_trace_elem =
+  let pred nt =
+    match nt with
+    | Exception _ -> true
+    | Condition _ | Procedure_start _ | Procedure_end _ -> false in
+  List.exists ~f:pred loc_trace_elem.lt_node_tags
+
 let make_trace_element lt_level lt_loc lt_description lt_node_tags =
   { lt_level; lt_loc; lt_description; lt_node_tags }
 
 (** Trace of locations *)
 type loc_trace = loc_trace_elem list
 
+let compute_local_exception_line loc_trace =
+  let compute_local_exception_line state step =
+    match state with
+    | `Stop _ -> state
+    | `Continue (last_known_step_at_level_zero_opt, line_opt) ->
+        let last_known_step_at_level_zero_opt' =
+          if Int.equal step.lt_level 0 then Some step
+          else last_known_step_at_level_zero_opt in
+        match last_known_step_at_level_zero_opt' with
+        | Some step_zero when contains_exception step ->
+            `Stop (last_known_step_at_level_zero_opt', Some step_zero.lt_loc.line)
+        | _ -> `Continue (last_known_step_at_level_zero_opt', line_opt) in
+  snd (List_.fold_until ~init:(`Continue (None, None)) ~f:compute_local_exception_line loc_trace)
+
 type node_id_key = {
   node_id : int;
   node_key : int

infer/src/IR/Errlog.mli

@@ -31,6 +31,12 @@ val make_trace_element : int -> Location.t -> string -> node_tag list -> loc_tra
 (** Trace of locations *)
 type loc_trace = loc_trace_elem list
 
+(** Look at all the trace steps and find those that are arising any exception,
+    then bind them to the closest step at level 0.
+    This extra information adds value to the report itself, and may avoid
+    digging into the trace to understand the cause of the report. *)
+val compute_local_exception_line : loc_trace -> int option
+
 type node_id_key = private {
   node_id : int;
   node_key : int

infer/src/backend/InferPrint.re

@@ -429,6 +429,8 @@ module IssuesCsv = {
   };
 };
 
+let potential_exception_message = "potential exception at line";
+
 module IssuesJson = {
   let is_first_item = ref true;
   let pp_json_open fmt () => F.fprintf fmt "[@?";
@@ -464,11 +466,26 @@ module IssuesJson = {
           | _ => None
           };
         let visibility = Exceptions.string_of_visibility err_data.visibility;
+        let qualifier = {
+          let base_qualifier = error_desc_to_plain_string key.err_desc;
+          if (Localise.equal key.err_name Localise.resource_leak) {
+            switch (Errlog.compute_local_exception_line err_data.loc_trace) {
+            | None => base_qualifier
+            | Some line =>
+              let potential_exception_message =
+                Format.asprintf
+                  "%a: %s %d" MarkupFormatter.pp_bold "Note" potential_exception_message line;
+              Format.sprintf "%s\n%s" base_qualifier potential_exception_message
+            }
+          } else {
+            base_qualifier
+          }
+        };
         let bug = {
           Jsonbug_j.bug_class: Exceptions.err_class_string err_data.err_class,
           kind,
           bug_type,
-          qualifier: error_desc_to_plain_string key.err_desc,
+          qualifier,
           severity: key.severity,
           visibility,
           line: err_data.loc.Location.line,
@@ -538,6 +555,9 @@ let pp_custom_of_report fmt report fields => {
         Format.fprintf fmt "%s%d" (comma_separator index) (issue.line - issue.procedure_start_line)
       | `Issue_field_procedure_id_without_crc =>
         Format.fprintf fmt "%s%s" (comma_separator index) (DB.strip_crc issue.procedure_id)
+      | `Issue_field_qualifier_contains_potential_exception_note =>
+        Format.fprintf
+          fmt "%B" (String.is_substring issue.qualifier substring::potential_exception_message)
       };
     List.iteri f::pp_field fields;
     Format.fprintf fmt "@."

infer/src/base/Config.ml

@@ -94,6 +94,8 @@ let issues_fields_symbols = [
   ("hash", `Issue_field_hash);
   ("line_offset", `Issue_field_line_offset);
   ("procedure_id_without_crc", `Issue_field_procedure_id_without_crc);
+  ("qualifier_contains_potential_exception_note",
+   `Issue_field_qualifier_contains_potential_exception_note);
 ]
 
 type os_type = Unix | Win32 | Cygwin

infer/src/base/Config.mli

@@ -58,7 +58,8 @@ val issues_fields_symbols :
             | `Issue_field_key
             | `Issue_field_hash
             | `Issue_field_line_offset
-            | `Issue_field_procedure_id_without_crc]) list
+            | `Issue_field_procedure_id_without_crc
+            | `Issue_field_qualifier_contains_potential_exception_note]) list
 
 
 type os_type = Unix | Win32 | Cygwin
@@ -262,7 +263,8 @@ val issues_fields : [`Issue_field_bug_class
                     | `Issue_field_key
                     | `Issue_field_hash
                     | `Issue_field_line_offset
-                    | `Issue_field_procedure_id_without_crc] list
+                    | `Issue_field_procedure_id_without_crc
+                    | `Issue_field_qualifier_contains_potential_exception_note] list
 val iterations : int
 val java_jar_compiler : string option
 val javac_classes_out : string

infer/src/base/IStd.ml

@@ -46,6 +46,14 @@ module Unix_ = struct
 
 end
 
+module List_ = struct
+  let rec fold_until ~init ~f l =
+    match l, init with
+    | _, `Stop init'
+    | [], `Continue init' -> init'
+    | h :: t, `Continue _ -> fold_until ~init:(f init h) ~f t
+end
+
 (* Use Caml.Set since they are serialized using Marshal, and Core.Std.Set includes the comparison
    function in its representation, which Marshal cannot (de)serialize. *)
 module IntSet = Caml.Set.Make(Int)

infer/src/base/MarkupFormatter.ml

@@ -17,6 +17,10 @@ type 'a formatter = {
   wrap_code : (Format.formatter -> 'a -> unit) -> Format.formatter -> 'a -> unit;
   pp_code : Format.formatter -> string -> unit;
   code_to_string : string -> string;
+
+  wrap_bold : (Format.formatter -> 'a -> unit) -> Format.formatter -> 'a -> unit;
+  pp_bold : Format.formatter -> string -> unit;
+  bold_to_string : string -> string;
 }
 
 module NoFormatter : sig
@@ -30,7 +34,10 @@ end = struct
     monospaced_to_string = Fn.id;
     wrap_code = wrap_simple;
     pp_code = pp_simple;
-    code_to_string = Fn.id
+    code_to_string = Fn.id;
+    wrap_bold = wrap_simple;
+    pp_bold = pp_simple;
+    bold_to_string = Fn.id
   }
 end
 
@@ -46,6 +53,10 @@ end = struct
   let pp_code fmt s = wrap_code Format.pp_print_string fmt s
   let code_to_string s = Format.asprintf "%a" pp_code s
 
+  let wrap_bold pp fmt x = Format.fprintf fmt "**%a**" pp x
+  let pp_bold fmt s = wrap_bold Format.pp_print_string fmt s
+  let bold_to_string s = Format.asprintf "%a" pp_bold s
+
   let formatter = {
     wrap_monospaced;
     pp_monospaced;
@@ -53,6 +64,9 @@ end = struct
     wrap_code;
     pp_code;
     code_to_string;
+    wrap_bold;
+    pp_bold;
+    bold_to_string;
   }
 end
 
@@ -66,3 +80,6 @@ let monospaced_to_string = formatter.monospaced_to_string
 let wrap_code = formatter.wrap_code
 let pp_code = formatter.pp_code
 let code_to_string = formatter.code_to_string
+let wrap_bold = formatter.wrap_bold
+let pp_bold = formatter.pp_bold
+let bold_to_string = formatter.bold_to_string

infer/src/base/MarkupFormatter.mli

@@ -24,3 +24,12 @@ val pp_code : Format.formatter -> string -> unit
 
 (* wrap into a code block *)
 val code_to_string : string -> string
+
+(** used to combine pp together, wrap content into a bold block  *)
+val wrap_bold : (Format.formatter -> 'a -> unit) -> Format.formatter -> 'a -> unit
+
+(** pp to wrap into a bold block  *)
+val pp_bold : Format.formatter -> string -> unit
+
+(* wrap into a bold block *)
+val bold_to_string : string -> string

infer/tests/build_systems/resource_leak_exception_lines/Makefile

@@ -0,0 +1,43 @@
+# Copyright (c) 2017 - present Facebook, Inc.
+# All rights reserved.
+#
+# This source code is licensed under the BSD style license found in the
+# LICENSE file in the root directory of this source tree. An additional grant
+# of patent rights can be found in the PATENTS file in the same directory.
+
+TESTS_DIR = ../..
+include $(TESTS_DIR)/base.make
+
+INFER_OUT = infer-out
+REPORT = $(INFER_OUT)/report.json
+EXPECTED_TEST_OUTPUT = qualifier.exp.test
+
+default: analyze
+
+.PHONY: analyze
+analyze: $(REPORT)
+
+$(REPORT):
+	$(QUIET)$(call silent_on_success, Running analysis,\
+	  $(INFER_BIN) -o $(INFER_OUT) --project-root $(CURDIR) -- javac SimpleLeak.java)
+
+$(EXPECTED_TEST_OUTPUT): $(REPORT)
+	$(QUIET)$(INFERPRINT_BIN) \
+		--issues-fields "qualifier_contains_potential_exception_note,bug_type,file,procedure,line_offset" \
+		--from-json-report $(REPORT) \
+		--issues-tests $(EXPECTED_TEST_OUTPUT)
+
+.PHONY: print
+print: $(EXPECTED_TEST_OUTPUT)
+
+.PHONY: test
+test: print
+	$(QUIET)$(call check_no_diff,qualifier.exp,$(EXPECTED_TEST_OUTPUT))
+
+.PHONY: replace
+replace: $(EXPECTED_TEST_OUTPUT)
+	cp $(EXPECTED_TEST_OUTPUT) qualifier.exp
+
+.PHONY: clean
+clean:
+	$(REMOVE_DIR) *.class *.exp.test $(INFER_OUT)

infer/tests/build_systems/resource_leak_exception_lines/SimpleLeak.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+import java.io.*;
+
+public class SimpleLeak {
+  public void method(boolean v) {
+        try {
+            FileInputStream a = new FileInputStream("aaa");
+            FileInputStream z = null;
+            if (v) {
+              z = new FileInputStream("bbb");
+              z.read();
+            }
+            z.close();
+        } catch (IOException e) {
+            // do nothing
+        }
+    }
+}

infer/tests/build_systems/resource_leak_exception_lines/qualifier.exp

@@ -0,0 +1,3 @@
+false, RESOURCE_LEAK, SimpleLeak.java, void SimpleLeak.method(boolean), 2
+false, NULL_DEREFERENCE, SimpleLeak.java, void SimpleLeak.method(boolean), 8
+true, RESOURCE_LEAK, SimpleLeak.java, void SimpleLeak.method(boolean), 9