[clangd] Support go-to-definition on type hints. The protocol part #85497
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is in preparation for implementing go-to-definition support on type inlay hints, switching the label field within the InlayHint protocol to an array of InlayHintLabelPart.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: Younan Zhang (zyn0217) ChangesThis is in preparation for implementing go-to-definition support on type inlay hints, switching the label field within the InlayHint protocol to an array of InlayHintLabelPart. Full diff: https://github.com/llvm/llvm-project/pull/85497.diff 7 Files Affected:
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index f29dadde2b86d5..7fd599d4e1a0b0 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1390,7 +1390,7 @@ void ClangdLSPServer::onClangdInlayHints(const InlayHintsParams &Params,
// Extension doesn't have paddingLeft/Right so adjust the label
// accordingly.
{"label",
- ((Hint.paddingLeft ? " " : "") + llvm::StringRef(Hint.label) +
+ ((Hint.paddingLeft ? " " : "") + llvm::StringRef(Hint.joinLabels()) +
(Hint.paddingRight ? " " : ""))
.str()},
});
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index a0ebc631ef8285..cd4f1931b3ce1d 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -977,8 +977,9 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return;
bool PadLeft = Prefix.consume_front(" ");
bool PadRight = Suffix.consume_back(" ");
- Results.push_back(InlayHint{LSPPos, (Prefix + Label + Suffix).str(), Kind,
- PadLeft, PadRight, LSPRange});
+ Results.push_back(InlayHint{LSPPos,
+ /*label=*/{(Prefix + Label + Suffix).str()},
+ Kind, PadLeft, PadRight, LSPRange});
}
// Get the range of the main file that *exactly* corresponds to R.
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index 8aa18bb0058abe..b1e29953337256 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1483,9 +1483,18 @@ llvm::json::Value toJSON(const InlayHintKind &Kind) {
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
}
+namespace {
+
+llvm::json::Array toJSON(const std::vector<InlayHintLabelPart> &Labels) {
+ return llvm::json::Array{
+ llvm::map_range(Labels, [](auto &Label) { return toJSON(Label); })};
+}
+
+} // namespace
+
llvm::json::Value toJSON(const InlayHint &H) {
llvm::json::Object Result{{"position", H.position},
- {"label", H.label},
+ {"label", toJSON(H.label)},
{"paddingLeft", H.paddingLeft},
{"paddingRight", H.paddingRight}};
auto K = toJSON(H.kind);
@@ -1501,6 +1510,10 @@ bool operator<(const InlayHint &A, const InlayHint &B) {
return std::tie(A.position, A.range, A.kind, A.label) <
std::tie(B.position, B.range, B.kind, B.label);
}
+std::string InlayHint::joinLabels() const {
+ return llvm::join(llvm::map_range(label, [](auto &L) { return L.value; }),
+ "");
+}
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, InlayHintKind Kind) {
auto ToString = [](InlayHintKind K) {
@@ -1519,6 +1532,31 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, InlayHintKind Kind) {
return OS << ToString(Kind);
}
+llvm::json::Value toJSON(const InlayHintLabelPart &L) {
+ llvm::json::Object Result{{"value", L.value}};
+ if (L.tooltip)
+ Result["tooltip"] = *L.tooltip;
+ if (L.location)
+ Result["location"] = *L.location;
+ return Result;
+}
+
+bool operator==(const InlayHintLabelPart &LHS, const InlayHintLabelPart &RHS) {
+ return std::tie(LHS.value, LHS.location) == std::tie(RHS.value, RHS.location);
+}
+
+bool operator<(const InlayHintLabelPart &LHS, const InlayHintLabelPart &RHS) {
+ return std::tie(LHS.value, LHS.location) < std::tie(RHS.value, RHS.location);
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const InlayHintLabelPart &L) {
+ OS << L.value;
+ if (L.location)
+ OS << " (" << L.location << ")";
+ return OS;
+}
+
static const char *toString(OffsetEncoding OE) {
switch (OE) {
case OffsetEncoding::UTF8:
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 358d4c6feaf87d..6fd6b9955bfb91 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1689,6 +1689,42 @@ enum class InlayHintKind {
};
llvm::json::Value toJSON(const InlayHintKind &);
+/// An inlay hint label part allows for interactive and composite labels
+/// of inlay hints.
+struct InlayHintLabelPart {
+
+ InlayHintLabelPart(std::string value,
+ std::optional<Location> location = std::nullopt)
+ : value(std::move(value)), location(std::move(location)) {}
+
+ /// The value of this label part.
+ std::string value;
+
+ /// The tooltip text when you hover over this label part. Depending on
+ /// the client capability `inlayHint.resolveSupport`, clients might resolve
+ /// this property late using the resolve request.
+ std::optional<MarkupContent> tooltip;
+
+ /// An optional source code location that represents this
+ /// label part.
+ ///
+ /// The editor will use this location for the hover and for code navigation
+ /// features: This part will become a clickable link that resolves to the
+ /// definition of the symbol at the given location (not necessarily the
+ /// location itself), it shows the hover that shows at the given location,
+ /// and it shows a context menu with further code navigation commands.
+ ///
+ /// Depending on the client capability `inlayHint.resolveSupport` clients
+ /// might resolve this property late using the resolve request.
+ std::optional<Location> location;
+
+ /// The command field is not used for now, hence omitted.
+};
+llvm::json::Value toJSON(const InlayHintLabelPart &);
+bool operator==(const InlayHintLabelPart &, const InlayHintLabelPart &);
+bool operator<(const InlayHintLabelPart &, const InlayHintLabelPart &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const InlayHintLabelPart &);
+
/// Inlay hint information.
struct InlayHint {
/// The position of this hint.
@@ -1698,7 +1734,7 @@ struct InlayHint {
/// InlayHintLabelPart label parts.
///
/// *Note* that neither the string nor the label part can be empty.
- std::string label;
+ std::vector<InlayHintLabelPart> label;
/// The kind of this hint. Can be omitted in which case the client should fall
/// back to a reasonable default.
@@ -1724,6 +1760,9 @@ struct InlayHint {
/// The range allows clients more flexibility of when/how to display the hint.
/// This is an (unserialized) clangd extension.
Range range;
+
+ /// Join the label[].value together.
+ std::string joinLabels() const;
};
llvm::json::Value toJSON(const InlayHint &);
bool operator==(const InlayHint &, const InlayHint &);
diff --git a/clang-tools-extra/clangd/test/inlayHints.test b/clang-tools-extra/clangd/test/inlayHints.test
index 8f302dd17a5494..e5b3c0fb0b960a 100644
--- a/clang-tools-extra/clangd/test/inlayHints.test
+++ b/clang-tools-extra/clangd/test/inlayHints.test
@@ -51,7 +51,11 @@
# CHECK-NEXT: "result": [
# CHECK-NEXT: {
# CHECK-NEXT: "kind": 2,
-# CHECK-NEXT: "label": "bar:",
+# CHECK-NEXT: "label": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "value": "bar:"
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
# CHECK-NEXT: "paddingLeft": false,
# CHECK-NEXT: "paddingRight": true,
# CHECK-NEXT: "position": {
diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index b5c4d145619df3..a4d53b7fc91321 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -367,7 +367,13 @@ class Checker {
auto Hints = inlayHints(*AST, LineRange);
for (const auto &Hint : Hints) {
- vlog(" {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
+ vlog(" {0} {1} [{2}]", Hint.kind, Hint.position, [&] {
+ return llvm::join(llvm::map_range(Hint.label,
+ [&](auto &L) {
+ return llvm::formatv("{{{0}}", L);
+ }),
+ ", ");
+ }());
}
}
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 0fff0dfca6c9b8..5b1531eb2fa60b 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -25,7 +25,7 @@ namespace clangd {
llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
const InlayHint &Hint) {
- return Stream << Hint.label << "@" << Hint.range;
+ return Stream << Hint.joinLabels() << "@" << Hint.range;
}
namespace {
@@ -57,10 +57,11 @@ struct ExpectedHint {
MATCHER_P2(HintMatcher, Expected, Code, llvm::to_string(Expected)) {
llvm::StringRef ExpectedView(Expected.Label);
- if (arg.label != ExpectedView.trim(" ") ||
+ std::string ResultLabel = arg.joinLabels();
+ if (ResultLabel != ExpectedView.trim(" ") ||
arg.paddingLeft != ExpectedView.starts_with(" ") ||
arg.paddingRight != ExpectedView.ends_with(" ")) {
- *result_listener << "label is '" << arg.label << "'";
+ *result_listener << "label is '" << ResultLabel << "'";
return false;
}
if (arg.range != Code.range(Expected.RangeName)) {
@@ -72,7 +73,7 @@ MATCHER_P2(HintMatcher, Expected, Code, llvm::to_string(Expected)) {
return true;
}
-MATCHER_P(labelIs, Label, "") { return arg.label == Label; }
+MATCHER_P(labelIs, Label, "") { return arg.joinLabels() == Label; }
Config noHintsConfig() {
Config C;
|
| llvm::json::Value toJSON(const InlayHint &H) { | ||
| llvm::json::Object Result{{"position", H.position}, | ||
| {"label", H.label}, | ||
| {"label", toJSON(H.label)}, |
There was a problem hiding this comment.
Is this needed? Looking at the code for serializing other vectors in this file, the conversion to llvm::json::Array Just Works.
There was a problem hiding this comment.
Ah, good catch! I didn't realize there's such an overload previously...
| vlog(" {0} {1} [{2}]", Hint.kind, Hint.position, [&] { | ||
| return llvm::join(llvm::map_range(Hint.label, | ||
| [&](auto &L) { | ||
| return llvm::formatv("{{{0}}", L); |
There was a problem hiding this comment.
There are three { in this format string, I assume that is a typo
There was a problem hiding this comment.
I admit this is quite strange, but the current formatv implementation doesn't require escaping }s (but does require doubling {{s for a literal {). See https://reviews.llvm.org/D83888.
There was a problem hiding this comment.
(I don't know why Sam's comment was not reflected in that patch at last -- maybe I can add that in a separate NFC patch.)
|
✅ With the latest revision this PR passed the Python code formatter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is in preparation for implementing go-to-definition support on type inlay hints, switching the label field within the InlayHint protocol to an array of InlayHintLabelPart.