refactor: Simplify Autofix.render_fix interface (semgrep#6407)

All of the paremeters to `Autofix.render_fix` can be derived from a
`Pattern_match.t`, and it's hard to foresee a use case where a caller
doesn't have a `Pattern_match.t` but still wants a rendered autofix, and
this is a lot simpler.

`Autofix.render_fix` now returns a `Textedit.t` instead of the
replacement text alone. For now, the range in it is just the range of
the `Pattern_match.t`, but that might not always be the case. Maybe
later it will become a `Textedit.t list`.

The only downside is that this change introduces an additional file read
in tests, but that's not a big deal.

Test plan: Automated tests

semgrep-core/src/engine/Unit_engine.ml

@@ -242,7 +242,7 @@ let related_file_of_target ~ext ~file =
  * Semgrep's `--test` flag can also test autofix
  * (https://github.com/returntocorp/semgrep/pull/5190), but it has the same
  * problems as the existing autofix e2e tests for these purposes. *)
-let compare_fixes lang ~file matches =
+let compare_fixes ~file matches =
   let expected_fixed_text =
     let expected_fixed_file =
       match related_file_of_target ~ext:"fixed" ~file with
@@ -251,7 +251,7 @@ let compare_fixes lang ~file matches =
     in
     Common.read_file expected_fixed_file
   in
-  let fixed_text = Autofix.apply_fixes_to_file lang matches ~file in
+  let fixed_text = Autofix.apply_fixes_to_file matches ~file in
   Alcotest.(check string) "applied autofixes" expected_fixed_text fixed_text
 
 let match_pattern ~lang ~hook ~file ~pattern ~fix_pattern =
@@ -333,7 +333,7 @@ let regression_tests_for_lang ~with_caching files lang =
                      ~file ~pattern ~fix_pattern
                  in
                  (match fix_pattern with
-                 | Some _ -> compare_fixes lang ~file matches
+                 | Some _ -> compare_fixes ~file matches
                  | None -> ());
                  let actual = !E.g_errors in
                  let expected = E.expected_error_lines_of_files [ file ] in

semgrep-core/src/fixing/Autofix.ml

@@ -56,7 +56,16 @@ let validate_fix lang text =
  * - Printing of the resulting fix AST fails (probably because there is simply a
  *   node that is unhandled).
  * *)
-let render_fix lang metavars ~fix_pattern ~target_contents =
+let render_fix pm =
+  let* fix_pattern = pm.Pattern_match.rule_id.fix in
+  let* lang = List.nth_opt pm.Pattern_match.rule_id.languages 0 in
+  let metavars = pm.Pattern_match.env in
+  let start, end_ =
+    let start, end_ = pm.Pattern_match.range_loc in
+    let _, _, end_charpos = Parse_info.get_token_end_info end_ in
+    (start.Parse_info.charpos, end_charpos)
+  in
+  let target_contents = lazy (Common.read_file pm.Pattern_match.file) in
   let result =
     (* Fixes are not exactly patterns, but they can contain metavariables that
      * should be substituted with the nodes to which they are bound in the match.
@@ -96,7 +105,8 @@ let render_fix lang metavars ~fix_pattern ~target_contents =
     validate_fix lang text
   in
   match result with
-  | Ok x -> Some x
+  | Ok replacement_text ->
+      Some { Textedit.path = pm.file; start; end_; replacement_text }
   | Error err ->
       let msg = spf "Failed to render fix `%s`:\n%s" fix_pattern err in
       (* Print line-by-line so that each line is preceded by the logging header.
@@ -108,24 +118,13 @@ let render_fix lang metavars ~fix_pattern ~target_contents =
 (* Apply the fix for the list of matches to the given file, returning the
  * resulting file contents. Currently used only for tests, but with some changes
  * could be used in production as well. *)
-let apply_fixes_to_file lang matches ~file =
+let apply_fixes_to_file matches ~file =
   let file_text = Common.read_file file in
   let edits =
     Common.map
       (fun pm ->
-        let start, end_ =
-          let start, end_ = pm.Pattern_match.range_loc in
-          let _, _, end_charpos = Parse_info.get_token_end_info end_ in
-          (start.Parse_info.charpos, end_charpos)
-        in
-        (* TODO in production, don't assume that all matches have fixes *)
-        let fix_pattern = Option.get pm.Pattern_match.rule_id.fix in
-        match
-          render_fix lang pm.Pattern_match.env ~fix_pattern
-            ~target_contents:(lazy file_text)
-        with
-        | Some replacement_text ->
-            { Textedit.path = file; start; end_; replacement_text }
+        match render_fix pm with
+        | Some edit -> edit
         (* TODO option rather than exception if used in production *)
         | None -> failwith (spf "could not render fix for %s" file))
       matches

semgrep-core/src/fixing/Autofix.mli

@@ -1,23 +1,8 @@
-(* Attempts to render a fix. If successful, returns the text that should replace
- * the matched range in the target file. If unsuccessful, returns None. *)
-val render_fix :
-  Lang.t ->
-  (* Needed so that we can replace metavariables in the fix pattern with their
-   * corresponding AST nodes from the target file *)
-  Metavariable.bindings ->
-  (* The fix pattern itself, as written in the rule *)
-  fix_pattern:string ->
-  (* The contents of the target file. This is needed so that we can use the
-   * original text from the target file when printing metavariables.
-   *
-   * Lazy because in practice, computing this involves a read from disk which
-   * can be expensive, and it may not be needed if there is a failure before
-   * printing or if no metavariables appear in the fix pattern. *)
-  target_contents:string Lazy.t ->
-  string option
+(* Attempts to render a fix. If successful, returns the edit that should occur
+ * in the target file. If unsuccessful, returns None. *)
+val render_fix : Pattern_match.t -> Textedit.t option
 
 (* Apply the fix for the list of matches to the given file, returning the
  * resulting file contents. Currently used only for tests, but with some changes
  * could be used in production as well. *)
-val apply_fixes_to_file :
-  Lang.t -> Pattern_match.t list -> file:string -> string
+val apply_fixes_to_file : Pattern_match.t list -> file:string -> string

semgrep-core/src/reporting/JSON_report.ml

@@ -33,12 +33,7 @@ module OutH = Output_from_core_util
  * Autofix.render_fix in this library, so we need to pass it
  * as a function argument
  *)
-type render_fix =
-  Lang.t ->
-  Metavariable.bindings ->
-  fix_pattern:string ->
-  target_contents:string lazy_t ->
-  string option
+type render_fix = Pattern_match.t -> Textedit.t option
 
 (*****************************************************************************)
 (* Helpers *)
@@ -219,10 +214,8 @@ let unsafe_match_to_match render_fix_opt (x : Pattern_match.t) : Out.core_match
   in
   let rendered_fix =
     let* render_fix = render_fix_opt in
-    let* fix_pattern = x.rule_id.fix in
-    let* lang = List.nth_opt x.rule_id.languages 0 in
-    let target_contents = lazy (Common.read_file x.file) in
-    render_fix lang x.env ~fix_pattern ~target_contents
+    let* edit = render_fix x in
+    Some edit.Textedit.replacement_text
   in
   {
     Out.rule_id = x.rule_id.id;

semgrep-core/src/reporting/JSON_report.mli

@@ -1,9 +1,4 @@
-type render_fix =
-  Lang.t ->
-  Metavariable.bindings ->
-  fix_pattern:string ->
-  target_contents:string lazy_t ->
-  string option
+type render_fix = Pattern_match.t -> Textedit.t option
 
 (* Can return an Error because when have a NoTokenLocation exn when
  * trying to get the range of a match or metavar.