Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -190,9 +190,16 @@ llvm::Expected ClangdServer::formatRange(StringRef Code, PathRef File, Range Rng) { - size_t Begin = positionToOffset(Code, Rng.start); - size_t Len = positionToOffset(Code, Rng.end) - Begin; - return formatCode(Code, File, {tooling::Range(Begin, Len)}); + llvm::Expected Begin = positionToOffset(Code, Rng.start); + if (!Begin) + return Begin.takeError(); + + llvm::Expected End = positionToOffset(Code, Rng.end); + if (!End) + return End.takeError(); + + size_t Len = *End - *Begin; + return formatCode(Code, File, {tooling::Range(*Begin, Len)}); } llvm::Expected ClangdServer::formatFile(StringRef Code, @@ -205,11 +212,14 @@ ClangdServer::formatOnType(StringRef Code, PathRef File, Position Pos) { // Look for the previous opening brace from the character position and // format starting from there. - size_t CursorPos = positionToOffset(Code, Pos); - size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos); + llvm::Expected CursorPos = positionToOffset(Code, Pos); + if (!CursorPos) + return CursorPos.takeError(); + + size_t PreviousLBracePos = StringRef(Code).find_last_of('{', *CursorPos); if (PreviousLBracePos == StringRef::npos) - PreviousLBracePos = CursorPos; - size_t Len = CursorPos - PreviousLBracePos; + PreviousLBracePos = *CursorPos; + size_t Len = *CursorPos - PreviousLBracePos; return formatCode(Code, File, {tooling::Range(PreviousLBracePos, Len)}); } Index: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -21,8 +21,13 @@ namespace clangd { -/// Turn a [line, column] pair into an offset in Code. -size_t positionToOffset(llvm::StringRef Code, Position P); +/// Turn a [line, column] pair into an offset in Code according to the LSP +/// definition of a Position. If the character value is greater than the line +/// length, it defaults back to the line length. If the line number is greater +/// than the number of lines in the document, return an error. +/// +/// The returned value is in the range [0, Code.size()]. +llvm::Expected positionToOffset(llvm::StringRef Code, Position P); /// Turn an offset in Code into a [line, column] pair. Position offsetToPosition(llvm::StringRef Code, size_t Offset); Index: clangd/SourceCode.cpp =================================================================== --- clangd/SourceCode.cpp +++ clangd/SourceCode.cpp @@ -9,23 +9,39 @@ #include "SourceCode.h" #include "clang/Basic/SourceManager.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/Errc.h" namespace clang { namespace clangd { using namespace llvm; -size_t positionToOffset(StringRef Code, Position P) { +llvm::Expected positionToOffset(StringRef Code, Position P) { + auto Err = [] (const Twine &S) { + return llvm::make_error(S, llvm::errc::invalid_argument); + }; + if (P.line < 0) - return 0; + return Err(llvm::formatv("Line value can't be negative ({0})", P.line)); + + if (P.character < 0) + return Err(llvm::formatv("Character value can't be negative ({0})", P.character)); + size_t StartOfLine = 0; + size_t NextNL = Code.find('\n', StartOfLine); for (int I = 0; I != P.line; ++I) { - size_t NextNL = Code.find('\n', StartOfLine); if (NextNL == StringRef::npos) - return Code.size(); + return Err(llvm::formatv("Line value is out of range ({0})", P.line)); StartOfLine = NextNL + 1; + NextNL = Code.find('\n', StartOfLine); } + + if (NextNL == StringRef::npos) { + NextNL = Code.size(); + } + // FIXME: officially P.character counts UTF-16 code units, not UTF-8 bytes! - return std::min(Code.size(), StartOfLine + std::max(0, P.character)); + return std::min(NextNL, StartOfLine + P.character); } Position offsetToPosition(StringRef Code, size_t Offset) { Index: unittests/clangd/Annotations.cpp =================================================================== --- unittests/clangd/Annotations.cpp +++ unittests/clangd/Annotations.cpp @@ -86,7 +86,13 @@ std::pair Annotations::offsetRange(llvm::StringRef Name) const { auto R = range(Name); - return {positionToOffset(Code, R.start), positionToOffset(Code, R.end)}; + llvm::Expected Start = positionToOffset(Code, R.start); + llvm::Expected End = positionToOffset(Code, R.end); + + assert(Start); + assert(End); + + return {*Start, *End}; } } // namespace clangd Index: unittests/clangd/SourceCodeTests.cpp =================================================================== --- unittests/clangd/SourceCodeTests.cpp +++ unittests/clangd/SourceCodeTests.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "SourceCode.h" #include "llvm/Support/raw_os_ostream.h" +#include "llvm/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -31,32 +32,58 @@ return Pos; } +/// Return true if the return value of positionToOffset is equal to ExpectedOffset. +bool positionToOffsetCheck(StringRef Code, Position P, size_t ExpectedOffset) +{ + llvm::Expected Result = positionToOffset(Code, P); + assert(Result); + return *Result == ExpectedOffset; +} + +void ignoreError(llvm::Error Err) { + handleAllErrors(std::move(Err), [](const llvm::ErrorInfoBase &) {}); +} + +/// Return true if the call to positionToOffset generates an error. +bool positionToOffsetFails(StringRef Code, Position P) +{ + llvm::Expected Result = positionToOffset(Code, P); + + bool HasError = !Result; + + if (HasError) + ignoreError(Result.takeError()); + + return HasError; +} + TEST(SourceCodeTests, PositionToOffset) { // line out of bounds - EXPECT_EQ(0u, positionToOffset(File, position(-1, 2))); + EXPECT_TRUE(positionToOffsetFails(File, position(-1, 2))); // first line - EXPECT_EQ(0u, positionToOffset(File, position(0, -1))); // out of range - EXPECT_EQ(0u, positionToOffset(File, position(0, 0))); // first character - EXPECT_EQ(3u, positionToOffset(File, position(0, 3))); // middle character - EXPECT_EQ(6u, positionToOffset(File, position(0, 6))); // last character - EXPECT_EQ(7u, positionToOffset(File, position(0, 7))); // the newline itself - EXPECT_EQ(8u, positionToOffset(File, position(0, 8))); // out of range + EXPECT_TRUE(positionToOffsetFails(File, position(0, -1))); // out of range + EXPECT_TRUE(positionToOffsetCheck(File, position(0, 0), 0)); // first character + EXPECT_TRUE(positionToOffsetCheck(File, position(0, 3), 3)); // middle character + EXPECT_TRUE(positionToOffsetCheck(File, position(0, 6), 6)); // last character + EXPECT_TRUE(positionToOffsetCheck(File, position(0, 7), 7)); // the newline itself + EXPECT_TRUE(positionToOffsetCheck(File, position(0, 8), 7)); // out of range // middle line - EXPECT_EQ(8u, positionToOffset(File, position(1, -1))); // out of range - EXPECT_EQ(8u, positionToOffset(File, position(1, 0))); // first character - EXPECT_EQ(11u, positionToOffset(File, position(1, 3))); // middle character - EXPECT_EQ(14u, positionToOffset(File, position(1, 6))); // last character - EXPECT_EQ(15u, positionToOffset(File, position(1, 7))); // the newline itself - EXPECT_EQ(16u, positionToOffset(File, position(1, 8))); // out of range + EXPECT_TRUE(positionToOffsetFails(File, position(1, -1))); // out of range + EXPECT_TRUE(positionToOffsetCheck(File, position(1, 0), 8)); // first character + EXPECT_TRUE(positionToOffsetCheck(File, position(1, 3), 11)); // middle character + EXPECT_TRUE(positionToOffsetCheck(File, position(1, 6), 14)); // last character + EXPECT_TRUE(positionToOffsetCheck(File, position(1, 7), 15)); // the newline itself + EXPECT_TRUE(positionToOffsetCheck(File, position(1, 8), 15)); // out of range // last line - EXPECT_EQ(16u, positionToOffset(File, position(2, -1))); // out of range - EXPECT_EQ(16u, positionToOffset(File, position(2, 0))); // first character - EXPECT_EQ(19u, positionToOffset(File, position(2, 3))); // middle character - EXPECT_EQ(23u, positionToOffset(File, position(2, 7))); // last character - EXPECT_EQ(24u, positionToOffset(File, position(2, 8))); // EOF - EXPECT_EQ(24u, positionToOffset(File, position(2, 9))); // out of range + EXPECT_TRUE(positionToOffsetFails(File, position(2, -1))); // out of range + EXPECT_TRUE(positionToOffsetCheck(File, position(2, 0), 16)); // first character + EXPECT_TRUE(positionToOffsetCheck(File, position(2, 3), 19)); // middle character + EXPECT_TRUE(positionToOffsetCheck(File, position(2, 7), 23)); // last character + EXPECT_TRUE(positionToOffsetCheck(File, position(2, 8), 24)); // EOF + EXPECT_TRUE(positionToOffsetCheck(File, position(2, 8), 24)); // out of range // line out of bounds - EXPECT_EQ(24u, positionToOffset(File, position(3, 1))); + EXPECT_TRUE(positionToOffsetFails(File, position(3, 0))); + EXPECT_TRUE(positionToOffsetFails(File, position(3, 1))); } TEST(SourceCodeTests, OffsetToPosition) {