From 89efc55328efe1b68879bbc5b388d913ad63ff0b Mon Sep 17 00:00:00 2001 From: xvw Date: Tue, 2 Jul 2024 18:43:25 +0200 Subject: [PATCH] Get rid of `rangeEnd` for `Position | Range` input --- .../docs/ocamllsp/typeEnclosing-spec.md | 12 +- .../src/custom_requests/req_type_enclosing.ml | 79 +++-- .../custom_requests/req_type_enclosing.mli | 6 +- .../test/e2e-new/type_enclosing.ml | 278 +++++++++--------- 4 files changed, 179 insertions(+), 196 deletions(-) diff --git a/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md b/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md index a027cd6a5..14bb1f3a9 100644 --- a/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md +++ b/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md @@ -25,11 +25,10 @@ There is no client capability relative to this request. ```json { - "textDocument": TextDocumentIdentifier, - "position": Position, + "uri": TextDocumentIdentifier, + "at": (Position | Range), "index": uinteger, "verbosity?": uinteger, - "rangeEnd?": Position } ``` @@ -37,9 +36,10 @@ There is no client capability relative to this request. the types lazily: normally, Merlin would return the signature of all enclosing modules, which can be very expensive. - `verbosity` determines the number of expansions of aliases in answers. - - `rangeEnd` an optional end position. If provided, only enclosings that contain the - range `[super.position; end[` will be included in the answer. - + - `at` : + - if a `Position` is given, it will returns all enclosing around the position + - if a `Range` is given, only enclosings that contain the range + `[range.start; range.end[` will be included in the answer ## Response diff --git a/ocaml-lsp-server/src/custom_requests/req_type_enclosing.ml b/ocaml-lsp-server/src/custom_requests/req_type_enclosing.ml index 2eb3853dc..9060fdfdd 100644 --- a/ocaml-lsp-server/src/custom_requests/req_type_enclosing.ml +++ b/ocaml-lsp-server/src/custom_requests/req_type_enclosing.ml @@ -7,28 +7,27 @@ let meth = "ocamllsp/typeEnclosing" module Request_params = struct type t = - { text_document_position : TextDocumentPositionParams.t + { text_document : TextDocumentIdentifier.t + ; at : [ `Range of Range.t | `Position of Position.t ] ; index : int - ; range_end : Position.t option ; verbosity : int } - let yojson_of_t { text_document_position; index; range_end; verbosity } = - match TextDocumentPositionParams.yojson_of_t text_document_position with + let yojson_of_at = function + | `Range r -> Range.yojson_of_t r + | `Position p -> Position.yojson_of_t p + + let yojson_of_t { text_document; index; at; verbosity } = + match TextDocumentIdentifier.yojson_of_t text_document with | `Assoc assoc -> let index = ("index", `Int index) in - let range_end = - ( "rangeEnd" - , match range_end with - | Some x -> Position.yojson_of_t x - | None -> `Null ) - in + let range_end = ("at", yojson_of_at at) in let verbosity = ("verbosity", `Int verbosity) in `Assoc (index :: range_end :: verbosity :: assoc) | _ -> (* unreachable *) assert false - let create ?range_end ?(verbosity = 0) ~text_document_position ~index () = - { text_document_position; index; range_end; verbosity } + let create ?(verbosity = 0) ~text_document ~at ~index () = + { text_document; index; at; verbosity } let json_error json = Json.error "invalid Req_type_enclosing.Request_params" json @@ -49,23 +48,23 @@ module Request_params = struct the we ask for a verbosity level set to 0. *) 0 - let range_end_of_yojson params = - match List.assoc_opt "rangeEnd" params with - | Some range_end -> Some (Position.t_of_yojson range_end) + let at_of_yojson json params = + match List.assoc_opt "at" params with + | Some at -> ( + try `Position (Position.t_of_yojson at) + with _ -> `Range (Range.t_of_yojson at)) | _ -> - (* If the parameter is incorrectly formatted or missing, it is assumed that - the we do not provide rangeEnd parameter. *) - None + (* If the parameter is incorrectly formatted or missing, we refuse to build + the parameter, [at] is mandatory. *) + json_error json let t_of_yojson = function | `Assoc params as json -> let verbosity = verbosity_of_yojson params in - let range_end = range_end_of_yojson params in + let at = at_of_yojson json params in let index = index_of_yojson json params in - let text_document_position = - TextDocumentPositionParams.t_of_yojson json - in - { index; range_end; verbosity; text_document_position } + let text_document = TextDocumentIdentifier.t_of_yojson json in + { index; at; verbosity; text_document } | json -> json_error json end @@ -104,19 +103,9 @@ let make_enclosing_command position index = let get_first_enclosing_index range_end enclosings = List.find_mapi enclosings ~f:(fun i (loc, _, _) -> let range = Range.of_loc loc in - match - ( Position.compare range_end range.start - , Position.compare range_end range.end_ ) - with - | Ordering.(Gt, Gt) - | Ordering.(Eq, Lt) - | Ordering.(Gt, Eq) - | Ordering.(Eq, Eq) - | Ordering.(Gt, Lt) -> Some i - | Ordering.Lt, Ordering.Lt - | Ordering.Lt, Ordering.Eq - | Ordering.Lt, Ordering.Gt - | Ordering.Eq, Ordering.Gt -> None) + match Position.compare range_end range.end_ with + | Ordering.Lt | Ordering.Eq -> Some i + | Ordering.Gt -> None) let dispatch_command pipeline command first_index index = let rec aux i acc = function @@ -161,10 +150,8 @@ let dispatch_without_range_end pipeline position index = let command = make_enclosing_command position index in dispatch_command pipeline command 0 index -let dispatch_type_enclosing - (text_document_position : TextDocumentPositionParams.t) index range_end - pipeline = - let position = Position.logical text_document_position.position in +let dispatch_type_enclosing position index range_end pipeline = + let position = Position.logical position in let result = match range_end with | None -> dispatch_without_range_end pipeline position index @@ -181,11 +168,15 @@ let dispatch_type_enclosing let on_request ~params state = Fiber.of_thunk (fun () -> let params = (Option.value ~default:(`Assoc []) params :> Json.t) in - let Request_params. - { index; verbosity; text_document_position; range_end; _ } = + let Request_params.{ index; verbosity; text_document; at } = Request_params.t_of_yojson params in - let uri = text_document_position.textDocument.uri in + let position, range_end = + match at with + | `Position p -> (p, None) + | `Range r -> (r.start, Some r.end_) + in + let uri = text_document.uri in let verbosity = Mconfig.Verbosity.Lvl verbosity in with_pipeline state uri verbosity - @@ dispatch_type_enclosing text_document_position index range_end) + @@ dispatch_type_enclosing position index range_end) diff --git a/ocaml-lsp-server/src/custom_requests/req_type_enclosing.mli b/ocaml-lsp-server/src/custom_requests/req_type_enclosing.mli index 5ccb559b1..a396ee10a 100644 --- a/ocaml-lsp-server/src/custom_requests/req_type_enclosing.mli +++ b/ocaml-lsp-server/src/custom_requests/req_type_enclosing.mli @@ -4,9 +4,9 @@ module Request_params : sig type t val create : - ?range_end:Position.t - -> ?verbosity:int - -> text_document_position:Lsp.Types.TextDocumentPositionParams.t + ?verbosity:int + -> text_document:Lsp.Types.TextDocumentIdentifier.t + -> at:[ `Position of Position.t | `Range of Range.t ] -> index:int -> unit -> t diff --git a/ocaml-lsp-server/test/e2e-new/type_enclosing.ml b/ocaml-lsp-server/test/e2e-new/type_enclosing.ml index b3460fffc..48959f32f 100644 --- a/ocaml-lsp-server/test/e2e-new/type_enclosing.ml +++ b/ocaml-lsp-server/test/e2e-new/type_enclosing.ml @@ -1,20 +1,23 @@ open Test.Import module Util = struct - let call_type_enclosing ?(verbosity = 0) ?range_end client position index = + let call_type_enclosing ?(verbosity = 0) client at index = let uri = DocumentUri.of_path "test.ml" in let text_document = TextDocumentIdentifier.create ~uri in + let at = + match at with + | `Range r -> Range.yojson_of_t r + | `Position p -> Position.yojson_of_t p + in let params = - `Assoc - ([ ("textDocument", TextDocumentIdentifier.yojson_of_t text_document) - ; ("position", Position.yojson_of_t position) - ; ("index", `Int index) - ; ("verbosity", `Int verbosity) - ] - @ - match range_end with - | None -> [] - | Some x -> [ ("rangeEnd", Position.yojson_of_t x) ]) + match TextDocumentIdentifier.yojson_of_t text_document with + | `Assoc assoc -> + `Assoc + (("at", at) + :: ("index", `Int index) + :: ("verbosity", `Int verbosity) + :: assoc) + | _ -> (* unreachable *) assert false in let params = Some (Jsonrpc.Structured.t_of_yojson params) in let req = @@ -27,22 +30,103 @@ module Util = struct result |> Yojson.Safe.pretty_to_string ~std:false |> print_endline let test ?range_end ~verbosity ~index ~line ~character source = - let position = Position.create ~line ~character in - let range_end = - Option.map - ~f:(fun (line, character) -> Position.create ~line ~character) - range_end + let start = Position.create ~line ~character in + let at = + match range_end with + | None -> `Position start + | Some (character, line) -> + let end_ = Position.create ~character ~line in + let range = Range.create ~start ~end_ in + `Range range in let request client = let open Fiber.O in - let+ response = - call_type_enclosing ~verbosity ?range_end client position index - in + let+ response = call_type_enclosing ~verbosity client at index in print_type_enclosing response in Helpers.test source request end +let%expect_test "Application of function without range end" = + let source = "string_of_int 42" in + let line = 0 + and character = 0 + and verbosity = 0 + and index = 0 in + Util.test ~verbosity ~index ~line ~character source; + [%expect + {| + { + "index": 0, + "enclosings": [ + { + "end": { "character": 13, "line": 0 }, + "start": { "character": 0, "line": 0 } + }, + { + "end": { "character": 13, "line": 0 }, + "start": { "character": 0, "line": 0 } + }, + { + "end": { "character": 16, "line": 0 }, + "start": { "character": 0, "line": 0 } + } + ], + "type": "int -> string" + } |}] + +let%expect_test "Application of function with range end (including the current \ + enclosing) it should not change the result" = + let source = "string_of_int 42" in + let line = 0 + and character = 0 + and range_end = (13, 0) + and verbosity = 0 + and index = 0 in + Util.test ~range_end ~verbosity ~index ~line ~character source; + [%expect + {| + { + "index": 0, + "enclosings": [ + { + "end": { "character": 13, "line": 0 }, + "start": { "character": 0, "line": 0 } + }, + { + "end": { "character": 13, "line": 0 }, + "start": { "character": 0, "line": 0 } + }, + { + "end": { "character": 16, "line": 0 }, + "start": { "character": 0, "line": 0 } + } + ], + "type": "int -> string" + } |}] + +let%expect_test "Application of function with range end (excluding the current \ + enclosing)" = + let source = "string_of_int 42" in + let line = 0 + and character = 0 + and range_end = (14, 0) + and verbosity = 0 + and index = 0 in + Util.test ~range_end ~verbosity ~index ~line ~character source; + [%expect + {| + { + "index": 0, + "enclosings": [ + { + "end": { "character": 16, "line": 0 }, + "start": { "character": 0, "line": 0 } + } + ], + "type": "string" + } |}] + let%expect_test {| The cursor is positioned on [x]. @@ -70,6 +154,37 @@ let%expect_test {| "type": "string" } |}] +let%expect_test {| + The cursor is positioned on [string_of_int] and we do not give a range. +|} + = + let source = "let x = string_of_int 2002" in + let line = 0 + and character = 8 + and verbosity = 0 + and index = 0 in + Util.test ~verbosity ~index ~line ~character source; + [%expect + {| + { + "index": 0, + "enclosings": [ + { + "end": { "character": 21, "line": 0 }, + "start": { "character": 8, "line": 0 } + }, + { + "end": { "character": 21, "line": 0 }, + "start": { "character": 8, "line": 0 } + }, + { + "end": { "character": 26, "line": 0 }, + "start": { "character": 8, "line": 0 } + } + ], + "type": "int -> string" + } |}] + let%expect_test {| The cursor is positioned on [2002]. @@ -138,7 +253,7 @@ let%expect_test {| let%expect_test {| First, let's locate on [A.z], we expect the type [t], but we - will increase the verbosity in order to get the fuill expansion of + will increase the verbosity in order to get the full expansion of [type t]. And we will have 3 enclosings: 0 : [16:06 - 16:07], the [z] expr. 1 : [02:11 - 17:03], the [struct ... end] expr. @@ -380,126 +495,3 @@ end|} ], "type": "b * int" } |}] - -let%expect_test {| - Now, the list is a little bit to large and we just want enclosings - that start at the [struct ... end] attached to the module B. - We use a [range_end] argument. - - 0. [07:13 - 12:05] the [struct .. end] (of [module B]) - 1. [02:11 - 17:03] the [struct .. end] (of [module A]) - 2. [02:00 - 17:03], the [module A] expr. -|} - = - let source = - {|type a = Foo | Bar - -module A = struct - let f () = 10 - let g = Bar - let h x = x - - module B = struct - type b = Baz - - let x = (Baz, 10) - let y = (Bar, Foo) - end - - type t = { a : string; b : float } - - let z = { a = "Hello"; b = 1.0 } -end|} - in - let line = 10 - and character = 18 - and verbosity = 0 - and index = 0 - and range_end = (7, 17) in - Util.test ~verbosity ~index ~range_end ~line ~character source; - [%expect - {| - { - "index": 0, - "enclosings": [ - { - "end": { "character": 5, "line": 12 }, - "start": { "character": 13, "line": 7 } - }, - { - "end": { "character": 5, "line": 12 }, - "start": { "character": 2, "line": 7 } - }, - { - "end": { "character": 3, "line": 17 }, - "start": { "character": 11, "line": 2 } - }, - { - "end": { "character": 3, "line": 17 }, - "start": { "character": 0, "line": 2 } - } - ], - "type": "sig type b = Baz val x : b * int val y : a * a end" - } |}] - -let%expect_test {| - Now, the list is a little bit to large and we just want enclosings - that start at the [struct ... end] attached to the module B. - We use a [range_end] argument and we can couple it with [index], - [2] for example, we get the type of [module A]. - - 0. [07:13 - 12:05] the [struct .. end] (of [module B]) - 1. [02:11 - 17:03] the [struct .. end] (of [module A]) - 2. [02:00 - 17:03], the [module A] expr. -|} - = - let source = - {|type a = Foo | Bar - -module A = struct - let f () = 10 - let g = Bar - let h x = x - - module B = struct - type b = Baz - - let x = (Baz, 10) - let y = (Bar, Foo) - end - - type t = { a : string; b : float } - - let z = { a = "Hello"; b = 1.0 } -end|} - in - let line = 10 - and character = 18 - and range_end = (7, 17) - and verbosity = 0 - and index = 2 in - Util.test ~verbosity ~range_end ~index ~line ~character source; - [%expect - {| - { - "index": 2, - "enclosings": [ - { - "end": { "character": 5, "line": 12 }, - "start": { "character": 13, "line": 7 } - }, - { - "end": { "character": 5, "line": 12 }, - "start": { "character": 2, "line": 7 } - }, - { - "end": { "character": 3, "line": 17 }, - "start": { "character": 11, "line": 2 } - }, - { - "end": { "character": 3, "line": 17 }, - "start": { "character": 0, "line": 2 } - } - ], - "type": "sig\n val f : unit -> int\n val g : a\n val h : 'a -> 'a\n module B : sig type b = Baz val x : b * int val y : a * a end\n type t = { a : string; b : float; }\n val z : t\nend" - } |}]