From b4c2ca187eeaabd9332fd399480f684ec80ed76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Thu, 21 Nov 2024 14:27:14 +0100 Subject: [PATCH 1/2] Experiment: project-wide renaming --- ocaml-lsp-server/src/import.ml | 2 + ocaml-lsp-server/src/rename.ml | 91 ++++++++++++------- .../e2e/__tests__/textDocument-rename.test.ts | 72 +++++++-------- 3 files changed, 94 insertions(+), 71 deletions(-) diff --git a/ocaml-lsp-server/src/import.ml b/ocaml-lsp-server/src/import.ml index 7e25c5f42..586f2328e 100644 --- a/ocaml-lsp-server/src/import.ml +++ b/ocaml-lsp-server/src/import.ml @@ -130,6 +130,8 @@ include struct include Uri let to_dyn t = Dyn.string (to_string t) + + module Map = Stdlib.Map.Make (Uri) end end diff --git a/ocaml-lsp-server/src/rename.ml b/ocaml-lsp-server/src/rename.ml index eb810e516..6563ea9da 100644 --- a/ocaml-lsp-server/src/rename.ml +++ b/ocaml-lsp-server/src/rename.ml @@ -8,38 +8,55 @@ let rename (state : State.t) { RenameParams.textDocument = { uri }; position; ne | `Other -> Fiber.return (WorkspaceEdit.create ()) | `Merlin merlin -> let command = - Query_protocol.Occurrences (`Ident_at (Position.logical position), `Buffer) + Query_protocol.Occurrences (`Ident_at (Position.logical position), `Renaming) in let+ locs, _desync = Document.Merlin.dispatch_exn ~name:"rename" merlin command in let version = Document.version doc in - let source = Document.source doc in + let uri = Document.uri doc in let edits = - List.map locs ~f:(fun (loc : Warnings.loc) -> + List.fold_left locs ~init:Uri.Map.empty ~f:(fun acc (loc : Warnings.loc) -> let range = Range.of_loc loc in - let make_edit () = TextEdit.create ~range ~newText:newName in - match - let occur_start_pos = - Position.of_lexical_position loc.loc_start |> Option.value_exn - in - occur_start_pos - with - | { character = 0; _ } -> make_edit () - | pos -> - let mpos = Position.logical pos in - let (`Offset index) = Msource.get_offset source mpos in - assert (index > 0) - (* [index = 0] if we pass [`Logical (1, 0)], but we handle the case - when [character = 0] in a separate matching branch *); - let source_txt = Msource.text source in - (match source_txt.[index - 1] with - | '~' (* the occurrence is a named argument *) - | '?' (* is an optional argument *) -> - let empty_range_at_occur_end = - let occur_end_pos = range.Range.end_ in - { range with start = occur_end_pos } - in - TextEdit.create ~range:empty_range_at_occur_end ~newText:(":" ^ newName) - | _ -> make_edit ())) + let edit = TextEdit.create ~range ~newText:newName in + let uri = + match loc.loc_start.pos_fname with + | "" -> uri + | path -> Uri.of_path path + in + Uri.Map.add_to_list uri edit acc) + in + let edits = + Uri.Map.mapi + (fun doc_uri edits -> + let source = + match Document_store.get_opt state.store doc_uri with + | Some doc when DocumentUri.equal doc_uri (Document.uri doc) -> + Document.source doc + | Some _ | None -> + let source_path = Uri.to_path doc_uri in + In_channel.with_open_text source_path In_channel.input_all |> Msource.make + in + List.map edits ~f:(fun (edit : TextEdit.t) -> + let start_position = edit.range.start in + match start_position with + | { character = 0; _ } -> edit + | pos -> + let mpos = Position.logical pos in + let (`Offset index) = Msource.get_offset source mpos in + assert (index > 0) + (* [index = 0] if we pass [`Logical (1, 0)], but we handle the case + when [character = 0] in a separate matching branch *); + let source_txt = Msource.text source in + (* TODO: handle record field puning *) + (match source_txt.[index - 1] with + | '~' (* the occurrence is a named argument *) + | '?' (* is an optional argument *) -> + let empty_range_at_occur_end = + let occur_end_pos = edit.range.end_ in + { edit.range with start = occur_end_pos } + in + TextEdit.create ~range:empty_range_at_occur_end ~newText:(":" ^ newName) + | _ -> edit))) + edits in let workspace_edits = let documentChanges = @@ -53,15 +70,19 @@ let rename (state : State.t) { RenameParams.textDocument = { uri }; position; ne in if documentChanges then ( - let textDocument = - OptionalVersionedTextDocumentIdentifier.create ~uri ~version () + let documentChanges = + Uri.Map.to_list edits + |> List.map ~f:(fun (uri, edits) -> + let textDocument = + OptionalVersionedTextDocumentIdentifier.create ~uri ~version () + in + let edits = List.map edits ~f:(fun e -> `TextEdit e) in + `TextDocumentEdit (TextDocumentEdit.create ~textDocument ~edits)) in - let edits = List.map edits ~f:(fun e -> `TextEdit e) in - WorkspaceEdit.create - ~documentChanges: - [ `TextDocumentEdit (TextDocumentEdit.create ~textDocument ~edits) ] - ()) - else WorkspaceEdit.create ~changes:[ uri, edits ] () + WorkspaceEdit.create ~documentChanges ()) + else ( + let changes = Uri.Map.to_list edits in + WorkspaceEdit.create ~changes ()) in workspace_edits ;; diff --git a/ocaml-lsp-server/test/e2e/__tests__/textDocument-rename.test.ts b/ocaml-lsp-server/test/e2e/__tests__/textDocument-rename.test.ts index 5b05d3a53..5f718ae6a 100644 --- a/ocaml-lsp-server/test/e2e/__tests__/textDocument-rename.test.ts +++ b/ocaml-lsp-server/test/e2e/__tests__/textDocument-rename.test.ts @@ -111,12 +111,12 @@ describe("textDocument/rename", () => { "newText": "new_num", "range": { "end": { - "character": 7, - "line": 0, + "character": 13, + "line": 1, }, "start": { - "character": 4, - "line": 0, + "character": 10, + "line": 1, }, }, }, @@ -124,12 +124,12 @@ describe("textDocument/rename", () => { "newText": "new_num", "range": { "end": { - "character": 13, - "line": 1, + "character": 7, + "line": 0, }, "start": { - "character": 10, - "line": 1, + "character": 4, + "line": 0, }, }, }, @@ -163,12 +163,12 @@ describe("textDocument/rename", () => { "newText": "new_num", "range": { "end": { - "character": 7, - "line": 0, + "character": 13, + "line": 1, }, "start": { - "character": 4, - "line": 0, + "character": 10, + "line": 1, }, }, }, @@ -176,12 +176,12 @@ describe("textDocument/rename", () => { "newText": "new_num", "range": { "end": { - "character": 13, - "line": 1, + "character": 7, + "line": 0, }, "start": { - "character": 10, - "line": 1, + "character": 4, + "line": 0, }, }, }, @@ -218,28 +218,28 @@ let () = bar ~foo "changes": { "file:///test.ml": [ { - "newText": "ident", + "newText": ":ident", "range": { "end": { - "character": 7, - "line": 0, + "character": 17, + "line": 4, }, "start": { - "character": 4, - "line": 0, + "character": 17, + "line": 4, }, }, }, { - "newText": ":ident", + "newText": "ident", "range": { "end": { - "character": 17, - "line": 4, + "character": 7, + "line": 0, }, "start": { - "character": 17, - "line": 4, + "character": 4, + "line": 0, }, }, }, @@ -272,28 +272,28 @@ ignore (bar ?foo ()) "changes": { "file:///test.ml": [ { - "newText": "sunit", + "newText": ":sunit", "range": { "end": { - "character": 7, - "line": 0, + "character": 16, + "line": 5, }, "start": { - "character": 4, - "line": 0, + "character": 16, + "line": 5, }, }, }, { - "newText": ":sunit", + "newText": "sunit", "range": { "end": { - "character": 16, - "line": 5, + "character": 7, + "line": 0, }, "start": { - "character": 16, - "line": 5, + "character": 4, + "line": 0, }, }, }, From a56fee0a6821c2343629898176cce4107dd920bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Fri, 10 Jan 2025 17:58:10 +0100 Subject: [PATCH 2/2] Add a changelog entry for #1431 --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 77c5c2f37..2d9e558ea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,9 @@ +# Unreleased + +## Features + +- Enable experimental project-wide renaming of identifiers (#1431) + # 1.21.0 ## Features