ソースを参照

Fix multiline incremental sync. (#5046)

Previously the start index was incorrect. Adding comments where I was
double-checking things along the way.
Jon Ross-Perkins 1 年間 前
コミット
dc0c2622ac

+ 23 - 22
toolchain/language_server/handle_text_document.cpp

@@ -6,6 +6,8 @@
 
 namespace Carbon::LanguageServer {
 
+// Implements `textDocument/didOpen`:
+// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didOpen
 auto HandleDidOpenTextDocument(
     Context& context, const clang::clangd::DidOpenTextDocumentParams& params)
     -> void {
@@ -26,36 +28,31 @@ auto HandleDidOpenTextDocument(
   }
 }
 
+// Returns the result of moving from `cursor_index` past `line_count` lines.
+static auto GetNthLineIndex(llvm::StringRef contents, size_t cursor_index,
+                            size_t line_count) -> size_t {
+  for ([[maybe_unused]] auto _ : llvm::seq(line_count)) {
+    const size_t newline_index = contents.find('\n', cursor_index);
+    CARBON_CHECK(newline_index != std::string::npos,
+                 "Line number greater than number of lines in the file");
+    cursor_index = newline_index + 1;
+  }
+  return cursor_index;
+}
+
 // Takes start and end positions and returns a tuple with start and end
 // offsets. Positions are based on row and column numbers in the source
 // code. We often need to know the offsets when modifying strings, so
 // this function helps us calculate the offsets. It assumes that the start
 // position comes before the end position.
-static auto PositionToIndex(const std::string& contents,
+static auto PositionToIndex(llvm::StringRef contents,
                             const clang::clangd::Position& start,
                             const clang::clangd::Position& end)
     -> std::tuple<size_t, size_t> {
-  size_t start_index = 0;
-  size_t end_index = 0;
-
-  for (auto line_number : llvm::seq(end.line)) {
-    const size_t newline_index = contents.find('\n', end_index);
-
-    CARBON_CHECK(newline_index != std::string::npos,
-                 "Line number greater than number of lines in the file");
-
-    end_index = newline_index + 1;
-
-    // This condition won't be met if start.line == end.line
-    // so we need to also check this outside the loop.
-    if (line_number == start.line) {
-      start_index = end_index;
-    }
-  }
-
-  if (start.line == end.line) {
-    start_index = end_index;
-  }
+  size_t start_index =
+      GetNthLineIndex(contents, /*cursor_index=*/0, start.line);
+  size_t end_index = GetNthLineIndex(contents, /*cursor_index=*/start_index,
+                                     end.line - start.line);
 
   start_index += start.character;
   end_index += end.character;
@@ -94,6 +91,8 @@ static auto ApplyChanges(
   }
 }
 
+// Implements `textDocument/didChange`:
+// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didChange
 auto HandleDidChangeTextDocument(
     Context& context, const clang::clangd::DidChangeTextDocumentParams& params)
     -> void {
@@ -112,6 +111,8 @@ auto HandleDidChangeTextDocument(
   }
 }
 
+// Implements `textDocument/didClose`:
+// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didClose
 auto HandleDidCloseTextDocument(
     Context& context, const clang::clangd::DidCloseTextDocumentParams& params)
     -> void {

+ 40 - 0
toolchain/language_server/testdata/text_document/incremental_sync.carbon

@@ -2,6 +2,46 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
+// Content after `didOpen`:
+//
+// ```
+// fn SecondFunCTIonNNN() {}
+//
+// fn ThirdFn() {}
+// ```
+//
+// After first `didChange`:
+//
+// ```
+// fn SecondFunction() {}
+//
+// fn ThirdFunction() {}
+// ```
+//
+// After second `didChange`:
+//
+// ```
+// fn SecondFunction() {}
+//
+// fn ThirdFunction() {}
+//
+// fn FourthFunction() {}
+// ```
+//
+// After third `didChange`:
+//
+// ```
+// fn FirstFunction() {}
+//
+// fn SecondFunction() {}
+//
+// fn ThirdFunction() {}
+//
+// fn FourthFunction() {}
+// ```
+//
+// Fourth `didChange` replaces full content.
+
 // AUTOUPDATE
 // TIP: To test this file alone, run:
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/language_server/testdata/text_document/incremental_sync.carbon

+ 183 - 0
toolchain/language_server/testdata/text_document/incremental_sync_multiline.carbon

@@ -0,0 +1,183 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// The `didChange` here deletes the full middle line.
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/language_server/testdata/text_document/incremental_sync_multiline.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/language_server/testdata/text_document/incremental_sync_multiline.carbon
+
+// --- STDIN
+[[@LSP-CALL:initialize]]
+[[@LSP-NOTIFY:textDocument/didOpen:
+  "textDocument": {"uri": "file:/test.carbon", "languageId": "carbon",
+                   "text": "fn F() {}\n// Test\nfn Bar() {}\n"}
+]]
+[[@LSP-CALL:textDocument/documentSymbol:
+  "textDocument": {"uri": "file:/test.carbon"}
+]]
+[[@LSP-NOTIFY:textDocument/didChange:
+  "textDocument": {"uri": "file:/test.carbon"},
+  "contentChanges": [
+    {"range": {"start": {"line": 1, "character": 0},
+               "end": {"line": 2, "character": 0}},
+     "text": ""}
+  ]
+]]
+[[@LSP-CALL:textDocument/documentSymbol:
+  "textDocument": {"uri": "file:/test.carbon"}
+]]
+[[@LSP-CALL:shutdown]]
+[[@LSP-NOTIFY:exit]]
+
+// --- AUTOUPDATE-SPLIT
+
+// CHECK:STDOUT: Content-Length: 146{{\r}}
+// CHECK:STDOUT: {{\r}}
+// CHECK:STDOUT: {
+// CHECK:STDOUT:   "id": 1,
+// CHECK:STDOUT:   "jsonrpc": "2.0",
+// CHECK:STDOUT:   "result": {
+// CHECK:STDOUT:     "capabilities": {
+// CHECK:STDOUT:       "documentSymbolProvider": true,
+// CHECK:STDOUT:       "textDocumentSync": 2
+// CHECK:STDOUT:     }
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }Content-Length: 144{{\r}}
+// CHECK:STDOUT: {{\r}}
+// CHECK:STDOUT: {
+// CHECK:STDOUT:   "jsonrpc": "2.0",
+// CHECK:STDOUT:   "method": "textDocument/publishDiagnostics",
+// CHECK:STDOUT:   "params": {
+// CHECK:STDOUT:     "diagnostics": [],
+// CHECK:STDOUT:     "uri": "file:///test.carbon"
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }Content-Length: 870{{\r}}
+// CHECK:STDOUT: {{\r}}
+// CHECK:STDOUT: {
+// CHECK:STDOUT:   "id": 2,
+// CHECK:STDOUT:   "jsonrpc": "2.0",
+// CHECK:STDOUT:   "result": [
+// CHECK:STDOUT:     {
+// CHECK:STDOUT:       "kind": 12,
+// CHECK:STDOUT:       "name": "F",
+// CHECK:STDOUT:       "range": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 9,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 0,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       },
+// CHECK:STDOUT:       "selectionRange": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 4,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 3,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       }
+// CHECK:STDOUT:     },
+// CHECK:STDOUT:     {
+// CHECK:STDOUT:       "kind": 12,
+// CHECK:STDOUT:       "name": "Bar",
+// CHECK:STDOUT:       "range": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 11,
+// CHECK:STDOUT:           "line": 2
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 0,
+// CHECK:STDOUT:           "line": 2
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       },
+// CHECK:STDOUT:       "selectionRange": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 6,
+// CHECK:STDOUT:           "line": 2
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 3,
+// CHECK:STDOUT:           "line": 2
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       }
+// CHECK:STDOUT:     }
+// CHECK:STDOUT:   ]
+// CHECK:STDOUT: }Content-Length: 144{{\r}}
+// CHECK:STDOUT: {{\r}}
+// CHECK:STDOUT: {
+// CHECK:STDOUT:   "jsonrpc": "2.0",
+// CHECK:STDOUT:   "method": "textDocument/publishDiagnostics",
+// CHECK:STDOUT:   "params": {
+// CHECK:STDOUT:     "diagnostics": [],
+// CHECK:STDOUT:     "uri": "file:///test.carbon"
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }Content-Length: 870{{\r}}
+// CHECK:STDOUT: {{\r}}
+// CHECK:STDOUT: {
+// CHECK:STDOUT:   "id": 3,
+// CHECK:STDOUT:   "jsonrpc": "2.0",
+// CHECK:STDOUT:   "result": [
+// CHECK:STDOUT:     {
+// CHECK:STDOUT:       "kind": 12,
+// CHECK:STDOUT:       "name": "F",
+// CHECK:STDOUT:       "range": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 9,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 0,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       },
+// CHECK:STDOUT:       "selectionRange": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 4,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 3,
+// CHECK:STDOUT:           "line": 0
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       }
+// CHECK:STDOUT:     },
+// CHECK:STDOUT:     {
+// CHECK:STDOUT:       "kind": 12,
+// CHECK:STDOUT:       "name": "Bar",
+// CHECK:STDOUT:       "range": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 11,
+// CHECK:STDOUT:           "line": 1
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 0,
+// CHECK:STDOUT:           "line": 1
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       },
+// CHECK:STDOUT:       "selectionRange": {
+// CHECK:STDOUT:         "end": {
+// CHECK:STDOUT:           "character": 6,
+// CHECK:STDOUT:           "line": 1
+// CHECK:STDOUT:         },
+// CHECK:STDOUT:         "start": {
+// CHECK:STDOUT:           "character": 3,
+// CHECK:STDOUT:           "line": 1
+// CHECK:STDOUT:         }
+// CHECK:STDOUT:       }
+// CHECK:STDOUT:     }
+// CHECK:STDOUT:   ]
+// CHECK:STDOUT: }Content-Length: 51{{\r}}
+// CHECK:STDOUT: {{\r}}
+// CHECK:STDOUT: {
+// CHECK:STDOUT:   "id": 4,
+// CHECK:STDOUT:   "jsonrpc": "2.0",
+// CHECK:STDOUT:   "result": null
+// CHECK:STDOUT: }