[clangd] Fix unicode handling, using UTF-16 where LSP requires it.
Summary:
The Language Server Protocol unfortunately mandates that locations in files
be represented by line/column pairs, where the "column" is actually an index
into the UTF-16-encoded text of the line.
(This is because VSCode is written in JavaScript, which is UTF-16-native).
Internally clangd treats source files at UTF-8, the One True Encoding, and
generally deals with byte offsets (though there are exceptions).
Before this patch, conversions between offsets and LSP Position pretended
that Position.character was UTF-8 bytes, which is only true for ASCII lines.
Now we examine the text to convert correctly (but don't actually need to
transcode it, due to some nice details of the encodings).
The updated functions in SourceCode are the blessed way to interact with
the Position.character field, and anything else is likely to be wrong.
So I also updated the other accesses:
- CodeComplete needs a "clang-style" line/column, with column in utf-8 bytes.
This is now converted via Position -> offset -> clang line/column
(a new function is added to SourceCode.h for the second conversion).
- getBeginningOfIdentifier skipped backwards in UTF-16 space, which is will
behave badly when it splits a surrogate pair. Skipping backwards in UTF-8
coordinates gives the lexer a fighting chance of getting this right.
While here, I clarified(?) the logic comments, fixed a bug with identifiers
containing digits, simplified the signature slightly and added a test.
This seems likely to cause problems with editors that have the same bug, and
treat the protocol as if columns are UTF-8 bytes. But we can find and fix those.
Reviewers: hokein
Subscribers: klimek, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46035
llvm-svn: 331029
diff --git a/clang-tools-extra/unittests/clangd/SourceCodeTests.cpp b/clang-tools-extra/unittests/clangd/SourceCodeTests.cpp
index d787641..57276af 100644
--- a/clang-tools-extra/unittests/clangd/SourceCodeTests.cpp
+++ b/clang-tools-extra/unittests/clangd/SourceCodeTests.cpp
@@ -24,9 +24,10 @@
return arg.line == Line && arg.character == Col;
}
+// The = → 🡆 below are ASCII (1 byte), BMP (3 bytes), and astral (4 bytes).
const char File[] = R"(0:0 = 0
-1:0 = 8
-2:0 = 16)";
+1:0 → 8
+2:0 🡆 18)";
/// A helper to make tests easier to read.
Position position(int line, int character) {
@@ -66,25 +67,31 @@
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3), false),
HasValue(11));
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 6)),
- HasValue(14)); // last character
+ HasValue(16)); // last character
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 7)),
- HasValue(15)); // the newline itself
+ HasValue(17)); // the newline itself
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8)),
- HasValue(15)); // out of range
+ HasValue(17)); // out of range
EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8), false),
Failed()); // out of range
// last line
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, -1)),
Failed()); // out of range
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 0)),
- HasValue(16)); // first character
+ HasValue(18)); // first character
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 3)),
- HasValue(19)); // middle character
- EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 7)),
- HasValue(23)); // last character
+ HasValue(21)); // middle character
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 5), false),
+ Failed()); // middle of surrogate pair
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 5)),
+ HasValue(26)); // middle of surrogate pair
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 6), false),
+ HasValue(26)); // end of surrogate pair
EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 8)),
- HasValue(24)); // EOF
- EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 9), false),
+ HasValue(28)); // last character
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 9)),
+ HasValue(29)); // EOF
+ EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 10), false),
Failed()); // out of range
// line out of bounds
EXPECT_THAT_EXPECTED(positionToOffset(File, position(3, 0)), Failed());
@@ -97,14 +104,19 @@
EXPECT_THAT(offsetToPosition(File, 6), Pos(0, 6)) << "end of first line";
EXPECT_THAT(offsetToPosition(File, 7), Pos(0, 7)) << "first newline";
EXPECT_THAT(offsetToPosition(File, 8), Pos(1, 0)) << "start of second line";
- EXPECT_THAT(offsetToPosition(File, 11), Pos(1, 3)) << "in second line";
- EXPECT_THAT(offsetToPosition(File, 14), Pos(1, 6)) << "end of second line";
- EXPECT_THAT(offsetToPosition(File, 15), Pos(1, 7)) << "second newline";
- EXPECT_THAT(offsetToPosition(File, 16), Pos(2, 0)) << "start of last line";
- EXPECT_THAT(offsetToPosition(File, 19), Pos(2, 3)) << "in last line";
- EXPECT_THAT(offsetToPosition(File, 23), Pos(2, 7)) << "end of last line";
- EXPECT_THAT(offsetToPosition(File, 24), Pos(2, 8)) << "EOF";
- EXPECT_THAT(offsetToPosition(File, 25), Pos(2, 8)) << "out of bounds";
+ EXPECT_THAT(offsetToPosition(File, 12), Pos(1, 4)) << "before BMP char";
+ EXPECT_THAT(offsetToPosition(File, 13), Pos(1, 5)) << "in BMP char";
+ EXPECT_THAT(offsetToPosition(File, 15), Pos(1, 5)) << "after BMP char";
+ EXPECT_THAT(offsetToPosition(File, 16), Pos(1, 6)) << "end of second line";
+ EXPECT_THAT(offsetToPosition(File, 17), Pos(1, 7)) << "second newline";
+ EXPECT_THAT(offsetToPosition(File, 18), Pos(2, 0)) << "start of last line";
+ EXPECT_THAT(offsetToPosition(File, 21), Pos(2, 3)) << "in last line";
+ EXPECT_THAT(offsetToPosition(File, 22), Pos(2, 4)) << "before astral char";
+ EXPECT_THAT(offsetToPosition(File, 24), Pos(2, 6)) << "in astral char";
+ EXPECT_THAT(offsetToPosition(File, 26), Pos(2, 6)) << "after astral char";
+ EXPECT_THAT(offsetToPosition(File, 28), Pos(2, 8)) << "end of last line";
+ EXPECT_THAT(offsetToPosition(File, 29), Pos(2, 9)) << "EOF";
+ EXPECT_THAT(offsetToPosition(File, 30), Pos(2, 9)) << "out of bounds";
}
} // namespace