Compute differential of certain files only, if desired

Summary: This change introduces the a new argument that lets you restrict the results of a differential report to only certain files.

Reviewed By: mbouaziz

Differential Revision: D5236626

fbshipit-source-id: 52711e9

.gitignore

@@ -22,6 +22,7 @@
 _build_infer
 *.exp.test*
 *.test.dot
+*.test.txt
 duplicates.txt
 *.ast.sh
 *.ast.bdump

Makefile

@@ -60,6 +60,7 @@ endif # BUILD_C_ANALYZERS
 
 ifeq ($(BUILD_JAVA_ANALYZERS),yes)
 BUILD_SYSTEMS_TESTS += \
+	differential_interesting_paths_filter \
   differential_resolve_infer_eradicate_conflict \
   differential_skip_anonymous_class_renamings \
   differential_skip_duplicated_types_on_filenames \

infer/src/backend/DifferentialFilters.ml

@@ -192,11 +192,34 @@ let resolve_infer_eradicate_conflict
     preexisting = filter diff.preexisting;
   }
 
+(* 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
+  | Some (paths: SourceFile.t list) ->
+      let interesting_paths_set =
+        paths
+        |> List.filter_map
+          ~f:(fun p ->
+              if not (SourceFile.is_invalid p) && SourceFile.is_under_project_root p then
+                Some (SourceFile.to_string p)
+              else None)
+        |> String.Set.of_list in
+      fun report ->
+        List.filter
+          ~f:(fun issue -> String.Set.mem interesting_paths_set issue.Jsonbug_t.file) report
+  | None -> Fn.id
+
 let do_filter
     (diff: Differential.t)
     (renamings: FileRenamings.t)
-    ~(skip_duplicated_types: bool): Differential.t =
-  if Config.filtering then (
+    ~(skip_duplicated_types: bool)
+    ~(interesting_paths: SourceFile.t list option): Differential.t =
+  let paths_filter = interesting_paths_filter interesting_paths in
+  let apply_paths_filter_if_needed label issues =
+    if List.exists ~f:(PVariant.(=) label) Config.differential_filter_set then
+      paths_filter issues
+    else issues in
+  let diff' =
     diff
     |> (if Config.equal_analyzer Config.analyzer Config.BiAbduction then
           skip_anonymous_class_renamings
@@ -206,8 +229,13 @@ let do_filter
          else Fn.id)
     |> (if Config.resolve_infer_eradicate_conflict then
           resolve_infer_eradicate_conflict Config.analyzer Inferconfig.create_filters
-        else Fn.id))
-  else diff
+        else Fn.id) in
+  {
+    introduced = apply_paths_filter_if_needed `Introduced diff'.introduced;
+    fixed = apply_paths_filter_if_needed `Fixed diff'.fixed;
+    preexisting = apply_paths_filter_if_needed `Preexisting diff'.preexisting;
+  }
+
 
 module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY = struct
   let relative_complements = relative_complements
@@ -216,4 +244,5 @@ module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY = struct
   let value_of_qualifier_tag = value_of_qualifier_tag
   let skip_anonymous_class_renamings = skip_anonymous_class_renamings
   let resolve_infer_eradicate_conflict = resolve_infer_eradicate_conflict
+  let interesting_paths_filter = interesting_paths_filter
 end

infer/src/backend/DifferentialFilters.mli

@@ -28,7 +28,8 @@ sig
   end
 end
 
-val do_filter : Differential.t -> FileRenamings.t -> skip_duplicated_types:bool -> Differential.t
+val do_filter : Differential.t -> FileRenamings.t -> skip_duplicated_types:bool ->
+  interesting_paths:SourceFile.t list option -> Differential.t
 
 module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY : sig
   val relative_complements :
@@ -41,4 +42,6 @@ module VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY : sig
     Config.analyzer ->
     (Config.analyzer -> Inferconfig.filters) ->
     Differential.t -> Differential.t
+  val interesting_paths_filter :
+    SourceFile.t list option -> Jsonbug_t.jsonbug list -> Jsonbug_t.jsonbug list
 end

infer/src/backend/infer.ml

@@ -551,13 +551,21 @@ let differential_mode () =
       ~default:empty_report filename_opt in
   let current_report = load_report Config.report_current in
   let previous_report = load_report Config.report_previous in
+  let diff =
+    let unfiltered_diff = Differential.of_reports ~current_report ~previous_report in
+    if Config.filtering then
       let file_renamings = match Config.file_renamings with
         | Some f -> DifferentialFilters.FileRenamings.from_json_file f
         | None -> DifferentialFilters.FileRenamings.empty in
-  let diff = DifferentialFilters.do_filter
-      (Differential.of_reports ~current_report ~previous_report)
+      let interesting_paths = Option.map ~f:(fun fname ->
+          List.map ~f:(SourceFile.create ~warn_on_error:false) (In_channel.read_lines fname))
+          Config.differential_filter_files in
+      DifferentialFilters.do_filter
+        unfiltered_diff
         file_renamings
-      ~skip_duplicated_types:Config.skip_duplicated_types in
+        ~skip_duplicated_types:Config.skip_duplicated_types
+        ~interesting_paths
+    else unfiltered_diff in
   let out_path = Config.results_dir ^/ "differential" in
   Unix.mkdir_p out_path;
   Differential.to_files diff out_path

infer/src/base/Config.ml

@@ -974,6 +974,20 @@ and dependencies =
     "Translate all the dependencies during the capture. The classes in the given jar file will be \
      translated. No sources needed."
 
+and differential_filter_files =
+  CLOpt.mk_string_opt
+    ~long:"differential-filter-files" ~in_help:CLOpt.[Report, manual_generic]
+    "Specify the file containing the list of source files for which a differential report \
+     is desired. Source files should be specified relative to project root or be absolute"
+
+and differential_filter_set =
+  CLOpt.mk_symbol_seq ~long:"differential-filter-set" ~eq:PVariant.(=)
+    "Specify which set of the differential results is filtered with the modified files provided \
+     through the $(b,--differential-modified-files) argument. By default it is applied to all sets \
+     ($(b,introduced), $(b,fixed), and $(b,preexisting))"
+    ~symbols:[("introduced", `Introduced); ("fixed", `Fixed); ("preexisting", `Preexisting)]
+    ~default:[`Introduced; `Fixed; `Preexisting]
+
 and disable_checks =
   CLOpt.mk_string_list ~deprecated:["disable_checks"] ~long:"disable-checks" ~meta:"error name"
     ~in_help:CLOpt.[Report, manual_generic]
@@ -1869,6 +1883,8 @@ and debug_exceptions = !debug_exceptions
 and debug_mode = !debug
 and dependency_mode = !dependencies
 and developer_mode = !developer_mode
+and differential_filter_files = !differential_filter_files
+and differential_filter_set = !differential_filter_set
 and disable_checks = !disable_checks
 and dotty_cfg_libs = !dotty_cfg_libs
 and enable_checks = !enable_checks

infer/src/base/Config.mli

@@ -216,6 +216,8 @@ val debug_exceptions : bool
 val debug_mode : bool
 val dependency_mode : bool
 val developer_mode : bool
+val differential_filter_files : string option
+val differential_filter_set : [`Introduced | `Fixed | `Preexisting ] list
 val disable_checks : string list
 val dotty_cfg_libs : bool
 val dump_duplicate_symbols : bool

infer/src/unit/DifferentialFiltersTests.ml

@@ -514,10 +514,46 @@ let test_resolve_infer_eradicate_conflict =
     ~f:(fun (name, analyzer, expected_output) ->
         name >:: create_test analyzer expected_output)
 
+let test_interesting_paths_filter =
+  let report = [
+    create_fake_jsonbug ~bug_type:"bug_type_1" ~file:"file_1.java" ~hash:1 ();
+    create_fake_jsonbug
+      ~bug_type:(Localise.to_issue_id Localise.null_dereference) ~file:"file_2.java" ~hash:2 ();
+    create_fake_jsonbug ~bug_type:"bug_type_1" ~file:"file_4.java" ~hash:4 ();
+  ] in
+  let create_test interesting_paths expected_hashes _ =
+    let filter =
+      DifferentialFilters.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.interesting_paths_filter
+        interesting_paths in
+    let filtered_report = filter report in
+    assert_equal
+      ~pp_diff:(pp_diff_of_int_list "Bug hash")
+      expected_hashes (sorted_hashes_of_issues filtered_report) in
+  [
+    ("test_interesting_paths_filter_with_none_interesting_paths",
+     None,
+     [1;2;4]);
+    ("test_interesting_paths_filter_with_some_interesting_paths",
+     Some [
+       SourceFile.create ~warn_on_error:false "file_not_existing.java";
+       SourceFile.create ~warn_on_error:false "file_4.java";
+     ],
+     [4]);
+    ("test_interesting_paths_filter_with_some_interesting_paths_that_are_not_in_report",
+     Some [
+       SourceFile.create ~warn_on_error:false "file_not_existing.java";
+       SourceFile.create ~warn_on_error:false "file_whatever.java";
+     ],
+     []);
+  ] |> List.map
+    ~f:(fun (name, interesting_paths, expected_output) ->
+        name >:: create_test interesting_paths expected_output)
+
 let tests = "differential_filters_suite" >:::
             test_file_renamings_from_json @
             test_file_renamings_find_previous @
             test_relative_complements @
             test_skip_anonymous_class_renamings @
             test_resolve_infer_eradicate_conflict @
+            test_interesting_paths_filter @
             [test_skip_duplicated_types_on_filenames; test_value_of_qualifier_tag]

infer/tests/build_systems/differential_interesting_paths_filter/Makefile

@@ -0,0 +1,66 @@
+# 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.
+
+# E2E test involving the interesting_paths_filter function
+
+TESTS_DIR = ../..
+
+MODIFIED_FILES_FILE = filter_files.test.txt
+DIFFERENTIAL_ARGS = --differential-filter-files $(MODIFIED_FILES_FILE) \
+	--differential-filter-set fixed --differential-filter-set preexisting
+SRC_OBJECT_FILES = src/com/example/*.java src/com/example/*.class com/
+CLEAN_EXTRA = $(SRC_OBJECT_FILES) $(MODIFIED_FILES_FILE)
+
+include ../../differential.make
+
+.PHONY: compare_reports replace_reports
+compare_reports: current.exp.test previous.exp.test
+	$(QUIET)$(call check_no_diff,current.exp,current.exp.test)
+	$(QUIET)$(call check_no_diff,previous.exp,previous.exp.test)
+
+replace_reports:
+	$(COPY) current.exp.test current.exp
+	$(COPY) previous.exp.test previous.exp
+
+$(MODIFIED_FILES_FILE):
+	$(QUIET)$(call silent_on_success,Creating interesting paths input file, \
+		sed 's#__ABSOLUTE_PATH__#$(CURDIR)#g' '$(MODIFIED_FILES_FILE).template' > $@)
+
+current.exp.test: $(CURRENT_REPORT)
+	$(QUIET)$(INFER_BIN) report \
+		--issues-fields $(INFERPRINT_ISSUES_FIELDS) \
+		--from-json-report $(CURRENT_DIR)/report.json \
+		--issues-tests current.exp.test
+
+previous.exp.test: $(PREVIOUS_REPORT)
+	$(QUIET)$(INFER_BIN) report \
+		--issues-fields $(INFERPRINT_ISSUES_FIELDS) \
+		--from-json-report $(PREVIOUS_DIR)/report.json \
+		--issues-tests previous.exp.test
+
+test: compare_reports
+print: current.exp.test previous.exp.test
+replace: replace_reports
+$(DIFFERENTIAL_REPORT): $(MODIFIED_FILES_FILE)
+
+$(CURRENT_REPORT):
+	$(QUIET)$(COPY) src/com/example/DiffClass1.java.current src/com/example/DiffClass1.java
+	$(QUIET)$(COPY) src/com/example/DiffClass2.java.current src/com/example/DiffClass2.java
+	$(QUIET)$(COPY) src/com/example/DiffClass3.java.current src/com/example/DiffClass3.java
+	$(QUIET)$(COPY) src/com/example/DiffClassUnchanged.java.unchanged src/com/example/DiffClassUnchanged.java
+	$(QUIET)$(call silent_on_success,Testing Differential with interesting paths: current,\
+		$(INFER_BIN) -o $(CURRENT_DIR) -- $(JAVAC) src/com/example/*.java)
+	$(QUIET)$(REMOVE_DIR) $(SRC_OBJECT_FILES)
+
+$(PREVIOUS_REPORT):
+	$(QUIET)$(COPY) src/com/example/DiffClass1.java.previous src/com/example/DiffClass1.java
+	$(QUIET)$(COPY) src/com/example/DiffClass2.java.previous src/com/example/DiffClass2.java
+	$(QUIET)$(COPY) src/com/example/DiffClassThree.java.previous src/com/example/DiffClassThree.java
+	$(QUIET)$(COPY) src/com/example/DiffClassUnchanged.java.unchanged src/com/example/DiffClassUnchanged.java
+	$(QUIET)$(call silent_on_success,Testing Differential with interesting paths: previous,\
+		$(INFER_BIN) -o $(PREVIOUS_DIR) -- $(JAVAC) src/com/example/*.java)
+	$(QUIET)$(REMOVE_DIR) $(SRC_OBJECT_FILES)

infer/tests/build_systems/differential_interesting_paths_filter/current.exp

@@ -0,0 +1,5 @@
+NULL_DEREFERENCE, src/com/example/DiffClass1.java, int DiffClass1.triggerNpe(), 1, com.example.DiffClass1.triggerNpe():int.0f35dc00a0f5d9c32cb58361b79c5a0c, com.example.DiffClass1.triggerNpe():int
+RESOURCE_LEAK, src/com/example/DiffClass2.java, void DiffClass2.openResource(), 5, com.example.DiffClass2.openResource():void.6561ab0ad86c3847cc965b847ac80324, com.example.DiffClass2.openResource():void
+NULL_DEREFERENCE, src/com/example/DiffClass3.java, int DiffClassThree.doStuff3(), 1, com.example.DiffClassThree.doStuff3():int.d5d2e8b4c4f1e60721b0e4cebf811191, com.example.DiffClassThree.doStuff3():int
+NULL_DEREFERENCE, src/com/example/DiffClassUnchanged.java, int DiffClassUnchanged.doWrongStuff(), 1, com.example.DiffClassUnchanged.doWrongStuff():int.8aa1760ea64381fc9a842a0c0c3f0909, com.example.DiffClassUnchanged.doWrongStuff():int
+NULL_DEREFERENCE, src/com/example/DiffClassUnchanged.java, int DiffClassUnchanged.tellMeTheLength(), 1, com.example.DiffClassUnchanged.tellMeTheLength():int.06e7bdb4fc272c724e2b9dfc4e5026dc, com.example.DiffClassUnchanged.tellMeTheLength():int

infer/tests/build_systems/differential_interesting_paths_filter/filter_files.test.txt.template

@@ -0,0 +1,6 @@
+src/com/example/DiffClass2.java
+__ABSOLUTE_PATH__/src/com/example/DiffClass3.java
+src/com/example/DiffClassThreeRenamed.java
+non-existing-unrelated-file.txt
+src/com/example/unrelated_file.txt
+/absolute/non-existing/p_a_t_h/to/file.txt

infer/tests/build_systems/differential_interesting_paths_filter/fixed.exp

@@ -0,0 +1 @@
+NULL_DEREFERENCE, src/com/example/DiffClass2.java, int DiffClass2.doStuff(), 1, com.example.DiffClass2.doStuff():int.0bd745e5e0ee00c272a09256adbe19d2, com.example.DiffClass2.doStuff():int

infer/tests/build_systems/differential_interesting_paths_filter/introduced.exp

@@ -0,0 +1,2 @@
+NULL_DEREFERENCE, src/com/example/DiffClass3.java, int DiffClassThree.doStuff3(), 1, com.example.DiffClassThree.doStuff3():int.d5d2e8b4c4f1e60721b0e4cebf811191, com.example.DiffClassThree.doStuff3():int
+NULL_DEREFERENCE, src/com/example/DiffClassUnchanged.java, int DiffClassUnchanged.tellMeTheLength(), 1, com.example.DiffClassUnchanged.tellMeTheLength():int.06e7bdb4fc272c724e2b9dfc4e5026dc, com.example.DiffClassUnchanged.tellMeTheLength():int

infer/tests/build_systems/differential_interesting_paths_filter/preexisting.exp

@@ -0,0 +1 @@
+RESOURCE_LEAK, src/com/example/DiffClass2.java, void DiffClass2.openResource(), 5, com.example.DiffClass2.openResource():void.6561ab0ad86c3847cc965b847ac80324, com.example.DiffClass2.openResource():void

infer/tests/build_systems/differential_interesting_paths_filter/previous.exp

@@ -0,0 +1,4 @@
+NULL_DEREFERENCE, src/com/example/DiffClass1.java, int DiffClass1.triggerNpe(), 1, com.example.DiffClass1.triggerNpe():int.0f35dc00a0f5d9c32cb58361b79c5a0c, com.example.DiffClass1.triggerNpe():int
+NULL_DEREFERENCE, src/com/example/DiffClass2.java, int DiffClass2.doStuff(), 1, com.example.DiffClass2.doStuff():int.0bd745e5e0ee00c272a09256adbe19d2, com.example.DiffClass2.doStuff():int
+RESOURCE_LEAK, src/com/example/DiffClass2.java, void DiffClass2.openResource(), 5, com.example.DiffClass2.openResource():void.6561ab0ad86c3847cc965b847ac80324, com.example.DiffClass2.openResource():void
+NULL_DEREFERENCE, src/com/example/DiffClassUnchanged.java, int DiffClassUnchanged.doWrongStuff(), 1, com.example.DiffClassUnchanged.doWrongStuff():int.8aa1760ea64381fc9a842a0c0c3f0909, com.example.DiffClassUnchanged.doWrongStuff():int

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClass1.java.current

@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+class DiffClass1 {
+  private String genString() {
+      return null;
+  }
+
+
+  private int triggerNpe() {
+      return this.genString().length();
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClass1.java.previous

@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+class DiffClass1 {
+  private String genString() {
+      return null;
+  }
+
+
+  private int triggerNpe() {
+      return this.genString().length();
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClass2.java.current

@@ -0,0 +1,24 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+import java.io.FileInputStream;
+
+class DiffClass2 {
+  private void openResource() {
+      try {
+          FileInputStream fis = new FileInputStream("AAA");
+          fis.read();
+          fis.close();
+      } catch (Exception exc) {
+          // do nothing
+      }
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClass2.java.previous

@@ -0,0 +1,34 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+import java.io.FileInputStream;
+
+class DiffClass2 {
+  private String createString() {
+      return null;
+  }
+
+
+  private int doStuff() {
+      return this.createString().length();
+  }
+
+
+  private void openResource() {
+      try {
+          FileInputStream fis = new FileInputStream("AAA");
+          fis.read();
+          fis.close();
+      } catch (Exception exc) {
+          // do nothing
+      }
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClass3.java.current

@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+class DiffClassThree {
+  public static String createString3() {
+      return null;
+  }
+
+
+  private int doStuff3() {
+      return DiffClassThree.createString3().length();
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClassThree.java.previous

@@ -0,0 +1,16 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+class DiffClassThree {
+  public static String createString3() {
+      return "this is a string";
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/DiffClassUnchanged.java.unchanged

@@ -0,0 +1,25 @@
+/*
+ * 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.
+ */
+
+package com.example;
+
+class DiffClassUnchanged {
+  private int tellMeTheLength() {
+      return DiffClassThree.createString3().length();
+  }
+
+  private String createNullString() {
+      return null;
+  }
+
+
+  private int doWrongStuff() {
+      return this.createNullString().length();
+  }
+}

infer/tests/build_systems/differential_interesting_paths_filter/src/com/example/unrelated_file.txt

@@ -0,0 +1 @@
+this file is unrelated_file.txt