fix rewrite for -newline-separated option and nested matches

lib/app/comby.mli

@@ -35,6 +35,7 @@ module Pipeline : sig
     -> ?metasyntax:Matchers.metasyntax
     -> ?fresh:(unit -> string)
     -> ?configuration:Matchers.configuration
+    -> ?substitute_in_place:bool
     -> single_source
     -> Matchers.specification
     -> output

lib/app/configuration/command_configuration.ml

@@ -335,14 +335,9 @@ module Printer = struct
       | Json_lines of json_kind
       | Match_only
 
-    type replacement_output =
-      { output_format : output_format
-      ; substitution_kind : substitution_kind
-      }
+    val convert : output_options -> output_format
 
-    val convert : output_options -> replacement_output
-
-    val print : replacement_output -> string option -> Replacement.t list -> string -> string -> unit
+    val print : output_format -> string option -> Replacement.t list -> string -> string -> unit
 
   end = struct
 
@@ -364,44 +359,26 @@ module Printer = struct
       | Json_lines of json_kind
       | Match_only
 
-    type replacement_output =
-      { output_format : output_format
-      ; substitution_kind : substitution_kind
-      }
+    type replacement_output = output_format
 
-    let convert output_options : replacement_output =
-      let output_format =
-        match output_options with
-        | { interactive_review = Some _; _ } -> Interactive_review
-        | { overwrite_file_in_place = true; _ } -> Overwrite_file
-        | { stdout = true; _ } -> Stdout
-        | { json_lines = true; overwrite_file_in_place = false; json_only_diff; _ } ->
-          if json_only_diff then
-            Json_lines Only_diff
-          else
-            Json_lines Everything
-        | { diff = true; color = false; _ } ->
-          Diff Plain
-        | { color = true; _ }
-        | _ ->
-          Diff Colored
-      in
-      if output_options.substitute_in_place then
-        { output_format; substitution_kind = In_place }
-      else
-        { output_format; substitution_kind = Newline_separated }
+    let convert output_options : output_format =
+      match output_options with
+      | { interactive_review = Some _; _ } -> Interactive_review
+      | { overwrite_file_in_place = true; _ } -> Overwrite_file
+      | { stdout = true; _ } -> Stdout
+      | { json_lines = true; overwrite_file_in_place = false; json_only_diff; _ } ->
+        if json_only_diff then
+          Json_lines Only_diff
+        else
+          Json_lines Everything
+      | { diff = true; color = false; _ } ->
+        Diff Plain
+      | { color = true; _ }
+      | _ ->
+        Diff Colored
 
-    let print { output_format; substitution_kind } path replacements rewritten_source source_content =
-      let open Replacement in
+    let print output_format path replacements rewritten_source source_content =
       let ppf = Format.std_formatter in
-      let rewritten_source =
-        match substitution_kind with
-        | In_place -> rewritten_source
-        | Newline_separated ->
-          List.rev_map replacements ~f:(fun { replacement_content; _ } -> replacement_content)
-          |> String.concat ~sep:"\n"
-          |> Format.sprintf "%s\n"
-      in
       let print_if_some output = Option.value_map output ~default:() ~f:(Format.fprintf ppf "%s@.") in
       match output_format with
       | Stdout ->
@@ -435,6 +412,7 @@ type t =
   ; interactive_review : interactive_review option
   ; matcher : (module Matchers.Matcher.S)
   ; metasyntax : Matchers.Metasyntax.t option
+  ; substitute_in_place : bool
   }
 
 let emit_errors { input_options; output_options; _ } =
@@ -728,6 +706,7 @@ let create
           ; color
           ; count
           ; interactive_review
+          ; substitute_in_place
           ; _
           } as output_options)
      } as configuration)
@@ -820,7 +799,7 @@ let create
       Printer.Rewrite.convert output_options
       |> fun replacement_output ->
       if match_only && color then
-        Printer.Rewrite.print { replacement_output with output_format = Match_only } source_path replacements result source_content
+        Printer.Rewrite.print Match_only source_path replacements result source_content
       else
         Printer.Rewrite.print replacement_output source_path replacements result source_content
   in
@@ -834,4 +813,5 @@ let create
     ; output_printer
     ; interactive_review
     ; metasyntax
+    ; substitute_in_place
     }

lib/app/configuration/command_configuration.mli

@@ -92,6 +92,7 @@ type t =
   ; interactive_review : interactive_review option
   ; matcher : (module Matchers.Matcher.S)
   ; metasyntax :  Matchers.Metasyntax.t option
+  ; substitute_in_place : bool
   }
 
 val create : user_input -> t Or_error.t

lib/app/pipeline/pipeline.ml

@@ -53,6 +53,7 @@ let process_single_source
     ?(timeout = 3)
     ?metasyntax
     ?fresh
+    ?(substitute_in_place = true)
     configuration
     source
     (Specification.{ rewrite_template; _ } as specification)
@@ -84,7 +85,8 @@ let process_single_source
         (* If there are no matches, return the original source (for editor support). *)
         Replacement ([], input_text, 0)
       | matches ->
-        match Rewrite.all ~source:input_text ?metasyntax ?fresh ?filepath ~rewrite_template matches with
+        let source = if substitute_in_place then Some input_text else None in
+        match Rewrite.all ?source ?metasyntax ?fresh ?filepath ~rewrite_template matches with
         | None -> Nothing
         | Some { rewritten_source; in_place_substitutions } ->
           Replacement (in_place_substitutions, rewritten_source, List.length matches)
@@ -197,6 +199,7 @@ let run_interactive
     matcher
     fast_offset_conversion
     match_configuration
+    substitute_in_place
     verbose
     timeout
     sources
@@ -212,6 +215,7 @@ let run_interactive
            ~fast_offset_conversion
            ~verbose
            ~timeout
+           ~substitute_in_place
            match_configuration
            input
            specification)
@@ -236,6 +240,7 @@ let run
     { matcher
     ; sources
     ; specifications
+    ; substitute_in_place
     ; run_options =
         { verbose
         ; match_timeout = timeout
@@ -278,6 +283,7 @@ let run
            ~timeout
            ?metasyntax
            ?fresh
+           ~substitute_in_place
            match_configuration
            input
            specification)
@@ -297,6 +303,7 @@ let run
         matcher
         fast_offset_conversion
         match_configuration
+        substitute_in_place
         verbose
         timeout
         sources
@@ -311,6 +318,7 @@ let execute
     ?metasyntax
     ?fresh
     ?(configuration = Matchers.Configuration.create ())
+    ?substitute_in_place
     source
     specification =
   process_single_source
@@ -320,6 +328,7 @@ let execute
     ?timeout
     ?metasyntax
     ?fresh
+    ?substitute_in_place
     configuration
     source
     specification

lib/app/pipeline/pipeline.mli

@@ -15,6 +15,7 @@ val process_single_source
   -> ?timeout:int
   -> ?metasyntax:Matchers.Metasyntax.t
   -> ?fresh:(unit -> string)
+  -> ?substitute_in_place:bool
   -> Matchers.Configuration.t
   -> single_source
   -> Matchers.Specification.t
@@ -26,6 +27,7 @@ val execute
   -> ?metasyntax:Matchers.Metasyntax.t
   -> ?fresh:(unit -> string)
   -> ?configuration:Matchers.Configuration.t
+  -> ?substitute_in_place:bool
   -> single_source
   -> Matchers.Specification.t
   -> output

test/common/test_cli.ml

@@ -1268,3 +1268,19 @@ let%expect_test "test_custom_metasyntax_reserved_identifiers" =
   print_string result;
   [%expect "
     (fun x -> f (x x))"]
+
+let%expect_test "test_nested_newline_rewrite" =
+  let source = "foo(foo(foo(x)))" in
+  let match_ = "foo(:[x])" in
+  let rule = "where nested" in
+  let rewrite = ":[x]" in
+  let command_args =
+    Format.sprintf "-stdin -sequential -stdout '%s' '%s' -rule '%s' -f .c -newline-separated" match_ rewrite rule
+  in
+  let command = Format.sprintf "%s %s" binary_path command_args in
+  let result = read_expect_stdin_and_stdout command source in
+  print_string result;
+  [%expect{|
+    foo(foo(x))
+    foo(x)
+    x|}]

test/common/test_pipeline.ml

@@ -21,6 +21,7 @@ let configuration (module E : Engine.S) =
   ; output_printer = (fun _ -> ())
   ; interactive_review = None
   ; metasyntax = None
+  ; substitute_in_place = true
   }
 
 (* TODO restore this, can't access the Parallel_hack module *)