introduce lazy code action resolution (codeAction/resolve)

Summary:
This diff adds infra for lazily-resolved code actions, and uses the infra for the "flip around comma" code action.

This change is a no-op from the user's point of view.

The point of these changes are to make a performant "extract into method" refactor feasible. Since "extract into method" uses libhackfmt under the hood when generating edits, it is best to generate these edits lazily.

I implement lazy code actions for "flip around comma" first (in this diff) because it's easier to demonstrate lazy code action resolution with a pre-existing refactor and also the integration test is easier to read for a more straightforward refactor.

A subsequent diff in this stack also uses the infra here for the "extract into method" refactor.

## LSP background

A "lazily-resolved code action" is a code action with this flow (LSP protocol):

- client sends [`textDocument/codeAction`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction)
- server replies with a partially-resolved code action with no `edit` nor `command` field and a `data` field with the shape of its choosing
- client sends a [`codeAction/resolve`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeAction_resolve) request with the partially-resolved code action including the `data` field

## Goals of the design

I designed lazy code action resolution to be:
1. Easy to use: for a particular refactor, all one has to to be lazy is replace `EditOnly (compute_edit args))` with `UnresolvedEdit (lazy (compute_edit args))`. No need to have a custom protocol for each refactoring. See `CodeActionsServiceFlipAroundComma.ml`.
2. Correct: we ensure the logic for finding the title and location of the code action is identical when handling the `textDocument/codeAction` request compared to the `codeAction/Resolve` request.
3. Type-safe: we distinguish at the type level between resolved and unresolved code actions. In addition, code actions have a well-typed `data` field. These are in contrast to the types in the LSP spec, which allow more kinds of invalid states to be represented.

## Design

The LSP protocol is such that the `codeAction/resolve` handler has access to the original code action returned from `textDocument/resolve`, including a custom `data` field.

We set `data` to be the original parameters to the `codeAction/resolve` request, which ensures we have enough info to resolve the rest of the code action regardless of the specific refactoring we are implementing (see goals 1-3).

A specific refactoring implements a single `find` function that is used regardless of whether the request is `textDocument/codeAction` or `codeAction/resolve` (see goals 1 and 2). CodeActionsService then does slightly different things depending on the request. The following is type-safe due to a sparing use of a phantom type param (see goal 3 and lsp.mli):
- For `textDocument/codeAction`, CodeActionsService has to transform the code action to remove functional values so it's easy to marshal the code action across our internal-to-the-language-server IPC stuff. This is just the trivial transformation from `UnresolvedEdit lazy_edit` to `UnresolvedEdit ()`.
- For `codeAction/resolve`, CodeActionsService finds the (possibly-unresolved) code action matching the code action to resolve based on the request params and title. CodeActionService then converts `UnresolvedEdit lazy_edit` to `EditOnly edit` by forcing the edit. For a `CodeAction.resolved_command_or_action`, `UnresolvedEdit` is uninhabitable, so we guarantee that `codeAction/resolve` returns a resolved code action.

Reviewed By: ljw1004

Differential Revision: D45828606

fbshipit-source-id: 8bc1f253141fa00d9bcced5b65f0f7f30b0e7fa1

hphp/hack/src/client/clientLsp.ml

@@ -3719,17 +3719,41 @@ let do_codeAction_local
   in
   Lwt.return (actions, file_path, errors_opt)
 
-let do_codeAction
+let do_codeAction_resolve_local
+    (ide_service : ClientIdeService.t ref)
+    (env : env)
+    (tracking_id : string)
+    (ref_unblocked_time : float ref)
+    (editor_open_files : Lsp.TextDocumentItem.t UriMap.t)
+    (params : CodeActionRequest.params)
+    ~(resolve_title : string) : CodeActionResolve.result Lwt.t =
+  let text_document =
+    get_text_document_item
+      editor_open_files
+      params.CodeActionRequest.textDocument.TextDocumentIdentifier.uri
+  in
+  let document = lsp_document_to_ide text_document in
+  let range = lsp_range_to_ide params.CodeActionRequest.range in
+  ide_rpc
+    ide_service
+    ~env
+    ~tracking_id
+    ~ref_unblocked_time
+    (ClientIdeMessage.Code_action_resolve { document; range; resolve_title })
+
+let do_codeAction_resolve
     (conn : server_conn)
     (ref_unblocked_time : float ref)
-    (params : CodeActionRequest.params) :
-    CodeAction.command_or_action list Lwt.t =
-  let filename =
+    (params : CodeActionRequest.params)
+    ~(resolve_title : string) : CodeActionResolve.result Lwt.t =
+  let path =
     lsp_uri_to_path
       params.CodeActionRequest.textDocument.TextDocumentIdentifier.uri
   in
   let range = lsp_range_to_ide params.CodeActionRequest.range in
-  let command = ServerCommandTypes.CODE_ACTIONS (filename, range) in
+  let command =
+    ServerCommandTypes.CODE_ACTION_RESOLVE { path; range; resolve_title }
+  in
   rpc conn ref_unblocked_time ~desc:"code_actions" command
 
 let do_signatureHelp
@@ -4795,11 +4819,12 @@ let do_initialize (local_config : ServerLocalConfig.t) : Initialize.result =
           hoverProvider = true;
           completionProvider =
             Some
-              {
-                resolveProvider = true;
-                completion_triggerCharacters =
-                  ["$"; ">"; "\\"; ":"; "<"; "["; "'"; "\""; "{"; "#"];
-              };
+              CompletionOptions.
+                {
+                  resolveProvider = true;
+                  completion_triggerCharacters =
+                    ["$"; ">"; "\\"; ":"; "<"; "["; "'"; "\""; "{"; "#"];
+                };
           signatureHelpProvider =
             Some { sighelp_triggerCharacters = ["("; ","] };
           definitionProvider = true;
@@ -4809,7 +4834,7 @@ let do_initialize (local_config : ServerLocalConfig.t) : Initialize.result =
           documentHighlightProvider = true;
           documentSymbolProvider = true;
           workspaceSymbolProvider = true;
-          codeActionProvider = true;
+          codeActionProvider = Some CodeActionOptions.{ resolveProvider = true };
           codeLensProvider = None;
           documentFormattingProvider = true;
           documentRangeFormattingProvider = true;
@@ -5737,7 +5762,10 @@ let handle_client_message
           editor_open_files
           params
       in
-      respond_jsonrpc ~powered_by:Serverless_ide id (CodeActionResult result);
+      respond_jsonrpc
+        ~powered_by:Serverless_ide
+        id
+        (CodeActionResult (result, params));
       begin
         match errors_opt with
         | None -> ()
@@ -5746,13 +5774,47 @@ let handle_client_message
       end;
       Lwt.return_some
         { result_count = List.length result; result_extra_telemetry = None }
-    (* textDocument/codeAction request, when not in serverless IDE mode *)
-    | (Main_loop menv, None, RequestMessage (id, CodeActionRequest params)) ->
+    (* codeAction/resolve request *)
+    | (_, Some ide_service, RequestMessage (id, CodeActionResolveRequest params))
+      ->
+      let CodeActionResolveRequest.{ data = code_action_request_params; title }
+          =
+        params
+      in
       let%lwt () = cancel_if_stale client timestamp short_timeout in
-      let%lwt result = do_codeAction menv.conn ref_unblocked_time params in
-      respond_jsonrpc ~powered_by:Hh_server id (CodeActionResult result);
-      Lwt.return_some
-        { result_count = List.length result; result_extra_telemetry = None }
+      let%lwt result =
+        do_codeAction_resolve_local
+          ide_service
+          env
+          tracking_id
+          ref_unblocked_time
+          editor_open_files
+          code_action_request_params
+          ~resolve_title:title
+      in
+      respond_jsonrpc
+        ~powered_by:Serverless_ide
+        id
+        (CodeActionResolveResult result);
+      Lwt.return_some { result_count = 1; result_extra_telemetry = None }
+    (* codeAction/resolve request, when not in serverless IDE mode *)
+    | ( Main_loop menv,
+        None,
+        RequestMessage (id, CodeActionResolveRequest params) ) ->
+      let%lwt () = cancel_if_stale client timestamp short_timeout in
+      let CodeActionResolveRequest.{ data = code_action_request_params; title }
+          =
+        params
+      in
+      let%lwt result =
+        do_codeAction_resolve
+          menv.conn
+          ref_unblocked_time
+          code_action_request_params
+          ~resolve_title:title
+      in
+      respond_jsonrpc ~powered_by:Hh_server id (CodeActionResolveResult result);
+      Lwt.return_some { result_count = 1; result_extra_telemetry = None }
     (* textDocument/formatting *)
     | (_, _, RequestMessage (id, DocumentFormattingRequest params)) ->
       let result = do_documentFormatting editor_open_files params in

hphp/hack/src/client/ide_service/clientIdeDaemon.ml

@@ -1450,7 +1450,6 @@ let handle_request
     in
 
     let path = Path.to_string document.file_path in
-    (* TODO: should be using RelativePath.t, not string *)
     let results =
       Provider_utils.respect_but_quarantine_unsaved_changes ~ctx ~f:(fun () ->
           CodeActionsService.go ~ctx ~entry ~path ~range)
@@ -1488,6 +1487,17 @@ let handle_request
         Some (Errors.sort_and_finalize errors)
     in
     Lwt.return (Initialized istate, Ok (results, errors_opt))
+  (* Code action resolve (refactorings, quickfixes) *)
+  | (Initialized istate, Code_action_resolve { document; range; resolve_title })
+    ->
+    let (istate, ctx, entry, _) = update_file_ctx istate document in
+
+    let path = Path.to_string document.file_path in
+    let result =
+      Provider_utils.respect_but_quarantine_unsaved_changes ~ctx ~f:(fun () ->
+          CodeActionsService.resolve ~ctx ~entry ~path ~range ~resolve_title)
+    in
+    Lwt.return (Initialized istate, Ok result)
   (* Go to definition *)
   | (Initialized istate, Definition (document, { line; column })) ->
     let (istate, ctx, entry, _) = update_file_ctx istate document in

hphp/hack/src/client/ide_service/clientIdeMessage.ml

@@ -162,6 +162,12 @@ type _ t =
       because this is called so frequently by VSCode (when you switch tab, and every
       time the caret moves) in the hope that this will be a good balance of simple
       code and decent experience. *)
+  | Code_action_resolve : {
+      document: document;
+      range: Ide_api_types.range;
+      resolve_title: string;
+    }
+      -> Lsp.CodeActionResolve.result t
 
 let t_to_string : type a. a t -> string = function
   | Initialize_from_saved_state _ -> "Initialize_from_saved_state"
@@ -204,6 +210,11 @@ let t_to_string : type a. a t -> string = function
     Printf.sprintf "Signature_help(%s)" (Path.to_string file_path)
   | Code_action ({ file_path; _ }, _) ->
     Printf.sprintf "Code_action(%s)" (Path.to_string file_path)
+  | Code_action_resolve { document = { file_path; _ }; resolve_title; _ } ->
+    Printf.sprintf
+      "Code_action_resolve(%s, %s)"
+      (Path.to_string file_path)
+      resolve_title
   | Find_references ({ file_path; _ }, _, _) ->
     Printf.sprintf "Find_references(%s)" (Path.to_string file_path)
   | Rename ({ file_path; _ }, _, _, _) ->

hphp/hack/src/hh_single_type_check.ml

@@ -2041,12 +2041,24 @@ let handle_mode
     let (ctx, entry) = Provider_context.add_entry_if_missing ~ctx ~path in
     let src = Provider_context.read_file_contents_exn entry in
     let range = find_ide_range src in
+    let path = Relative_path.to_absolute path in
+    let resolve =
+      Lsp.CodeAction.(
+        function
+        | Action { title; _ } ->
+          (* Here (for simplicity) we are stressing CodeActionsService
+             more than a real client would: if the 'edit' field is provided
+             then the client should not request resolution but we always resolve.*)
+          CodeActionsService.resolve
+            ~ctx
+            ~entry
+            ~path
+            ~range
+            ~resolve_title:title
+        | Command _ as c -> c)
+    in
     let commands_or_actions =
-      CodeActionsService.go
-        ~ctx
-        ~entry
-        ~path:(Relative_path.to_absolute path)
-        ~range
+      CodeActionsService.go ~ctx ~entry ~path ~range |> List.map ~f:resolve
     in
     let hermeticize_paths =
       Str.global_replace (Str.regexp "\".+?.php\"") "\"FILE.php\""
@@ -2055,7 +2067,8 @@ let handle_mode
       Format.printf "No commands or actions found\n"
     else
       commands_or_actions
-      |> Lsp_fmt.print_codeActionResult
+      |> List.map ~f:Lsp_fmt.print_codeActionResolveResult
+      |> Hh_json.array_ Fn.id
       |> Hh_json.json_to_string ~sort_keys:true ~pretty:true
       |> hermeticize_paths
       |> Format.printf "%s\n"

hphp/hack/src/server/codeActionsService.ml

@@ -96,7 +96,7 @@ let text_edits (classish_starts : Pos.t SMap.t) (quickfix : Pos.t Quickfix.t) :
 
 let fix_action
     path (classish_starts : Pos.t SMap.t) (quickfix : Pos.t Quickfix.t) :
-    Lsp.CodeAction.command_or_action =
+    Lsp.CodeAction.resolvable_command_or_action =
   let open Lsp in
   let changes = SMap.singleton path (text_edits classish_starts quickfix) in
   CodeAction.Action
@@ -112,7 +112,7 @@ let fix_action
 
 let refactor_action
     path (classish_starts : Pos.t SMap.t) (quickfix : Pos.t Quickfix.t) :
-    Lsp.CodeAction.command_or_action =
+    Lsp.CodeAction.resolvable_command_or_action =
   let open Lsp in
   let changes = SMap.singleton path (text_edits classish_starts quickfix) in
   CodeAction.Action
@@ -131,7 +131,7 @@ let actions_for_errors
     (path : string)
     (classish_starts : Pos.t SMap.t)
     ~(start_line : int)
-    ~(start_col : int) : Lsp.CodeAction.command_or_action list =
+    ~(start_col : int) : Lsp.CodeAction.resolvable_command_or_action list =
   let errors = Errors.get_error_list ~drop_fixmed:false errors in
   let errors_here =
     List.filter errors ~f:(fun e ->
@@ -154,11 +154,12 @@ let lsp_range_of_ide_range (ide_range : Ide_api_types.range) : Lsp.range =
       end_ = lsp_pos_of_ide_pos ide_range.I.ed;
     }
 
-let go
+let find
     ~(ctx : Provider_context.t)
     ~(entry : Provider_context.entry)
     ~(path : string)
-    ~(range : Ide_api_types.range) : Lsp.CodeAction.command_or_action list =
+    ~(range : Ide_api_types.range) :
+    Lsp.CodeAction.resolvable_command_or_action list =
   let open Ide_api_types in
   let start_line = range.st.line in
   let start_col = range.st.column in
@@ -199,3 +200,59 @@ let go
   @ override_method_commands_or_actions
   @ variable_actions
   @ CodeActionsServiceFlipAroundComma.find ~range:lsp_range ~path ~entry ctx
+
+let go
+    ~(ctx : Provider_context.t)
+    ~(entry : Provider_context.entry)
+    ~(path : string)
+    ~(range : Ide_api_types.range) : Lsp.CodeAction.command_or_action list =
+  let open Lsp.CodeAction in
+  let strip : resolvable_command_or_action -> command_or_action = function
+    | Command _ as c -> c
+    | Action ({ action; _ } as a) ->
+      let action =
+        match action with
+        | UnresolvedEdit _ -> UnresolvedEdit ()
+        | EditOnly e -> EditOnly e
+        | CommandOnly c -> CommandOnly c
+        | BothEditThenCommand ca -> BothEditThenCommand ca
+      in
+      Action { a with action }
+  in
+
+  find ~ctx ~entry ~path ~range |> List.map ~f:strip
+
+let resolve
+    ~(ctx : Provider_context.t)
+    ~(entry : Provider_context.entry)
+    ~(path : string)
+    ~(range : Ide_api_types.range)
+    ~(resolve_title : string) : Lsp.CodeAction.resolved_command_or_action =
+  let open Lsp.CodeAction in
+  let resolve_command_or_action :
+      resolvable_command_or_action -> resolved_command_or_action = function
+    | Command _ as c -> c
+    | Action ({ action; _ } as a) ->
+      let action =
+        match action with
+        | UnresolvedEdit lazy_edit -> EditOnly (Lazy.force lazy_edit)
+        | EditOnly e -> EditOnly e
+        | CommandOnly c -> CommandOnly c
+        | BothEditThenCommand ca -> BothEditThenCommand ca
+      in
+      Action { a with action }
+  in
+
+  find ~ctx ~entry ~path ~range
+  |> List.find ~f:(fun command_or_action ->
+         let title = Lsp_helpers.title_of_command_or_action command_or_action in
+         String.equal title resolve_title)
+  |> Option.value_exn
+       ~message:
+         {|Expected the code action requested with codeAction/resolve to be findable.
+Note: This error message may be caused by the source text changing between
+when the code action menu pops up and when the user selects the code action.
+In such cases we may not be able to find a code action at the same location with
+the same title, so cannot resolve the code action.
+|}
+  |> resolve_command_or_action

hphp/hack/src/server/codeActionsService.mli

@@ -12,3 +12,11 @@ val go :
   path:string ->
   range:Ide_api_types.range ->
   Lsp.CodeAction.result
+
+val resolve :
+  ctx:Provider_context.t ->
+  entry:Provider_context.entry ->
+  path:string ->
+  range:Ide_api_types.range ->
+  resolve_title:string ->
+  Lsp.CodeActionResolve.result

hphp/hack/src/server/codeActionsServiceExtractVariable.mli

@@ -10,4 +10,4 @@ val find :
   path:string ->
   entry:Provider_context.entry ->
   Provider_context.t ->
-  Lsp.CodeAction.command_or_action list
+  Lsp.CodeAction.resolvable_command_or_action list

hphp/hack/src/server/codeActionsServiceFlipAroundComma.ml

@@ -215,7 +215,8 @@ let edit_of_candidate
 
 let command_or_action_of_candidate ~path ~source_text candidate =
   let action =
-    Lsp.CodeAction.EditOnly (edit_of_candidate ~path ~source_text candidate)
+    Lsp.CodeAction.UnresolvedEdit
+      (lazy (edit_of_candidate ~path ~source_text candidate))
   in
   let code_action =
     {

hphp/hack/src/server/codeActionsServiceFlipAroundComma.mli

@@ -10,4 +10,4 @@ val find :
   path:string ->
   entry:Provider_context.entry ->
   Provider_context.t ->
-  Lsp.CodeAction.command_or_action list
+  Lsp.CodeAction.resolvable_command_or_action list

hphp/hack/src/server/codeActionsServiceInlineVariable.mli

@@ -0,0 +1,13 @@
+(*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the "hack" directory of this source tree.
+ *
+ *)
+val find :
+  range:Lsp.range ->
+  path:string ->
+  entry:Provider_context.entry ->
+  Provider_context.t ->
+  Lsp.CodeAction.resolvable_command_or_action list

hphp/hack/src/server/serverCommand.ml

@@ -73,7 +73,8 @@ let rpc_command_needs_full_check : type a. a t -> bool =
   | FORMAT _ -> false
   | DUMP_FULL_FIDELITY_PARSE _ -> false
   | IDE_AUTOCOMPLETE _ -> false
-  | CODE_ACTIONS _ -> false
+  | CODE_ACTION _ -> false
+  | CODE_ACTION_RESOLVE _ -> false
   | OUTLINE _ -> false
   | IDE_IDLE -> false
   | RAGE -> false