From 78a7865f2adaf343afb6f06375d1bd8bf9238e85 Mon Sep 17 00:00:00 2001
From: Sam Blackshear <shb@fb.com>
Date: Fri, 28 Apr 2017 10:44:13 -0700
Subject: [PATCH] [CLI] parse argfile contents into Config vars

Reviewed By: jeremydubreil

Differential Revision: D4909287

fbshipit-source-id: 5b2b9f2
---
 infer/src/base/CommandLineOption.ml | 40 ++++++++++++++++++++++++++++-
 infer/src/base/Config.ml            |  2 --
 infer/src/integration/Javac.ml      | 20 +++------------
 3 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/infer/src/base/CommandLineOption.ml b/infer/src/base/CommandLineOption.ml
index e456bb1e8..7c8feb241 100644
--- a/infer/src/base/CommandLineOption.ml
+++ b/infer/src/base/CommandLineOption.ml
@@ -780,7 +780,45 @@ let extra_env_args = ref []
 let extend_env_args args =
   extra_env_args := List.rev_append args !extra_env_args
 
-let parse_args ~usage ?parse_all action args =
+let extend_env_args args =
+  extra_env_args := List.rev_append args !extra_env_args
+
+let parse_args ~usage ?parse_all action args0 =
+  (* look inside argfiles so we can move select arguments into the top line CLI and parse them into
+     Config vars. note that we don't actually delete the arguments to the file, we just duplicate
+     them on the CLI. javac is ok with this.  *)
+  let expand_argfiles acc arg =
+    if String.is_prefix arg ~prefix:"@"
+    then
+      (* for now, we only need to parse -d. we could parse more if we wanted to, but we would risk
+         incurring the wrath of ARGUMENT_LIST_TOO_LONG *)
+      let should_parse =
+        String.is_prefix ~prefix:"-d" in
+      let fname = String.slice arg 1 (String.length arg) in
+      match In_channel.read_lines fname with
+      | lines ->
+          (* crude but we only care about simple cases that will not involve trickiness, eg
+             unbalanced or escaped quotes such as "ending in\"" *)
+          let strip =
+            String.strip ~drop:(function '"' | '\'' -> true | _ -> false) in
+          let rec parse_argfile_args acc = function
+            | flag :: ((value :: args) as rest) ->
+                let stripped_flag = strip flag in
+                if should_parse stripped_flag
+                then parse_argfile_args (stripped_flag :: strip value :: acc) args
+                else parse_argfile_args acc rest
+            | _ ->
+                acc in
+          parse_argfile_args (arg :: acc) lines
+      | exception _ ->
+          acc
+    else
+      arg :: acc in
+
+  let args =
+    if Option.is_none parse_all
+    then List.fold ~f:expand_argfiles ~init:[] (List.rev args0)
+    else args0 in
   let exe_name = Sys.executable_name in
   args_to_parse := Array.of_list (exe_name :: args);
   arg_being_parsed := 0;
diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml
index ea6bebc82..bc2cdb932 100644
--- a/infer/src/base/Config.ml
+++ b/infer/src/base/Config.ml
@@ -1436,8 +1436,6 @@ and xml_specs =
 (* The "rest" args must appear after "--" on the command line, and hence after other args, so they
    are allowed to refer to the other arg variables. *)
 
-(* BUG: these arguments will not be detected if put inside @argfiles, as supported by javac. See
-   Infer.run_javac for a version that looks inside argfiles, and discussion in D4397716. *)
 let javac_classes_out =
   CLOpt.mk_string ~parse_mode:CLOpt.Javac
     ~deprecated:["classes_out"] ~long:"" ~short:'d' ~default:CLOpt.init_work_dir
diff --git a/infer/src/integration/Javac.ml b/infer/src/integration/Javac.ml
index 1a04c4a04..280bc6c11 100644
--- a/infer/src/integration/Javac.ml
+++ b/infer/src/integration/Javac.ml
@@ -22,27 +22,13 @@ let compile compiler build_prog build_args =
     | _, Some jar -> (* fall back to java in PATH to avoid passing -jar to javac *)
         ("java", ["-jar"; jar]) in
   let cli_args, file_args =
-    let rec has_classes_out = function
-      | [] -> false
-      | ("-d" | "-classes_out")::_ -> true
-      | file_arg::tl when String.is_prefix file_arg ~prefix:"@" -> (
-          let fname = String.slice file_arg 1 (String.length file_arg) in
-          match In_channel.read_lines fname with
-          | lines ->
-              (* crude but we only care about simple cases that will not involve trickiness, eg
-                 unbalanced or escaped quotes such as "ending in\"" *)
-              let lines_without_quotes =
-                List.map ~f:(String.strip ~drop:(function '"' | '\'' -> true | _ -> false)) lines in
-              has_classes_out lines_without_quotes || has_classes_out tl
-          | exception _ ->
-              has_classes_out tl)
-      | _::tl ->
-          has_classes_out tl in
     let args =
+      let has_classes_out =
+        List.exists ~f:(function | "-d" | "-classes_out" -> true | _ -> false) build_args in
       "-verbose" :: "-g" ::
       (* Ensure that some form of "-d ..." is passed to javac. It's unclear whether this is strictly
          needed but the tests break without this for now. See discussion in D4397716. *)
-      if has_classes_out build_args then
+      if has_classes_out then
         build_args
       else
         "-d" :: Config.javac_classes_out :: build_args in