[clangd] Send highlighting diff beyond the end of the file.
Summary: This would make the client life (tracking the changes) easier.
Reviewers: jvikstrom
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66541
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@369884 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/clangd/ClangdLSPServer.cpp b/clangd/ClangdLSPServer.cpp
index 928bd25..ee17cb0 100644
--- a/clangd/ClangdLSPServer.cpp
+++ b/clangd/ClangdLSPServer.cpp
@@ -1195,7 +1195,7 @@
}
void ClangdLSPServer::onHighlightingsReady(
- PathRef File, std::vector<HighlightingToken> Highlightings, int NumLines) {
+ PathRef File, std::vector<HighlightingToken> Highlightings) {
std::vector<HighlightingToken> Old;
std::vector<HighlightingToken> HighlightingsCopy = Highlightings;
{
@@ -1205,8 +1205,7 @@
}
// LSP allows us to send incremental edits of highlightings. Also need to diff
// to remove highlightings from tokens that should no longer have them.
- std::vector<LineHighlightings> Diffed =
- diffHighlightings(Highlightings, Old, NumLines);
+ std::vector<LineHighlightings> Diffed = diffHighlightings(Highlightings, Old);
publishSemanticHighlighting(
{{URIForFile::canonicalize(File, /*TUPath=*/File)},
toSemanticHighlightingInformation(Diffed)});
diff --git a/clangd/ClangdLSPServer.h b/clangd/ClangdLSPServer.h
index 65aa447..4e7e042 100644
--- a/clangd/ClangdLSPServer.h
+++ b/clangd/ClangdLSPServer.h
@@ -55,9 +55,9 @@
// Implement DiagnosticsConsumer.
void onDiagnosticsReady(PathRef File, std::vector<Diag> Diagnostics) override;
void onFileUpdated(PathRef File, const TUStatus &Status) override;
- void onHighlightingsReady(PathRef File,
- std::vector<HighlightingToken> Highlightings,
- int NumLines) override;
+ void
+ onHighlightingsReady(PathRef File,
+ std::vector<HighlightingToken> Highlightings) override;
// LSP methods. Notifications have signature void(const Params&).
// Calls have signature void(const Params&, Callback<Response>).
diff --git a/clangd/ClangdServer.cpp b/clangd/ClangdServer.cpp
index 5f70264..460883b 100644
--- a/clangd/ClangdServer.cpp
+++ b/clangd/ClangdServer.cpp
@@ -73,19 +73,10 @@
if (SemanticHighlighting)
Highlightings = getSemanticHighlightings(AST);
- // FIXME: We need a better way to send the maximum line number to the
- // differ.
- // The differ needs the information about the max number of lines
- // to not send diffs that are outside the file.
- const SourceManager &SM = AST.getSourceManager();
- FileID MainFileID = SM.getMainFileID();
- int NumLines = SM.getBufferData(MainFileID).count('\n') + 1;
-
Publish([&]() {
DiagConsumer.onDiagnosticsReady(Path, std::move(Diagnostics));
if (SemanticHighlighting)
- DiagConsumer.onHighlightingsReady(Path, std::move(Highlightings),
- NumLines);
+ DiagConsumer.onHighlightingsReady(Path, std::move(Highlightings));
});
}
diff --git a/clangd/ClangdServer.h b/clangd/ClangdServer.h
index 378cea5..f6f07f5 100644
--- a/clangd/ClangdServer.h
+++ b/clangd/ClangdServer.h
@@ -56,8 +56,7 @@
/// where generated from.
virtual void
onHighlightingsReady(PathRef File,
- std::vector<HighlightingToken> Highlightings,
- int NumLines) {}
+ std::vector<HighlightingToken> Highlightings) {}
};
/// When set, used by ClangdServer to get clang-tidy options for each particular
diff --git a/clangd/SemanticHighlighting.cpp b/clangd/SemanticHighlighting.cpp
index a9bf5af..40a2296 100644
--- a/clangd/SemanticHighlighting.cpp
+++ b/clangd/SemanticHighlighting.cpp
@@ -339,9 +339,11 @@
std::vector<LineHighlightings>
diffHighlightings(ArrayRef<HighlightingToken> New,
- ArrayRef<HighlightingToken> Old, int NewMaxLine) {
- assert(std::is_sorted(New.begin(), New.end()) && "New must be a sorted vector");
- assert(std::is_sorted(Old.begin(), Old.end()) && "Old must be a sorted vector");
+ ArrayRef<HighlightingToken> Old) {
+ assert(std::is_sorted(New.begin(), New.end()) &&
+ "New must be a sorted vector");
+ assert(std::is_sorted(Old.begin(), Old.end()) &&
+ "Old must be a sorted vector");
// FIXME: There's an edge case when tokens span multiple lines. If the first
// token on the line started on a line above the current one and the rest of
@@ -371,9 +373,7 @@
return std::min(NextNew, NextOld);
};
- // If the New file has fewer lines than the Old file we don't want to send
- // highlightings beyond the end of the file.
- for (int LineNumber = 0; LineNumber < NewMaxLine;
+ for (int LineNumber = 0; NewLine.end() < NewEnd || OldLine.end() < OldEnd;
LineNumber = NextLineNumber()) {
NewLine = takeLine(New, NewLine.end(), LineNumber);
OldLine = takeLine(Old, OldLine.end(), LineNumber);
diff --git a/clangd/SemanticHighlighting.h b/clangd/SemanticHighlighting.h
index 9b2c203..8d90b33 100644
--- a/clangd/SemanticHighlighting.h
+++ b/clangd/SemanticHighlighting.h
@@ -71,15 +71,15 @@
/// Return a line-by-line diff between two highlightings.
/// - if the tokens on a line are the same in both hightlightings, this line is
/// omitted.
-/// - if a line exists in New but not in Old the tokens on this line are
+/// - if a line exists in New but not in Old, the tokens on this line are
/// emitted.
-/// - if a line does not exists in New but exists in Old an empty line is
+/// - if a line does not exist in New but exists in Old, an empty line is
/// emitted (to tell client to clear the previous highlightings on this line).
-/// \p NewMaxLine is the maximum line number from the new file.
+///
/// REQUIRED: Old and New are sorted.
std::vector<LineHighlightings>
diffHighlightings(ArrayRef<HighlightingToken> New,
- ArrayRef<HighlightingToken> Old, int NewMaxLine);
+ ArrayRef<HighlightingToken> Old);
} // namespace clangd
} // namespace clang
diff --git a/clangd/test/semantic-highlighting.test b/clangd/test/semantic-highlighting.test
index c500561..0b58713 100644
--- a/clangd/test/semantic-highlighting.test
+++ b/clangd/test/semantic-highlighting.test
@@ -92,7 +92,12 @@
{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.cpp","version":2},"contentChanges": [{"range":{"start": {"line": 0,"character": 10},"end": {"line": 1,"character": 10}},"rangeLength": 11,"text": ""}]}}
# CHECK: "method": "textDocument/semanticHighlighting",
# CHECK-NEXT: "params": {
-# CHECK-NEXT: "lines": [],
+# CHECK-NEXT: "lines": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "line": 1,
+# CHECK-NEXT: "tokens": ""
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
# CHECK-NEXT: "textDocument": {
# CHECK-NEXT: "uri": "file:///clangd-test/foo.cpp"
# CHECK-NEXT: }
diff --git a/clangd/unittests/SemanticHighlightingTests.cpp b/clangd/unittests/SemanticHighlightingTests.cpp
index 903f719..8a78264 100644
--- a/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clangd/unittests/SemanticHighlightingTests.cpp
@@ -18,6 +18,9 @@
namespace clangd {
namespace {
+MATCHER_P(LineNumber, L, "") { return arg.Line == L; }
+MATCHER(EmptyHighlightings, "") { return arg.Tokens.empty(); }
+
std::vector<HighlightingToken>
makeHighlightingTokens(llvm::ArrayRef<Range> Ranges, HighlightingKind Kind) {
std::vector<HighlightingToken> Tokens(Ranges.size());
@@ -92,9 +95,10 @@
{LineTokens.first, LineTokens.second});
std::vector<LineHighlightings> ActualDiffed =
- diffHighlightings(NewTokens, OldTokens, NewCode.count('\n'));
+ diffHighlightings(NewTokens, OldTokens);
EXPECT_THAT(ActualDiffed,
- testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting));
+ testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting))
+ << OldCode;
}
TEST(SemanticHighlighting, GetsCorrectTokens) {
@@ -463,9 +467,8 @@
std::atomic<int> Count = {0};
void onDiagnosticsReady(PathRef, std::vector<Diag>) override {}
- void onHighlightingsReady(PathRef File,
- std::vector<HighlightingToken> Highlightings,
- int NLines) override {
+ void onHighlightingsReady(
+ PathRef File, std::vector<HighlightingToken> Highlightings) override {
++Count;
}
};
@@ -574,17 +577,6 @@
R"(
$Class[[A]]
$Variable[[A]]
- $Class[[A]]
- $Variable[[A]]
- )",
- R"(
- $Class[[A]]
- $Variable[[A]]
- )"},
- {
- R"(
- $Class[[A]]
- $Variable[[A]]
)",
R"(
$Class[[A]]
@@ -608,6 +600,32 @@
checkDiffedHighlights(Test.OldCode, Test.NewCode);
}
+TEST(SemanticHighlighting, DiffBeyondTheEndOfFile) {
+ llvm::StringRef OldCode =
+ R"(
+ $Class[[A]]
+ $Variable[[A]]
+ $Class[[A]]
+ $Variable[[A]]
+ )";
+ llvm::StringRef NewCode =
+ R"(
+ $Class[[A]] // line 1
+ $Variable[[A]] // line 2
+ )";
+
+ Annotations OldTest(OldCode);
+ Annotations NewTest(NewCode);
+ std::vector<HighlightingToken> OldTokens = getExpectedTokens(OldTest);
+ std::vector<HighlightingToken> NewTokens = getExpectedTokens(NewTest);
+
+ auto ActualDiff = diffHighlightings(NewTokens, OldTokens);
+ EXPECT_THAT(ActualDiff,
+ testing::UnorderedElementsAre(
+ testing::AllOf(LineNumber(3), EmptyHighlightings()),
+ testing::AllOf(LineNumber(4), EmptyHighlightings())));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
