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 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 ;; 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..475477c2f 100644 --- a/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts +++ b/ocaml-lsp-server/test/e2e/__tests__/textDocument-selectionRange.test.ts @@ -48,6 +48,193 @@ describe("textDocument/selectionRange", () => { let result = await selectionRange([Types.Position.create(1, 17)]); expect(result).toMatchInlineSnapshot(` +Array [ + Object { + "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": 8, + "line": 0, + }, + }, + }, + "range": Object { + "end": Object { + "character": 17, + "line": 3, + }, + "start": Object { + "character": 2, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 22, + "line": 1, + }, + "start": Object { + "character": 2, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 22, + "line": 1, + }, + "start": Object { + "character": 15, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 18, + "line": 1, + }, + "start": Object { + "character": 15, + "line": 1, + }, + }, + }, +] +`); + }); + + 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 { + "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": 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": 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 { @@ -57,32 +244,128 @@ Array [ "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": 17, + "character": 4, "line": 3, }, "start": Object { - "character": 0, + "character": 11, "line": 0, }, }, }, "range": Object { "end": Object { - "character": 17, + "character": 3, "line": 3, }, "start": Object { - "character": 0, + "character": 21, "line": 0, }, }, }, "range": Object { "end": Object { - "character": 17, - "line": 3, + "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": 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 { + "range": Object { + "end": Object { + "character": 3, + "line": 2, }, "start": Object { "character": 0, @@ -92,60 +375,143 @@ Array [ }, "range": Object { "end": Object { - "character": 17, - "line": 3, + "character": 3, + "line": 2, }, "start": Object { - "character": 8, + "character": 11, "line": 0, }, }, }, "range": Object { "end": Object { - "character": 17, - "line": 3, + "character": 35, + "line": 1, }, "start": Object { - "character": 2, + "character": 4, "line": 1, }, }, }, "range": Object { "end": Object { - "character": 22, + "character": 35, "line": 1, }, "start": Object { - "character": 2, + "character": 10, "line": 1, }, }, }, "range": Object { "end": Object { - "character": 22, + "character": 35, "line": 1, }, "start": Object { - "character": 15, + "character": 20, "line": 1, }, }, }, "range": Object { "end": Object { - "character": 18, + "character": 35, "line": 1, }, "start": Object { - "character": 15, + "character": 34, "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 { + "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": 10, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 2, + "line": 2, + }, + "start": Object { + "character": 20, + "line": 1, + }, + }, + }, + "range": Object { + "end": Object { + "character": 35, + "line": 1, + }, + "start": Object { + "character": 34, + "line": 1, + }, + }, + }, +] +`)}); });