From 5af3be7700c3eb8f1827fa63462f6d974812f7c3 Mon Sep 17 00:00:00 2001 From: Sam Zhou Date: Mon, 1 Apr 2024 21:09:43 -0700 Subject: [PATCH] [flow][refactor] Remove code actions' dependence on Flow_lsp_conversion Summary: The goal is to make autocomplete work in try-flow. Flow_lsp_converison depends on ServerProp, and ServerProt depends on logging, which pulls in some unix stuff, which won't work on flow.js. Code action service doesn't have to depend on it. I moved some of the essential conversion functions into lsp.ml, and inlined `flow_loc_patch_to_lsp_edits` at the only call site. Now code action only depends on the pure LSP module. Changelog: [internal] Reviewed By: panagosg7 Differential Revision: D55555438 fbshipit-source-id: 76f11ab330176524dc40c1f85d6f850e2c1ff333 --- src/commands/glean/gleanRunner.ml | 4 +- src/common/flow_lsp_conversions.ml | 54 ++++--------------- src/hack_forked/utils/lsp/dune | 3 +- src/hack_forked/utils/lsp/lsp.ml | 27 ++++++++++ src/hack_forked/utils/lsp/lsp.mli | 10 ++++ src/lsp/documentSymbolProvider.ml | 4 +- src/lsp/flowLsp.ml | 2 +- src/lsp/selectionRangeProvider.ml | 2 +- src/server/command_handler/commandHandler.ml | 20 +++---- .../autocomplete/autocompleteService_js.ml | 2 +- .../code_action/code_action_service.ml | 27 ++++++---- src/services/code_action/dune | 1 - src/services/code_action/lsp_import_edits.ml | 4 +- src/services/references/renameModule.ml | 2 +- 14 files changed, 81 insertions(+), 81 deletions(-) diff --git a/src/commands/glean/gleanRunner.ml b/src/commands/glean/gleanRunner.ml index 712f6a1ef81..44cef5b398d 100644 --- a/src/commands/glean/gleanRunner.ml +++ b/src/commands/glean/gleanRunner.ml @@ -39,8 +39,8 @@ module DocumentationFullspanMap = struct let comment_loc_map_ref = ref comment_loc_map in let add_to_map SymbolInformation.{ selectionRange; location = Location.{ range; _ }; _ } = let documentation = None in - let loc = Flow_lsp_conversions.lsp_range_to_flow_loc selectionRange ~source in - let span = Some (Flow_lsp_conversions.lsp_range_to_flow_loc range ~source) in + let loc = Lsp.lsp_range_to_flow_loc selectionRange ~source in + let span = Some (Lsp.lsp_range_to_flow_loc range ~source) in comment_loc_map_ref := LMap.add ~combine loc { documentation; span } !comment_loc_map_ref in List.iter add_to_map symbol_spans; diff --git a/src/common/flow_lsp_conversions.ml b/src/common/flow_lsp_conversions.ml index e8169c71bfc..e8ae16c0286 100644 --- a/src/common/flow_lsp_conversions.ml +++ b/src/common/flow_lsp_conversions.ml @@ -7,41 +7,10 @@ module Ast = Flow_ast -let flow_position_to_lsp (line : int) (char : int) : Lsp.position = - Lsp.{ line = max 0 (line - 1); character = char } - -let lsp_position_to_flow (position : Lsp.position) : int * int = - Lsp.( - (* Flow's line numbers are 1-indexed; LSP's are 0-indexed *) - let line = position.line + 1 in - let char = position.character in - (line, char) - ) - -let lsp_position_to_flow_position p = - let (line, column) = lsp_position_to_flow p in - Loc.{ line; column } - -let lsp_range_to_flow_loc ?source (range : Lsp.range) = - { - Loc.source; - start = lsp_position_to_flow_position range.Lsp.start; - _end = lsp_position_to_flow_position range.Lsp.end_; - } - -let loc_to_lsp_range (loc : Loc.t) : Lsp.range = - Loc.( - let loc_start = loc.start in - let loc_end = loc._end in - let start = flow_position_to_lsp loc_start.line loc_start.column in - let end_ = flow_position_to_lsp loc_end.line loc_end.column in - { Lsp.start; end_ } - ) - let markup_string str = { Lsp.MarkupContent.kind = Lsp.MarkupKind.Markdown; value = str } let selection_range_of_loc ?parent (loc : Loc.t) : Lsp.SelectionRange.selection_range = - { Lsp.SelectionRange.range = loc_to_lsp_range loc; parent } + { Lsp.SelectionRange.range = Lsp.loc_to_lsp_range loc; parent } let flow_signature_help_to_lsp (details : (ServerProt.Response.func_details_result list * int) option) : @@ -102,16 +71,16 @@ let flow_completion_item_to_lsp `InsertReplaceEdit { Lsp.InsertReplaceEdit.newText; - insert = loc_to_lsp_range insert; - replace = loc_to_lsp_range replace; + insert = Lsp.loc_to_lsp_range insert; + replace = Lsp.loc_to_lsp_range replace; } else - `TextEdit { Lsp.TextEdit.range = loc_to_lsp_range insert; newText }) + `TextEdit { Lsp.TextEdit.range = Lsp.loc_to_lsp_range insert; newText }) item.text_edit in let additionalTextEdits = Base.List.map - ~f:(fun (loc, newText) -> { Lsp.TextEdit.range = loc_to_lsp_range loc; newText }) + ~f:(fun (loc, newText) -> { Lsp.TextEdit.range = Lsp.loc_to_lsp_range loc; newText }) item.additional_text_edits in let documentation = Base.Option.map item.documentation ~f:(fun doc -> [Lsp.MarkedString doc]) in @@ -231,7 +200,8 @@ let file_key_to_uri (file_key_opt : File_key.t option) : (Lsp.DocumentUri.t, str let loc_to_lsp (loc : Loc.t) : (Lsp.Location.t, string) result = let ( >>| ) = Base.Result.( >>| ) in - file_key_to_uri loc.Loc.source >>| fun uri -> { Lsp.Location.uri; range = loc_to_lsp_range loc } + file_key_to_uri loc.Loc.source >>| fun uri -> + { Lsp.Location.uri; range = Lsp.loc_to_lsp_range loc } let loc_to_lsp_with_default (loc : Loc.t) ~(default_uri : Lsp.DocumentUri.t) : Lsp.Location.t = let uri = @@ -239,15 +209,11 @@ let loc_to_lsp_with_default (loc : Loc.t) ~(default_uri : Lsp.DocumentUri.t) : L | Ok uri -> uri | Error _ -> default_uri in - { Lsp.Location.uri; range = loc_to_lsp_range loc } + { Lsp.Location.uri; range = Lsp.loc_to_lsp_range loc } let flow_edit_to_textedit (edit : Loc.t * string) : Lsp.TextEdit.t = let (loc, text) = edit in - { Lsp.TextEdit.range = loc_to_lsp_range loc; newText = text } - -let flow_loc_patch_to_lsp_edits (p : (Loc.t * string) list) : Lsp.TextEdit.t list = - let convert_edit (loc, text) = { Lsp.TextEdit.range = loc_to_lsp_range loc; newText = text } in - Base.List.map ~f:convert_edit p + { Lsp.TextEdit.range = Lsp.loc_to_lsp_range loc; newText = text } (* ~, . and .. have no meaning in file urls so we don't canonicalize them *) (* but symlinks must be canonicalized before being used in flow: *) @@ -256,7 +222,7 @@ let lsp_DocumentIdentifier_to_flow_path textDocument = Sys_utils.realpath fn |> Base.Option.value ~default:fn let position_of_document_position { Lsp.TextDocumentPositionParams.position; _ } = - lsp_position_to_flow position + Lsp.lsp_position_to_flow position let diagnostics_of_flow_errors = let error_to_lsp diff --git a/src/hack_forked/utils/lsp/dune b/src/hack_forked/utils/lsp/dune index 95d7e793e70..9a8ca54801a 100644 --- a/src/hack_forked/utils/lsp/dune +++ b/src/hack_forked/utils/lsp/dune @@ -6,8 +6,7 @@ file_content file_url hh_json - flow_exit_status - utils_core) + flow_exit_status) (preprocess (pps lwt_ppx ppx_deriving.show ppx_deriving.std ppx_deriving.enum ppx_deriving.eq))) diff --git a/src/hack_forked/utils/lsp/lsp.ml b/src/hack_forked/utils/lsp/lsp.ml index 67b67de07a4..20d8ebee8cf 100644 --- a/src/hack_forked/utils/lsp/lsp.ml +++ b/src/hack_forked/utils/lsp/lsp.ml @@ -67,6 +67,33 @@ type range = { end_: position; (** the range's end position [exclusive] *) } +let flow_position_to_lsp (line : int) (char : int) : position = + { line = max 0 (line - 1); character = char } + +let lsp_position_to_flow (position : position) : int * int = + (* Flow's line numbers are 1-indexed; LSP's are 0-indexed *) + let line = position.line + 1 in + let char = position.character in + (line, char) + +let lsp_position_to_flow_position p = + let (line, column) = lsp_position_to_flow p in + { Loc.line; column } + +let lsp_range_to_flow_loc ?source (range : range) = + { + Loc.source; + start = lsp_position_to_flow_position range.start; + _end = lsp_position_to_flow_position range.end_; + } + +let loc_to_lsp_range (loc : Loc.t) : range = + let loc_start = loc.Loc.start in + let loc_end = loc.Loc._end in + let start = flow_position_to_lsp loc_start.Loc.line loc_start.Loc.column in + let end_ = flow_position_to_lsp loc_end.Loc.line loc_end.Loc.column in + { start; end_ } + (** Represents a location inside a resource, such as a line inside a text file *) module Location = struct type t = { diff --git a/src/hack_forked/utils/lsp/lsp.mli b/src/hack_forked/utils/lsp/lsp.mli index 61a60094faf..0a9b5ccdc6b 100644 --- a/src/hack_forked/utils/lsp/lsp.mli +++ b/src/hack_forked/utils/lsp/lsp.mli @@ -38,6 +38,16 @@ type range = { end_: position; } +val flow_position_to_lsp : int -> int -> position + +val lsp_position_to_flow : position -> int * int + +val lsp_position_to_flow_position : position -> Loc.position + +val lsp_range_to_flow_loc : ?source:File_key.t -> range -> Loc.t + +val loc_to_lsp_range : Loc.t -> range + module Location : sig type t = { uri: DocumentUri.t; diff --git a/src/lsp/documentSymbolProvider.ml b/src/lsp/documentSymbolProvider.ml index 36c28cbda9e..30ab26a0051 100644 --- a/src/lsp/documentSymbolProvider.ml +++ b/src/lsp/documentSymbolProvider.ml @@ -60,8 +60,8 @@ let kind_of_property is_method = Lsp.SymbolInformation.Property let mk ~loc ~selection ~name ?detail ~kind ~children () = - let range = Flow_lsp_conversions.loc_to_lsp_range loc in - let selectionRange = Flow_lsp_conversions.loc_to_lsp_range selection in + let range = Lsp.loc_to_lsp_range loc in + let selectionRange = Lsp.loc_to_lsp_range selection in { Lsp.DocumentSymbol.name; detail; kind; deprecated = false; range; selectionRange; children } class visitor = diff --git a/src/lsp/flowLsp.ml b/src/lsp/flowLsp.ml index 854caa63deb..e0cb3e6bf12 100644 --- a/src/lsp/flowLsp.ml +++ b/src/lsp/flowLsp.ml @@ -1128,7 +1128,7 @@ let lsp_DocumentItem_to_flow (open_doc : Lsp.TextDocumentItem.t) : File_input.t let diagnostic_of_parse_error (loc, parse_error) : PublishDiagnostics.diagnostic = { - Lsp.PublishDiagnostics.range = Flow_lsp_conversions.loc_to_lsp_range loc; + Lsp.PublishDiagnostics.range = Lsp.loc_to_lsp_range loc; severity = Some PublishDiagnostics.Error; code = Lsp.PublishDiagnostics.StringCode "ParseError"; source = Some "Flow"; diff --git a/src/lsp/selectionRangeProvider.ml b/src/lsp/selectionRangeProvider.ml index fefac7aedf4..a5657876548 100644 --- a/src/lsp/selectionRangeProvider.ml +++ b/src/lsp/selectionRangeProvider.ml @@ -267,7 +267,7 @@ let provide_selection_ranges positions ast = let rev_results = List.fold_result ~f:(fun acc lsp_position -> - let position = Flow_lsp_conversions.lsp_position_to_flow_position lsp_position in + let position = Lsp.lsp_position_to_flow_position lsp_position in match selection_range_tree position ast with | Some range -> Ok (range :: acc) | None -> diff --git a/src/server/command_handler/commandHandler.ml b/src/server/command_handler/commandHandler.ml index 2aa6f7e7774..a8bdce3e09f 100644 --- a/src/server/command_handler/commandHandler.ml +++ b/src/server/command_handler/commandHandler.ml @@ -1093,8 +1093,7 @@ let auto_close_jsx ~options ~env ~profiling ~params ~client = | None -> (Ok None, None) | Some (Parse_artifacts { ast; _ }) -> let target_pos = - Flow_lsp_conversions.lsp_position_to_flow_position - params.TextDocumentPositionParams.position + Lsp.lsp_position_to_flow_position params.TextDocumentPositionParams.position in let edit = Auto_close_jsx.get_snippet ast target_pos in (Ok edit, None) @@ -1121,13 +1120,12 @@ let linked_editing_range ~options ~env ~profiling ~params ~client = | Some (Parse_artifacts { ast; _ }) -> let result = let target_pos = - Flow_lsp_conversions.lsp_position_to_flow_position - params.TextDocumentPositionParams.position + Lsp.lsp_position_to_flow_position params.TextDocumentPositionParams.position in let target_loc = Loc.cursor (Some filename) target_pos.Loc.line target_pos.Loc.column in let linked_locs = Linked_editing_jsx.get_linked_locs ast target_loc in Base.Option.map linked_locs ~f:(fun linked_locs -> - let ranges = Base.List.map linked_locs ~f:Flow_lsp_conversions.loc_to_lsp_range in + let ranges = Base.List.map linked_locs ~f:Lsp.loc_to_lsp_range in { LinkedEditingRange.ranges; wordPattern = None } ) in @@ -1396,7 +1394,7 @@ let find_code_actions ~reader ~options ~env ~profiling ~params ~client = Typecheck_artifacts { cx; typed_ast; _ } ) -> let uri = TextDocumentIdentifier.(textDocument.uri) in - let loc = Flow_lsp_conversions.lsp_range_to_flow_loc ~source:file_key range in + let loc = Lsp.lsp_range_to_flow_loc ~source:file_key range in let lsp_init_params = Persistent_connection.lsp_initialize_params client in let imports_ranked_usage = rank_autoimports_by_usage ~options client in let scope_info = @@ -2718,11 +2716,7 @@ let handle_persistent_document_highlight ~reader ~options ~id ~params ~metadata ~client ~profiling ~env = (* All the locs are implicitly in the same file *) let ref_to_highlight (_, loc) = - Some - { - DocumentHighlight.range = Flow_lsp_conversions.loc_to_lsp_range loc; - kind = Some DocumentHighlight.Text; - } + Some { DocumentHighlight.range = Lsp.loc_to_lsp_range loc; kind = Some DocumentHighlight.Text } in let%lwt (result, extra_data) = map_local_find_references_results @@ -2981,9 +2975,7 @@ let handle_persistent_coverage ~options ~id ~params ~file_input ~metadata ~clien final_candidate :: singles in (* Convert to LSP *) - let loc_to_lsp loc = - { TypeCoverage.range = Flow_lsp_conversions.loc_to_lsp_range loc; message = None } - in + let loc_to_lsp loc = { TypeCoverage.range = Lsp.loc_to_lsp_range loc; message = None } in let uncoveredRanges = Base.List.map singles ~f:loc_to_lsp in (* Send the results! *) let r = diff --git a/src/services/autocomplete/autocompleteService_js.ml b/src/services/autocomplete/autocompleteService_js.ml index 4236b2495ed..eafa7851dc7 100644 --- a/src/services/autocomplete/autocompleteService_js.ml +++ b/src/services/autocomplete/autocompleteService_js.ml @@ -664,7 +664,7 @@ let src_dir_of_loc ac_loc = Loc.source ac_loc |> Base.Option.map ~f:(fun key -> File_key.to_string key |> Filename.dirname) let flow_text_edit_of_lsp_text_edit { Lsp.TextEdit.range; newText } = - let loc = Flow_lsp_conversions.lsp_range_to_flow_loc range in + let loc = Lsp.lsp_range_to_flow_loc range in (loc, newText) let completion_item_of_autoimport diff --git a/src/services/code_action/code_action_service.ml b/src/services/code_action/code_action_service.ml index 12332f1a54f..af9f8353d7a 100644 --- a/src/services/code_action/code_action_service.ml +++ b/src/services/code_action/code_action_service.ml @@ -26,13 +26,20 @@ let include_add_missing_imports_action only = include_code_action ~only add_miss let include_organize_imports_actions only = include_code_action ~only Lsp.CodeActionKind.source_organize_imports +let flow_loc_patch_to_lsp_edits = + Base.List.map ~f:(fun (loc, text) -> + { Lsp.TextEdit.range = Lsp.loc_to_lsp_range loc; newText = text } + ) + let autofix_insert_type_annotation_helper ~options ~ast ~diagnostics ~uri new_ast = let open Lsp in let diff = Insert_type.mk_diff ast new_ast in let opts = layout_options options in let edits = Replacement_printer.mk_loc_patch_ast_differ ~opts diff - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + |> Base.List.map ~f:(fun (loc, text) -> + { Lsp.TextEdit.range = Lsp.loc_to_lsp_range loc; newText = text } + ) in [ CodeAction.Action @@ -140,7 +147,7 @@ let refactor_extract_code_actions let edits = Autofix_imports.add_imports ~options:opts ~added_imports ast @ Replacement_printer.mk_loc_patch_ast_differ ~opts diff - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + |> flow_loc_patch_to_lsp_edits in let diagnostic_title = "refactor_extract" in let open Lsp in @@ -204,7 +211,7 @@ let insert_jsdoc_code_actions ~options ~ast uri loc = ast' |> Flow_ast_differ.program ast |> Replacement_printer.mk_loc_patch_ast_differ ~opts:(layout_options options) - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + |> flow_loc_patch_to_lsp_edits |> Base.List.map ~f:(fun edit -> (* This hack is needed because the differ doesn't differentiate between [comment; \n; node] and [comment; node] *) @@ -230,7 +237,7 @@ let refactor_arrow_function_code_actions ~ast ~scope_info ~options ~only uri loc ast' |> Flow_ast_differ.program ast |> Replacement_printer.mk_loc_patch_ast_differ ~opts:(layout_options options) - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + |> flow_loc_patch_to_lsp_edits |> fun edits -> let open Lsp in [ @@ -288,7 +295,7 @@ let suggest_imports [] else let src_dir = Lsp_helpers.lsp_uri_to_path uri |> Filename.dirname |> Base.Option.return in - let error_range = Flow_lsp_conversions.loc_to_lsp_range loc in + let error_range = Lsp.loc_to_lsp_range loc in let relevant_diagnostics = let open PublishDiagnostics in let lsp_code = StringCode Error_codes.(string_of_code CannotResolveName) in @@ -388,7 +395,7 @@ let autofix_in_upstream_file transform ~cx ~file_sig ~ast ~typed_ast loc >>| Flow_ast_differ.program ast >>| Replacement_printer.mk_loc_patch_ast_differ ~opts:(layout_options options) - >>| Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + >>| flow_loc_patch_to_lsp_edits >>= fun edits -> match edits with | [] -> None @@ -921,7 +928,7 @@ let code_actions_of_errors let code_action_for_parser_error_with_suggestion acc diagnostics uri ~error_loc ~editor_loc title newText = if Loc.intersects error_loc editor_loc then - let error_range = Flow_lsp_conversions.loc_to_lsp_range error_loc in + let error_range = Lsp.loc_to_lsp_range error_loc in let open Lsp in let relevant_diagnostics = diagnostics |> List.filter (fun PublishDiagnostics.{ range; _ } -> range = error_range) @@ -1242,8 +1249,7 @@ let autofix_imports in let edits = let opts = layout_options options in - Autofix_imports.add_imports ~options:opts ~added_imports ast - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + Autofix_imports.add_imports ~options:opts ~added_imports ast |> flow_loc_patch_to_lsp_edits in edits @@ -1396,7 +1402,6 @@ let insert_type let organize_imports ~options ~ast = let edits = let opts = layout_options options in - Autofix_imports.organize_imports ~options:opts ast - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + Autofix_imports.organize_imports ~options:opts ast |> flow_loc_patch_to_lsp_edits in edits diff --git a/src/services/code_action/dune b/src/services/code_action/dune index 9e813060329..1c19f5d8240 100644 --- a/src/services/code_action/dune +++ b/src/services/code_action/dune @@ -4,7 +4,6 @@ (modules (:standard \ code_action_service)) (libraries - flow_common_lsp_conversions flow_service_type_info flow_typing collections) diff --git a/src/services/code_action/lsp_import_edits.ml b/src/services/code_action/lsp_import_edits.ml index 568ff98ca22..fbbf00009ce 100644 --- a/src/services/code_action/lsp_import_edits.ml +++ b/src/services/code_action/lsp_import_edits.ml @@ -198,7 +198,9 @@ let text_edits_of_import in let edits = Autofix_imports.add_import ~options:layout_options ~bindings ~from ast - |> Flow_lsp_conversions.flow_loc_patch_to_lsp_edits + |> Base.List.map ~f:(fun (loc, text) -> + { Lsp.TextEdit.range = Lsp.loc_to_lsp_range loc; newText = text } + ) in Some { title; edits; from } diff --git a/src/services/references/renameModule.ml b/src/services/references/renameModule.ml index df32aa2609d..d2dfa41a184 100644 --- a/src/services/references/renameModule.ml +++ b/src/services/references/renameModule.ml @@ -61,7 +61,7 @@ let get_edits_for_file ~old_haste_name ~new_haste_name file_sig = let newText = Source.contents (Pretty_printer.print ~skip_endline:true ~source_maps:None string_layout) in - { Lsp.TextEdit.range = Flow_lsp_conversions.loc_to_lsp_range loc; newText } :: acc) + { Lsp.TextEdit.range = Lsp.loc_to_lsp_range loc; newText } :: acc) loc_to_replacement_map []