[flow] Introduce flow apply-code-action 'experimental.quickfix' command

SamChou19815 · facebook-github-bot · commit 0e6c70e161f2 · 2025-09-16T19:52:05.000-07:00
Reviewed By: panagosg7

Differential Revision: D82492454

fbshipit-source-id: b7164bc0a254a7a4b4f0679d68e7277d7c248786
diff --git a/src/commands/applyCodeActionCommand.ml b/src/commands/applyCodeActionCommand.ml
@@ -11,6 +11,77 @@ open CommandSpec
 (* This module implements the flow command `apply-code-action` which exposes LSP code-actions via the CLI *)
 let handle_error ?(code = Exit.Unknown_error) msg = Exit.(exit ~msg code)
 
+module Quickfix = struct
+  let spec =
+    {
+      name = "Apply available quickfixes";
+      doc = "Runs all safe quickfixes. If requested, run one additional best effort quickfix";
+      usage =
+        Printf.sprintf
+          "Usage: %s apply-code-action 'experimental.quickfix'  [OPTION]... FILE"
+          exe_name;
+      args =
+        ArgSpec.(
+          empty
+          |> base_flags
+          |> connect_flags
+          |> root_flag
+          |> path_flag
+          |> wait_for_recheck_flag
+          |> flag "--in-place" truthy ~doc:"Overwrite the input file"
+          |> flag
+               "--include-best-effort-fix"
+               truthy
+               ~doc:"Whether to include one best effort quickfix"
+          |> anon "file" (required string)
+        );
+    }
+
+  let handle_ok in_place patch source_path input =
+    let write_patch content =
+      let output_channel =
+        if in_place then
+          open_out source_path
+        else
+          stdout
+      in
+      output_string output_channel @@ Replacement_printer.print patch content;
+      close_out output_channel
+    in
+    match File_input.content_of_file_input input with
+    | Ok content -> write_patch content
+    | Error msg -> handle_error msg
+
+  let main
+      base_flags
+      connect_params
+      root_arg
+      path
+      wait_for_recheck
+      in_place
+      include_best_effort_fix
+      file
+      () =
+    let source_path = expand_path file in
+    let input = get_file_from_filename_or_stdin ~cmd:spec.name path (Some source_path) in
+    let root = get_the_root ~base_flags ~input root_arg in
+    let flowconfig_name = base_flags.Base_flags.flowconfig_name in
+    let request =
+      ServerProt.Request.APPLY_CODE_ACTION
+        {
+          input;
+          action = ServerProt.Code_action.Quickfix { include_best_effort_fix };
+          wait_for_recheck;
+        }
+    in
+    let result = connect_and_make_request flowconfig_name connect_params root request in
+    match result with
+    | ServerProt.Response.APPLY_CODE_ACTION (Ok patch) -> handle_ok in_place patch source_path input
+    | _ -> handle_error "Flow: invalid server response"
+
+  let command = CommandSpec.command spec main
+end
+
 module SourceAddMissingImports = struct
   let spec =
     {
@@ -24,7 +95,7 @@ module SourceAddMissingImports = struct
         ArgSpec.(
           empty
           |> base_flags
-          |> connect_and_json_flags
+          |> connect_flags
           |> root_flag
           |> path_flag
           |> wait_for_recheck_flag
@@ -48,7 +119,7 @@ module SourceAddMissingImports = struct
     | Ok content -> write_patch content
     | Error msg -> handle_error msg
 
-  let main base_flags connect_params _json _pretty root_arg path wait_for_recheck in_place file () =
+  let main base_flags connect_params root_arg path wait_for_recheck in_place file () =
     let source_path = expand_path file in
     let input = get_file_from_filename_or_stdin ~cmd:spec.name path (Some source_path) in
     let root = get_the_root ~base_flags ~input root_arg in
@@ -118,6 +189,7 @@ let command =
       ~name:"apply-code-action"
       ~doc:""
       [
+        ("experimental.quickfix", Quickfix.command);
         ("source.addMissingImports", SourceAddMissingImports.command);
         ("suggestImports", SuggestImports.command);
       ]
diff --git a/src/server/command_handler/commandHandler.ml b/src/server/command_handler/commandHandler.ml
@@ -558,6 +558,30 @@ let autocomplete_on_parsed
   Autocomplete_js.autocomplete_unset_hooks ();
   (initial_json_props, ac_result)
 
+let autofix_errors_cli
+    ~options
+    ~env
+    ~profiling
+    ~loc_of_aloc
+    ~get_ast_from_shared_mem
+    ~module_system_info
+    ~get_type_sig
+    ~include_best_effort_fix
+    ~input =
+  let file_key = file_key_of_file_input ~options ~env input in
+  File_input.content_of_file_input input >>= fun file_content ->
+  Code_action_service.autofix_errors_cli
+    ~options
+    ~profiling
+    ~env
+    ~loc_of_aloc
+    ~get_ast_from_shared_mem
+    ~module_system_info
+    ~get_type_sig
+    ~include_best_effort_fix
+    ~file_key
+    ~file_content
+
 let autofix_imports_cli ~options ~env ~profiling ~loc_of_aloc ~module_system_info ~input =
   let file_key = file_key_of_file_input ~options ~env input in
   File_input.content_of_file_input input >>= fun file_content ->
@@ -1603,6 +1627,22 @@ let rank_autoimports_by_usage ~options client =
 let handle_apply_code_action ~options ~reader ~profiling ~env action file_input =
   ServerProt.Code_action.(
     match action with
+    | Quickfix { include_best_effort_fix } ->
+      let result =
+        try_with (fun () ->
+            autofix_errors_cli
+              ~options
+              ~profiling
+              ~env
+              ~loc_of_aloc:(Parsing_heaps.Reader.loc_of_aloc ~reader)
+              ~get_ast_from_shared_mem:(Parsing_heaps.Reader.get_ast ~reader)
+              ~module_system_info:(mk_module_system_info ~options ~reader)
+              ~get_type_sig:(Parsing_heaps.Reader.get_type_sig ~reader)
+              ~include_best_effort_fix
+              ~input:file_input
+        )
+      in
+      Lwt.return (ServerProt.Response.APPLY_CODE_ACTION result, None)
     | SourceAddMissingImports ->
       let result =
         try_with (fun () ->
diff --git a/src/server/protocol/serverProt.ml b/src/server/protocol/serverProt.ml
@@ -49,10 +49,13 @@ end
 
 module Code_action = struct
   type t =
+    | Quickfix of { include_best_effort_fix: bool }
     | SourceAddMissingImports
     | SuggestImports
 
   let to_string = function
+    | Quickfix { include_best_effort_fix = false } -> "quickfix.safe"
+    | Quickfix { include_best_effort_fix = true } -> "quickfix.include_best_effort_fix"
     | SourceAddMissingImports -> "source.addMissingImports"
     | SuggestImports -> "suggestImports"
 end
diff --git a/src/services/code_action/autofix_type_name.ml b/src/services/code_action/autofix_type_name.ml
@@ -12,7 +12,7 @@ class mapper target_loc ~incorrect_name ~replacement_name =
     method! generic_type loc t =
       let open Flow_ast.Type in
       let { Generic.id; targs; comments } = t in
-      if Option.is_none targs || (not @@ this#is_target loc) then
+      if not @@ this#is_target loc then
         super#generic_type loc t
       else
         match id with
diff --git a/src/services/code_action/code_action_service.ml b/src/services/code_action/code_action_service.ml
@@ -1950,13 +1950,89 @@ let with_type_checked_file ~options ~profiling ~env ~file_key ~file_content ~f =
         intermediate_result
   in
   match file_artifacts with
-  | Ok (Parse_artifacts { ast; _ }, Typecheck_artifacts { cx; _ }) -> f ~cx ~ast
+  | Ok (Parse_artifacts { ast; file_sig; _ }, Typecheck_artifacts { cx; typed_ast; _ }) ->
+    f ~cx ~file_sig ~ast ~typed_ast
   | _ -> Error "Failed to parse or check file"
 
+let autofix_errors_cli
+    ~options
+    ~profiling
+    ~env
+    ~loc_of_aloc
+    ~get_ast_from_shared_mem
+    ~module_system_info
+    ~get_type_sig
+    ~include_best_effort_fix
+    ~file_key
+    ~file_content =
+  let get_edits ~cx ~file_sig ~ast ~typed_ast =
+    let (safe_transforms, best_effort_transforms) =
+      Flow_error.ErrorSet.fold
+        (fun error acc ->
+          let lazy_error_loc =
+            lazy
+              (let { Flow_intermediate_error_types.loc; _ } =
+                 Flow_intermediate_error.make_intermediate_error ~loc_of_aloc error
+               in
+               loc
+              )
+          in
+          match
+            ast_transforms_of_error
+              ~loc_of_aloc
+              ~lazy_error_loc
+              ~get_ast_from_shared_mem
+              ~get_haste_module_info:module_system_info.Lsp_module_system_info.get_haste_module_info
+              ~get_type_sig
+              (error |> Flow_error.map_loc_of_error loc_of_aloc |> Flow_error.msg_of_error)
+            |> Base.List.hd
+          with
+          | None -> acc
+          | Some transform ->
+            let (safe_transforms, best_effort_transforms) = acc in
+            (match transform.confidence with
+            | WillFixErrorAndSafeForRunningOnSave ->
+              (transform :: safe_transforms, best_effort_transforms)
+            | BestEffort ->
+              if include_best_effort_fix then
+                (safe_transforms, transform :: best_effort_transforms)
+              else
+                acc))
+        (Context.errors cx)
+        ([], [])
+    in
+    let (new_ast, _) =
+      Base.List.fold
+        (safe_transforms @ best_effort_transforms)
+        ~init:(ast, false)
+        ~f:(fun (ast, has_run_best_effort_fix) transform ->
+          match (transform.confidence, has_run_best_effort_fix) with
+          | (BestEffort, true) -> (ast, true)
+          | (WillFixErrorAndSafeForRunningOnSave, _) ->
+            let new_ast =
+              Base.Option.value
+                ~default:ast
+                (transform.transform ~cx ~file_sig ~ast ~typed_ast transform.target_loc)
+            in
+            (new_ast, has_run_best_effort_fix)
+          | (BestEffort, false) ->
+            let new_ast =
+              Base.Option.value
+                ~default:ast
+                (transform.transform ~cx ~file_sig ~ast ~typed_ast transform.target_loc)
+            in
+            (new_ast, new_ast != ast)
+      )
+    in
+    let opts = layout_options options in
+    Ok (Insert_type.mk_patch ~opts ast new_ast file_content)
+  in
+  with_type_checked_file ~options ~profiling ~env ~file_key ~file_content ~f:get_edits
+
 let suggest_imports_cli
     ~options ~profiling ~env ~loc_of_aloc ~module_system_info ~file_key ~file_content =
   let uri = File_key.to_string file_key |> Lsp_helpers.path_to_lsp_uri ~default_path:"" in
-  let get_edits ~cx ~ast =
+  let get_edits ~cx ~file_sig:_ ~ast ~typed_ast:_ =
     let errors = Context.errors cx in
     let (imports, _) =
       Flow_error.ErrorSet.fold
@@ -1995,7 +2071,7 @@ let suggest_imports_cli
 let autofix_imports_cli
     ~options ~profiling ~env ~loc_of_aloc ~module_system_info ~file_key ~file_content =
   let src_dir = File_key.to_string file_key |> Filename.dirname |> Base.Option.return in
-  let get_edits ~cx ~ast =
+  let get_edits ~cx ~file_sig:_ ~ast ~typed_ast:_ =
     let edits = autofix_imports ~options ~env ~loc_of_aloc ~module_system_info ~cx ~ast ~src_dir in
     Ok (Replacement_printer.loc_patch_to_patch file_content edits)
   in
diff --git a/src/services/code_action/code_action_service.mli b/src/services/code_action/code_action_service.mli
@@ -67,6 +67,19 @@ val code_actions_at_loc :
   loc:Loc.t ->
   (Lsp.CodeAction.command_or_action list, string) result
 
+val autofix_errors_cli :
+  options:Options.t ->
+  profiling:Profiling_js.running ->
+  env:ServerEnv.env ->
+  loc_of_aloc:(ALoc.t -> Loc.t) ->
+  get_ast_from_shared_mem:(File_key.t -> (Loc.t, Loc.t) Flow_ast.Program.t option) ->
+  module_system_info:Lsp_module_system_info.t ->
+  get_type_sig:(File_key.t -> Type_sig_collections.Locs.index Packed_type_sig.Module.t option) ->
+  include_best_effort_fix:bool ->
+  file_key:File_key.t ->
+  file_content:string ->
+  (Replacement_printer.patch, string) result
+
 val autofix_imports_cli :
   options:Options.t ->
   profiling:Profiling_js.running ->
diff --git a/tests/apply_code_action_command/apply_code_action_command.exp b/tests/apply_code_action_command/apply_code_action_command.exp
@@ -81,4 +81,36 @@ Found 0 errors
       "title":"Import from ./ExportFoo2"
     }
   ]
-}
+}
+> apply-code-action 'experimental.quickfix' tmp/test-quickfixes.js
+// $FlowFixMe[internal-type]
+type T1 = React.Node;
+// $FlowFixMe[internal-type]
+type T2 = React.Node;
+
+type Expected = {
+    foo?: number,
+    bar: 'foo' | 'bar',
+}
+
+declare export function assertExpected(expected: Expected): void;
+
+const obj = {bar: 'foo' as const};
+// $FlowFixMe[incompatible-type]
+assertExpected(obj);
+> apply-code-action 'experimental.quickfix' tmp/test-quickfixes.js --include-best-effort-fix
+// $FlowFixMe[internal-type]
+type T1 = React.Node;
+// $FlowFixMe[internal-type]
+type T2 = React.Node;
+
+type Expected = {
+    foo?: number,
+    bar: 'foo' | 'bar',
+}
+
+declare export function assertExpected(expected: Expected): void;
+
+const obj: Expected = {bar: 'foo' as const};
+// $FlowFixMe[incompatible-type]
+assertExpected(obj);
diff --git a/tests/apply_code_action_command/test-quickfixes.js b/tests/apply_code_action_command/test-quickfixes.js
@@ -0,0 +1,15 @@
+// $FlowFixMe[internal-type]
+type T1 = React$Node;
+// $FlowFixMe[internal-type]
+type T2 = React$Node;
+
+type Expected = {
+    foo?: number,
+    bar: 'foo' | 'bar',
+}
+
+declare export function assertExpected(expected: Expected): void;
+
+const obj = {bar: 'foo' as const};
+// $FlowFixMe[incompatible-type]
+assertExpected(obj);
diff --git a/tests/apply_code_action_command/test.sh b/tests/apply_code_action_command/test.sh
@@ -22,3 +22,8 @@ echo '> Confirm no errors'
 assert_ok "$FLOW" focus-check tmp/multi.js
 echo '> apply-code-action suggestImports tmp/suggest_imports.js'
 "$FLOW" apply-code-action suggestImports --pretty tmp/suggest_imports.js | sed -e 's/file:.*:/file:tmp\/suggest_imports.js": /'
+
+echo '> apply-code-action '\''experimental.quickfix'\'' tmp/test-quickfixes.js'
+assert_ok "$FLOW" apply-code-action 'experimental.quickfix' tmp/test-quickfixes.js
+echo '> apply-code-action '\''experimental.quickfix'\'' tmp/test-quickfixes.js --include-best-effort-fix'
+assert_ok "$FLOW" apply-code-action 'experimental.quickfix' tmp/test-quickfixes.js --include-best-effort-fix
