From e5a83f8675a588ab1d21d34a3e57810d90b47974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Thu, 3 Oct 2024 15:39:53 +0200 Subject: [PATCH 1/4] Add more tests Co-authored-by: PizieDust --- .../textDocument-selectionRange.test.ts | 546 ++++++++++++++++++ 1 file changed, 546 insertions(+) diff --git a/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts b/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts index c091a7d88..b83aeb728 100644 --- a/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts +++ b/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts @@ -148,4 +148,550 @@ Array [ ] `); }); + + it("returns a selection range for more complex documents", async () => { + openDocument(outdent` + type _ typ = + | TInt : int typ + | TBool : bool typ + module M = struct + type t + let f (_ : _ typ) = () + end + `); + + let result = await selectionRange([Types.Position.create(5, 23)]); + expect(result).toMatchInlineSnapshot(` +Array [ + Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "range": Object { + "end": Object { + "character": 3, + "line": 6, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 6, + }, + "start": Object { + "character": 0, + "line": 3, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 6, + }, + "start": Object { + "character": 0, + "line": 3, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 6, + }, + "start": Object { + "character": 11, + "line": 3, + }, + }, + }, + "range": Object { + "end": Object { + "character": 24, + "line": 5, + }, + "start": Object { + "character": 2, + "line": 4, + }, + }, + }, + "range": Object { + "end": Object { + "character": 24, + "line": 5, + }, + "start": Object { + "character": 2, + "line": 5, + }, + }, + }, + "range": Object { + "end": Object { + "character": 24, + "line": 5, + }, + "start": Object { + "character": 2, + "line": 5, + }, + }, + }, + "range": Object { + "end": Object { + "character": 24, + "line": 5, + }, + "start": Object { + "character": 8, + "line": 5, + }, + }, + }, + "range": Object { + "end": Object { + "character": 24, + "line": 5, + }, + "start": Object { + "character": 22, + "line": 5, + }, + }, + }, +] +`)}); + + it("returns a selection range for functors", async () => { + openDocument(outdent` +module M = Map.Make (struct + type t = { o: < rank : int > } + let compare a b = a.o#rank - b.o#rank +end)`); + + let result = await selectionRange([Types.Position.create(2, 26)]); + expect(result).toMatchInlineSnapshot(` +Array [ + Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "range": Object { + "end": Object { + "character": 4, + "line": 3, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 4, + "line": 3, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 4, + "line": 3, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 4, + "line": 3, + }, + "start": Object { + "character": 11, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 3, + }, + "start": Object { + "character": 21, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 39, + "line": 2, + }, + "start": Object { + "character": 2, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 39, + "line": 2, + }, + "start": Object { + "character": 2, + "line": 2, + }, + }, + }, + "range": Object { + "end": Object { + "character": 39, + "line": 2, + }, + "start": Object { + "character": 2, + "line": 2, + }, + }, + }, + "range": Object { + "end": Object { + "character": 39, + "line": 2, + }, + "start": Object { + "character": 14, + "line": 2, + }, + }, + }, + "range": Object { + "end": Object { + "character": 39, + "line": 2, + }, + "start": Object { + "character": 20, + "line": 2, + }, + }, + }, + "range": Object { + "end": Object { + "character": 28, + "line": 2, + }, + "start": Object { + "character": 20, + "line": 2, + }, + }, + }, + "range": Object { + "end": Object { + "character": 28, + "line": 2, + }, + "start": Object { + "character": 23, + "line": 2, + }, + }, + }, +] +`)}); + +it("returns a reasonable selection range for ill-typed modules", async () => { + openDocument(outdent` +module M = struct + let f x : int = string_of_int x +end`); + +let result = await selectionRange([Types.Position.create(1, 34)]); +expect(result).toMatchInlineSnapshot(` +Array [ + Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "range": Object { + "end": Object { + "character": 3, + "line": 2, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 2, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 2, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 3, + "line": 2, + }, + "start": Object { + "character": 11, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 4, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 4, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 4, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 10, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 20, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 20, + "line": 1, + }, + }, + }, +] +`)}); + +it("returns a reasonable selection range in the presence of syntax errors", async () => { + openDocument(outdent` +module M = struct + let f x : int = string_of_int x +ed`); + +let result = await selectionRange([Types.Position.create(1, 34)]); +expect(result).toMatchInlineSnapshot(` +Array [ + Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "parent": Object { + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 0, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 11, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 4, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 4, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 4, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 10, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 20, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 20, + "line": 1, + }, + }, + }, +] +`)}); }); From 4d3fe5df3fdf89120faf798e0ed1c33389f50412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Tue, 3 Sep 2024 17:31:02 +0200 Subject: [PATCH 2/4] selection_range: use merlin enclosing query instead of shape --- ocaml-lsp-server/src/ocaml_lsp_server.ml | 42 ++++++++---------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/ocaml-lsp-server/src/ocaml_lsp_server.ml b/ocaml-lsp-server/src/ocaml_lsp_server.ml index 2229065a6..733b1903c 100644 --- a/ocaml-lsp-server/src/ocaml_lsp_server.ml +++ b/ocaml-lsp-server/src/ocaml_lsp_server.ml @@ -387,41 +387,27 @@ let selection_range match Document.kind doc with | `Other -> Fiber.return [] | `Merlin merlin -> - let selection_range_of_shapes - (cursor_position : Position.t) - (shapes : Query_protocol.shape list) + let selection_range_of_enclosings (enclosings : Warnings.loc list) : SelectionRange.t option = - let rec ranges_of_shape parent (s : Query_protocol.shape) = - let selectionRange = - let range = Range.of_loc s.shape_loc in - { SelectionRange.range; parent } - in - match s.shape_sub with - | [] -> [ selectionRange ] - | xs -> List.concat_map xs ~f:(ranges_of_shape (Some selectionRange)) - in - (* try to find the nearest range inside first, then outside *) - let nearest_range = - let ranges = List.concat_map ~f:(ranges_of_shape None) shapes in - List.min ranges ~f:(fun r1 r2 -> - let inc (r : SelectionRange.t) = - Position.compare_inclusion cursor_position r.range - in - match inc r1, inc r2 with - | `Outside x, `Outside y -> Position.compare x y - | `Outside _, `Inside -> Gt - | `Inside, `Outside _ -> Lt - | `Inside, `Inside -> Range.compare_size r1.range r2.range) + let ranges_of_enclosing parent (enclosing : Warnings.loc) = + let range = Range.of_loc enclosing in + { SelectionRange.range; parent } in - nearest_range + List.fold_left + ~f:(fun parent enclosing -> Some (ranges_of_enclosing parent enclosing)) + ~init:None + @@ List.rev enclosings in let+ ranges = Fiber.sequential_map positions ~f:(fun x -> - let+ shapes = - Document.Merlin.dispatch_exn ~name:"shape" merlin (Shape (Position.logical x)) + let+ enclosings = + Document.Merlin.dispatch_exn + ~name:"shape" + merlin + (Enclosing (Position.logical x)) in - selection_range_of_shapes x shapes) + selection_range_of_enclosings enclosings) in List.filter_opt ranges ;; From 198a59ca8e0df40275e559ee616609460847c171 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Thu, 3 Oct 2024 15:42:08 +0200 Subject: [PATCH 3/4] test: promote duplicate removal in test --- .../textDocument-selectionRange.test.ts | 242 +++--------------- 1 file changed, 31 insertions(+), 211 deletions(-) diff --git a/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts b/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts index b83aeb728..475477c2f 100644 --- a/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts +++ b/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts @@ -55,30 +55,6 @@ Array [ "parent": Object { "parent": Object { "parent": Object { - "parent": Object { - "parent": Object { - "range": Object { - "end": Object { - "character": 17, - "line": 3, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 17, - "line": 3, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, "range": Object { "end": Object { "character": 17, @@ -170,30 +146,6 @@ Array [ "parent": Object { "parent": Object { "parent": Object { - "parent": Object { - "parent": Object { - "range": Object { - "end": Object { - "character": 3, - "line": 6, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 3, - "line": 6, - }, - "start": Object { - "character": 0, - "line": 3, - }, - }, - }, "range": Object { "end": Object { "character": 3, @@ -201,7 +153,7 @@ Array [ }, "start": Object { "character": 0, - "line": 3, + "line": 0, }, }, }, @@ -211,19 +163,19 @@ Array [ "line": 6, }, "start": Object { - "character": 11, + "character": 0, "line": 3, }, }, }, "range": Object { "end": Object { - "character": 24, - "line": 5, + "character": 3, + "line": 6, }, "start": Object { - "character": 2, - "line": 4, + "character": 11, + "line": 3, }, }, }, @@ -234,7 +186,7 @@ Array [ }, "start": Object { "character": 2, - "line": 5, + "line": 4, }, }, }, @@ -293,72 +245,36 @@ Array [ "parent": Object { "parent": Object { "parent": Object { - "parent": Object { - "parent": Object { - "parent": Object { - "range": Object { - "end": Object { - "character": 4, - "line": 3, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 4, - "line": 3, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 4, - "line": 3, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, "range": Object { "end": Object { "character": 4, "line": 3, }, "start": Object { - "character": 11, + "character": 0, "line": 0, }, }, }, "range": Object { "end": Object { - "character": 3, + "character": 4, "line": 3, }, "start": Object { - "character": 21, + "character": 11, "line": 0, }, }, }, "range": Object { "end": Object { - "character": 39, - "line": 2, + "character": 3, + "line": 3, }, "start": Object { - "character": 2, - "line": 1, + "character": 21, + "line": 0, }, }, }, @@ -369,7 +285,7 @@ Array [ }, "start": Object { "character": 2, - "line": 2, + "line": 1, }, }, }, @@ -446,73 +362,25 @@ Array [ "parent": Object { "parent": Object { "parent": Object { - "parent": Object { - "parent": Object { - "parent": Object { - "parent": Object { - "range": Object { - "end": Object { - "character": 3, - "line": 2, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 3, - "line": 2, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 3, - "line": 2, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 3, - "line": 2, - }, - "start": Object { - "character": 11, - "line": 0, - }, - }, - }, "range": Object { "end": Object { - "character": 35, - "line": 1, + "character": 3, + "line": 2, }, "start": Object { - "character": 4, - "line": 1, + "character": 0, + "line": 0, }, }, }, "range": Object { "end": Object { - "character": 35, - "line": 1, + "character": 3, + "line": 2, }, "start": Object { - "character": 4, - "line": 1, + "character": 11, + "line": 0, }, }, }, @@ -555,7 +423,7 @@ Array [ "line": 1, }, "start": Object { - "character": 20, + "character": 34, "line": 1, }, }, @@ -578,62 +446,14 @@ Array [ "parent": Object { "parent": Object { "parent": Object { - "parent": Object { - "parent": Object { - "parent": Object { - "parent": Object { - "range": Object { - "end": Object { - "character": 2, - "line": 2, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 2, - "line": 2, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 2, - "line": 2, - }, - "start": Object { - "character": 0, - "line": 0, - }, - }, - }, - "range": Object { - "end": Object { - "character": 2, - "line": 2, - }, - "start": Object { - "character": 11, - "line": 0, - }, - }, - }, "range": Object { "end": Object { "character": 2, "line": 2, }, "start": Object { - "character": 4, - "line": 1, + "character": 0, + "line": 0, }, }, }, @@ -643,8 +463,8 @@ Array [ "line": 2, }, "start": Object { - "character": 4, - "line": 1, + "character": 11, + "line": 0, }, }, }, @@ -683,11 +503,11 @@ Array [ }, "range": Object { "end": Object { - "character": 2, - "line": 2, + "character": 35, + "line": 1, }, "start": Object { - "character": 20, + "character": 34, "line": 1, }, }, From bcb501878f6d708cfeb9e93976db48d954417645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Tue, 1 Oct 2024 15:13:03 +0200 Subject: [PATCH 4/4] Add changelog entry for #1368 --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 737791695..341b3dc42 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ - Fix fd leak in running external processes for preprocessing (#1349) - Fix prefix parsing for completion of object methods (#1363, fixes #1358) +- Remove some duplicates in the `selectionRange` answers (#1368) # 1.19.0