Skip to content

Commit

Permalink
[flow][refactor] Remove code actions' dependence on Flow_lsp_conversion
Browse files Browse the repository at this point in the history
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
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Apr 2, 2024
1 parent c203b1f commit 5af3be7
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 81 deletions.
4 changes: 2 additions & 2 deletions src/commands/glean/gleanRunner.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
54 changes: 10 additions & 44 deletions src/common/flow_lsp_conversions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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) :
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -231,23 +200,20 @@ 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 =
match file_key_to_uri loc.Loc.source with
| 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: *)
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/hack_forked/utils/lsp/dune
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

Expand Down
27 changes: 27 additions & 0 deletions src/hack_forked/utils/lsp/lsp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
10 changes: 10 additions & 0 deletions src/hack_forked/utils/lsp/lsp.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/lsp/documentSymbolProvider.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion src/lsp/flowLsp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion src/lsp/selectionRangeProvider.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
20 changes: 6 additions & 14 deletions src/server/command_handler/commandHandler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion src/services/autocomplete/autocompleteService_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 16 additions & 11 deletions src/services/code_action/code_action_service.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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] *)
Expand All @@ -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
[
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
1 change: 0 additions & 1 deletion src/services/code_action/dune
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
(modules
(:standard \ code_action_service))
(libraries
flow_common_lsp_conversions
flow_service_type_info
flow_typing
collections)
Expand Down
4 changes: 3 additions & 1 deletion src/services/code_action/lsp_import_edits.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
2 changes: 1 addition & 1 deletion src/services/references/renameModule.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
[]

Expand Down

0 comments on commit 5af3be7

Please sign in to comment.