diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -1063,6 +1063,40 @@ return llvm::Error::success(); } +// Workaround for editors that have buggy handling of newlines at end of file. +// +// The editor is supposed to expose document contents over LSP as an exact +// string, with whitespace and newlines well-defined. But internally many +// editors treat text as an array of lines, and there can be ambiguity over +// whether the last line ends with a newline or not. +// +// This confusion can lead to incorrect edits being sent. Failing to apply them +// is catastrophic: we're desynced, LSP has no mechanism to get back in sync. +// We apply a heuristic to avoid this state. +// +// If our current view of an N-line file does *not* end in a newline, but the +// editor refers to the start of the next line (an impossible location), then +// we silently add a newline to make this valid. +// We will still validate that the rangeLength is correct, *including* the +// inferred newline. +// +// See https://github.com/neovim/neovim/issues/17085 +static void inferFinalNewline(llvm::Expected &Err, + std::string &Contents, const Position &Pos) { + if (Err) + return; + if (!Contents.empty() && Contents.back() == '\n') + return; + if (Pos.character != 0) + return; + if (Pos.line != llvm::count(Contents, '\n') + 1) + return; + log("Editor sent invalid change coordinates, inferring newline at EOF"); + Contents.push_back('\n'); + consumeError(Err.takeError()); + Err = Contents.size(); +} + llvm::Error applyChange(std::string &Contents, const TextDocumentContentChangeEvent &Change) { if (!Change.range) { @@ -1072,11 +1106,13 @@ const Position &Start = Change.range->start; llvm::Expected StartIndex = positionToOffset(Contents, Start, false); + inferFinalNewline(StartIndex, Contents, Start); if (!StartIndex) return StartIndex.takeError(); const Position &End = Change.range->end; llvm::Expected EndIndex = positionToOffset(Contents, End, false); + inferFinalNewline(EndIndex, Contents, End); if (!EndIndex) return EndIndex.takeError(); diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -954,6 +954,44 @@ "the computed range length (2).")); } +// Test that we correct observed buggy edits from Neovim. +TEST(ApplyEditsTets, BuggyNeovimEdits) { + TextDocumentContentChangeEvent Change; + Change.range.emplace(); + + // https://github.com/neovim/neovim/issues/17085 + // Adding a blank line after a (missing) newline + std::string Code = "a"; + Change.range->start.line = 1; + Change.range->start.character = 0; + Change.range->end.line = 1; + Change.range->start.character = 0; + Change.rangeLength = 0; + Change.text = "\n"; + EXPECT_THAT_ERROR(applyChange(Code, Change), llvm::Succeeded()); + EXPECT_EQ(Code, "a\n\n"); + + // https://github.com/neovim/neovim/issues/17085#issuecomment-1269162264 + // Replacing the (missing) newline with \n\n in an empty file. + Code = ""; + Change.range->start.line = 0; + Change.range->start.character = 0; + Change.range->end.line = 1; + Change.range->end.character = 0; + Change.rangeLength = 1; + Change.text = "\n\n"; + + EXPECT_THAT_ERROR(applyChange(Code, Change), llvm::Succeeded()); + EXPECT_EQ(Code, "\n\n"); + + // We do not apply the heuristic fixes if the rangeLength doesn't match. + Code = ""; + Change.rangeLength = 0; + EXPECT_THAT_ERROR(applyChange(Code, Change), + FailedWithMessage("Change's rangeLength (0) doesn't match " + "the computed range length (1).")); +} + TEST(ApplyEditsTest, EndBeforeStart) { std::string Code = "int main() {}\n";