[diff] make use of --changed-file-index

Summary: Another useful functionality of the diff analysis.

Reviewed By: martinoluca

Differential Revision: D5499545

fbshipit-source-id: 65b1dd6

infer/src/backend/infer.ml

@@ -17,17 +17,6 @@ module CLOpt = CommandLineOption
 module L = Logging
 module F = Format
 
-let read_config_changed_files () =
-  match Config.changed_files_index with
-  | None
-   -> None
-  | Some index ->
-    match Utils.read_file index with
-    | Ok lines
-     -> Some (SourceFile.changed_sources_from_changed_files lines)
-    | Error error
-     -> L.external_error "Error reading the changed files index '%s': %s@." index error ; None
-
 let run driver_mode =
   let open Driver in
   run_prologue driver_mode ;
@@ -99,7 +88,7 @@ let () =
       in
       L.environment_info "Starting analysis %a" pp_cluster_opt Config.cluster_cmdline ;
       InferAnalyze.register_perf_stats_report () ;
-      Driver.analyze_and_report Analyze ~changed_files:(read_config_changed_files ())
+      Driver.analyze_and_report Analyze ~changed_files:(Driver.read_config_changed_files ())
   | Clang
    -> let prog, args =
         match Array.to_list Sys.argv with prog :: args -> (prog, args) | [] -> assert false

infer/src/base/Config.ml

@@ -771,7 +771,7 @@ and calls_csv =
 
 and changed_files_index =
   CLOpt.mk_path_opt ~long:"changed-files-index"
-    ~in_help:CLOpt.([(Analyze, manual_generic)])
+    ~in_help:CLOpt.([(Analyze, manual_generic); (Diff, manual_generic)])
     ~meta:"file"
     "Specify the file containing the list of source files from which reactive analysis should start. Source files should be specified relative to project root or be absolute"
 

infer/src/integration/Diff.ml

@@ -45,19 +45,18 @@ let save_report revision =
 
 let diff driver_mode =
   Driver.run_prologue driver_mode ;
+  let changed_files = Driver.read_config_changed_files () in
   (* TODO(t15553258) run gen-build script if specified *)
   (* run capture *)
-  Driver.capture driver_mode ~changed_files:None ;
-  (* run analysis TODO(t15553258) add --changed_files_index *)
-  Driver.analyze_and_report driver_mode ~changed_files:None ;
+  Driver.capture driver_mode ~changed_files ;
+  Driver.analyze_and_report driver_mode ~changed_files ;
   let current_report = Some (save_report Current) in
   (* TODO(t15553258) bail if nothing to analyze (configurable, some people might care about bugs
      fixed more than about time to analyze) *)
   checkout Previous ;
   (* TODO(t15553258) run gen-build script if specified *)
-  Driver.capture driver_mode ~changed_files:None ;
-  (* run analysis TODO(t15553258) add --changed_files_index *)
-  Driver.analyze_and_report driver_mode ~changed_files:None ;
+  Driver.capture driver_mode ~changed_files ;
+  Driver.analyze_and_report driver_mode ~changed_files ;
   checkout Current ;
   let previous_report = Some (save_report Previous) in
   (* compute differential *)

infer/src/integration/Driver.ml

@@ -551,3 +551,14 @@ let run_epilogue driver_mode =
         fail_on_issue_epilogue () ) ;
   if Config.buck_cache_mode then clean_results_dir () ;
   ()
+
+let read_config_changed_files () =
+  match Config.changed_files_index with
+  | None
+   -> None
+  | Some index ->
+    match Utils.read_file index with
+    | Ok lines
+     -> Some (SourceFile.changed_sources_from_changed_files lines)
+    | Error error
+     -> L.external_error "Error reading the changed files index '%s': %s@." index error ; None

infer/src/integration/Driver.mli

@@ -43,3 +43,6 @@ val analyze_and_report : changed_files:SourceFile.Set.t option -> mode -> unit
 
 val run_epilogue : mode -> unit
 (** cleanup infer-out/ for Buck, generate stats, and generally post-process the results of a run *)
+
+val read_config_changed_files : unit -> SourceFile.Set.t option
+(** return the list of changed files as read from Config.changed_files_index and passed to SourceFile.changed_sources_from_changed_files *)

infer/tests/build_systems/diff/Makefile

@@ -6,58 +6,22 @@
 # 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
+include $(TESTS_DIR)/diff.make
 
 SRC_DIR = $(CURDIR)/../codetoanalyze
-INFER_OUT = infer-out
 INFER_OPTIONS = \
   --previous-to-current-script '$(COPY) $(SRC_DIR)/some_bugs.c src/hello.c' \
   --current-to-previous-script '$(COPY) $(SRC_DIR)/some_different_bugs.c src/hello.c' \
+  --changed-files-index changed_files.txt \
   --report-hook '/bin/true' \
   -- clang -c src/hello.c
 
 SOURCES = $(SRC_DIR)/some_bugs.c $(SRC_DIR)/some_different_bugs.c
 
-default: analyze
-
-.PHONY: analyze
-analyze: $(INFER_OUT)/differential/introduced.json
-
-$(INFER_OUT)/differential/introduced.json: $(SOURCES) $(CLANG_DEPS)
+$(INFER_OUT)/differential/introduced.json: $(SOURCES) $(CLANG_DEPS) changed_files.txt
 	$(QUIET)$(MKDIR_P) src
 	$(QUIET)$(COPY) $(SRC_DIR)/some_bugs.c src/hello.c
 	$(QUIET)$(call silent_on_success,Running diff analysis in $(TEST_REL_DIR),\
 	  $(INFER_BIN) --no-keep-going -o $(INFER_OUT) --project-root $(CURDIR) diff \
 	        $(INFER_OPTIONS))
-
-introduced.exp.test: $(INFER_OUT)/differential/introduced.json $(INFER_BIN)
-	$(QUIET)$(INFER_BIN) report \
-		--from-json-report $(INFER_OUT)/differential/introduced.json \
-		--issues-tests introduced.exp.test
-	$(QUIET)$(INFER_BIN) report \
-		--from-json-report $(INFER_OUT)/differential/fixed.json \
-		--issues-tests fixed.exp.test
-	$(QUIET)$(INFER_BIN) report \
-		--from-json-report $(INFER_OUT)/differential/preexisting.json \
-		--issues-tests preexisting.exp.test
-
-.PHONY: print
-print: introduced.exp.test
-
-.PHONY: test
-test: print
-	$(QUIET)$(call check_no_diff,introduced.exp,introduced.exp.test)
-	$(QUIET)$(call check_no_diff,fixed.exp,fixed.exp.test)
-	$(QUIET)$(call check_no_diff,preexisting.exp,preexisting.exp.test)
-
-.PHONY: replace
-replace: introduced.exp.test
-	$(COPY) introduced.exp.test introduced.exp
-	$(COPY) fixed.exp.test fixed.exp
-	$(COPY) preexisting.exp.test preexisting.exp
-
-.PHONY: clean
-clean:
-	$(REMOVE_DIR) *.exp.test $(INFER_OUT) $(CURRENT_DIR) $(PREVIOUS_DIR) \
-		$(CLEAN_EXTRA)

infer/tests/build_systems/diff/changed_files.txt

@@ -0,0 +1 @@
+src/hello.c

infer/tests/diff.make

@@ -0,0 +1,50 @@
+# 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.
+
+
+# you need to define the following when including this file:
+# - TESTS_DIR, specified before including the file, pointing to infer/tests/
+# - INFER_OUT
+# - a rule to build $(INFER_OUT)/differential/introduced.json
+
+include $(TESTS_DIR)/base.make
+
+default: analyze
+
+.PHONY: analyze
+analyze: $(INFER_OUT)/differential/introduced.json
+
+introduced.exp.test: $(INFER_OUT)/differential/introduced.json $(INFER_BIN)
+	$(QUIET)$(INFER_BIN) report \
+		--from-json-report $(INFER_OUT)/differential/introduced.json \
+		--issues-tests introduced.exp.test
+	$(QUIET)$(INFER_BIN) report \
+		--from-json-report $(INFER_OUT)/differential/fixed.json \
+		--issues-tests fixed.exp.test
+	$(QUIET)$(INFER_BIN) report \
+		--from-json-report $(INFER_OUT)/differential/preexisting.json \
+		--issues-tests preexisting.exp.test
+
+.PHONY: print
+print: introduced.exp.test
+
+.PHONY: test
+test: print
+	$(QUIET)$(call check_no_diff,introduced.exp,introduced.exp.test)
+	$(QUIET)$(call check_no_diff,fixed.exp,fixed.exp.test)
+	$(QUIET)$(call check_no_diff,preexisting.exp,preexisting.exp.test)
+
+.PHONY: replace
+replace: introduced.exp.test
+	$(COPY) introduced.exp.test introduced.exp
+	$(COPY) fixed.exp.test fixed.exp
+	$(COPY) preexisting.exp.test preexisting.exp
+
+.PHONY: clean
+clean:
+	$(REMOVE_DIR) *.exp.test $(INFER_OUT) $(CURRENT_DIR) $(PREVIOUS_DIR) \
+		$(CLEAN_EXTRA)