diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -63,8 +63,8 @@ return !T.hidden(); // only enable non-hidden tweaks. }; - /// Enable preview of InlayHints feature. - bool InlayHints = false; + /// Enable InlayHints feature. + bool InlayHints = true; /// Limit the number of references returned (0 means no limit). size_t ReferencesLimit = 0; diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -608,6 +608,7 @@ if (Opts.FoldingRanges) ServerCaps["foldingRangeProvider"] = true; + // FIXME: once inlayHints can be disabled in config, always advertise. if (Opts.InlayHints) ServerCaps["clangdInlayHintsProvider"] = true; @@ -1210,7 +1211,8 @@ void ClangdLSPServer::onInlayHints(const InlayHintsParams &Params, Callback> Reply) { - Server->inlayHints(Params.textDocument.uri.file(), std::move(Reply)); + Server->inlayHints(Params.textDocument.uri.file(), Params.range, + std::move(Reply)); } void ClangdLSPServer::applyConfiguration( diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -264,7 +264,8 @@ Callback>); /// Resolve inlay hints for a given document. - void inlayHints(PathRef File, Callback>); + void inlayHints(PathRef File, llvm::Optional RestrictRange, + Callback>); /// Retrieve the top symbols from the workspace matching a query. void workspaceSymbols(StringRef Query, int Limit, diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -759,12 +759,13 @@ }); } -void ClangdServer::inlayHints(PathRef File, +void ClangdServer::inlayHints(PathRef File, llvm::Optional RestrictRange, Callback> CB) { - auto Action = [CB = std::move(CB)](Expected InpAST) mutable { + auto Action = [RestrictRange(std::move(RestrictRange)), + CB = std::move(CB)](Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - CB(clangd::inlayHints(InpAST->AST)); + CB(clangd::inlayHints(InpAST->AST, std::move(RestrictRange))); }; WorkScheduler->runWithAST("InlayHints", File, std::move(Action)); } diff --git a/clang-tools-extra/clangd/InlayHints.h b/clang-tools-extra/clangd/InlayHints.h --- a/clang-tools-extra/clangd/InlayHints.h +++ b/clang-tools-extra/clangd/InlayHints.h @@ -22,8 +22,10 @@ namespace clangd { class ParsedAST; -// Compute and return all inlay hints for a file. -std::vector inlayHints(ParsedAST &AST); +/// Compute and return inlay hints for a file. +/// If RestrictRange is set, return only hints whose location is in that range. +std::vector inlayHints(ParsedAST &AST, + llvm::Optional RestrictRange); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -8,20 +8,24 @@ #include "InlayHints.h" #include "HeuristicResolver.h" #include "ParsedAST.h" -#include "support/Logger.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/SourceManager.h" -#include "llvm/Support/raw_ostream.h" namespace clang { namespace clangd { +namespace { + +// For now, inlay hints are always anchored at the left or right of their range. +enum class HintSide { Left, Right }; class InlayHintVisitor : public RecursiveASTVisitor { public: - InlayHintVisitor(std::vector &Results, ParsedAST &AST) + InlayHintVisitor(std::vector &Results, ParsedAST &AST, + llvm::Optional RestrictRange) : Results(Results), AST(AST.getASTContext()), + RestrictRange(std::move(RestrictRange)), MainFileID(AST.getSourceManager().getMainFileID()), Resolver(AST.getHeuristicResolver()), TypeHintPolicy(this->AST.getPrintingPolicy()), @@ -88,7 +92,7 @@ QualType Deduced = AT->getDeducedType(); if (!Deduced.isNull()) { addTypeHint(D->getFunctionTypeLoc().getRParenLoc(), D->getReturnType(), - "-> "); + /*Prefix=*/"-> "); } } @@ -100,7 +104,7 @@ // but show hints for the individual bindings. if (auto *DD = dyn_cast(D)) { for (auto *Binding : DD->bindings()) { - addTypeHint(Binding->getLocation(), Binding->getType(), ": ", + addTypeHint(Binding->getLocation(), Binding->getType(), /*Prefix=*/": ", StructuredBindingPolicy); } return true; @@ -113,7 +117,7 @@ // (e.g. for `const auto& x = 42`, print `const int&`). // Alternatively, we could place the hint on the `auto` // (and then just print the type deduced for the `auto`). - addTypeHint(D->getLocation(), D->getType(), ": "); + addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": "); } } return true; @@ -160,8 +164,9 @@ if (!shouldHint(Args[I], Name)) continue; - addInlayHint(Args[I]->getSourceRange(), InlayHintKind::ParameterHint, - Name.str() + ": "); + addInlayHint(Args[I]->getSourceRange(), HintSide::Left, + InlayHintKind::ParameterHint, /*Prefix=*/"", Name, + /*Suffix=*/": "); } } @@ -313,20 +318,28 @@ return Result; } - void addInlayHint(SourceRange R, InlayHintKind Kind, llvm::StringRef Label) { + // We pass HintSide rather than SourceLocation because we want to ensure + // it is in the same file as the common file range. + void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind, + llvm::StringRef Prefix, llvm::StringRef Label, + llvm::StringRef Suffix) { auto FileRange = toHalfOpenFileRange(AST.getSourceManager(), AST.getLangOpts(), R); if (!FileRange) return; + Range LSPRange{ + sourceLocToPosition(AST.getSourceManager(), FileRange->getBegin()), + sourceLocToPosition(AST.getSourceManager(), FileRange->getEnd())}; + Position LSPPos = Side == HintSide::Left ? LSPRange.start : LSPRange.end; + if (RestrictRange && + (LSPPos < RestrictRange->start || !(LSPPos < RestrictRange->end))) + return; // The hint may be in a file other than the main file (for example, a header // file that was included after the preamble), do not show in that case. if (!AST.getSourceManager().isWrittenInMainFile(FileRange->getBegin())) return; - Results.push_back(InlayHint{ - Range{ - sourceLocToPosition(AST.getSourceManager(), FileRange->getBegin()), - sourceLocToPosition(AST.getSourceManager(), FileRange->getEnd())}, - Kind, Label.str()}); + Results.push_back( + InlayHint{LSPPos, LSPRange, Kind, (Prefix + Label + Suffix).str()}); } void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) { @@ -341,11 +354,13 @@ std::string TypeName = T.getAsString(Policy); if (TypeName.length() < TypeNameLimit) - addInlayHint(R, InlayHintKind::TypeHint, std::string(Prefix) + TypeName); + addInlayHint(R, HintSide::Right, InlayHintKind::TypeHint, Prefix, + TypeName, /*Suffix=*/""); } std::vector &Results; ASTContext &AST; + llvm::Optional RestrictRange; FileID MainFileID; StringRef MainFileBuf; const HeuristicResolver *Resolver; @@ -361,9 +376,12 @@ static const size_t TypeNameLimit = 32; }; -std::vector inlayHints(ParsedAST &AST) { +} // namespace + +std::vector inlayHints(ParsedAST &AST, + llvm::Optional RestrictRange) { std::vector Results; - InlayHintVisitor Visitor(Results, AST); + InlayHintVisitor Visitor(Results, AST, std::move(RestrictRange)); Visitor.TraverseAST(AST.getASTContext()); // De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1512,10 +1512,15 @@ }; llvm::json::Value toJSON(const CallHierarchyOutgoingCall &); -/// The parameter of a `textDocument/inlayHints` request. +/// The parameter of a `clangd/inlayHints` request. +/// +/// This is a clangd extension. struct InlayHintsParams { /// The text document for which inlay hints are requested. TextDocumentIdentifier textDocument; + + /// If set, requests inlay hints for only part of the document. + llvm::Optional range; }; bool fromJSON(const llvm::json::Value &, InlayHintsParams &, llvm::json::Path); @@ -1542,19 +1547,27 @@ llvm::json::Value toJSON(InlayHintKind); /// An annotation to be displayed inline next to a range of source code. +/// +/// This is a clangd extension. struct InlayHint { + /// The position between two characters where the hint should be displayed. + /// + /// For example, n parameter hint may be positioned before an argument. + Position position; + /// The range of source code to which the hint applies. - /// We provide the entire range, rather than just the endpoint - /// relevant to `position` (e.g. the start of the range for - /// InlayHintPosition::Before), to give clients the flexibility - /// to make choices like only displaying the hint while the cursor - /// is over the range, rather than displaying it all the time. + /// + /// For example, a parameter hint may have the argument as its range. + /// The range allows clients more flexibility of when/how to display the hint. Range range; - /// The type of hint. + /// The type of hint, such as a parameter hint. InlayHintKind kind; - /// The label that is displayed in the editor. + /// The label that is displayed in the editor, such as a parameter name. + /// + /// The label may contain punctuation and/or whitespace to allow it to read + /// naturally when placed inline with the code. std::string label; }; llvm::json::Value toJSON(const InlayHint &); diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -1317,7 +1317,7 @@ bool fromJSON(const llvm::json::Value &Params, InlayHintsParams &R, llvm::json::Path P) { llvm::json::ObjectMapper O(Params, P); - return O && O.map("textDocument", R.textDocument); + return O && O.map("textDocument", R.textDocument) && O.map("range", R.range); } llvm::json::Value toJSON(InlayHintKind K) { @@ -1331,16 +1331,18 @@ } llvm::json::Value toJSON(const InlayHint &H) { - return llvm::json::Object{ - {"range", H.range}, {"kind", H.kind}, {"label", H.label}}; + return llvm::json::Object{{"position", H.position}, + {"range", H.range}, + {"kind", H.kind}, + {"label", H.label}}; } bool operator==(const InlayHint &A, const InlayHint &B) { - return std::tie(A.kind, A.range, A.label) == - std::tie(B.kind, B.range, B.label); + return std::tie(A.position, A.range, A.kind, A.label) == + std::tie(B.position, B.range, B.kind, B.label); } bool operator<(const InlayHint &A, const InlayHint &B) { - return std::tie(A.kind, A.range, A.label) < - std::tie(B.kind, B.range, B.label); + return std::tie(A.position, A.range, A.kind, A.label) < + std::tie(B.position, B.range, B.kind, B.label); } static const char *toString(OffsetEncoding OE) { diff --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test --- a/clang-tools-extra/clangd/test/initialize-params.test +++ b/clang-tools-extra/clangd/test/initialize-params.test @@ -7,6 +7,7 @@ # CHECK-NEXT: "capabilities": { # CHECK-NEXT: "astProvider": true, # CHECK-NEXT: "callHierarchyProvider": true, +# CHECK-NEXT: "clangdInlayHintsProvider": true, # CHECK-NEXT: "codeActionProvider": true, # CHECK-NEXT: "compilationDatabase": { # CHECK-NEXT: "automaticReload": true diff --git a/clang-tools-extra/clangd/test/inlayHints.test b/clang-tools-extra/clangd/test/inlayHints.test new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/test/inlayHints.test @@ -0,0 +1,45 @@ +# RUN: clangd -lit-test < %s | FileCheck %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ + "uri":"test:///main.cpp", + "languageId":"cpp", + "version":1, + "text":"int foo(int bar);\nint x = foo(42);\nint y = foo(42);" +}}} +--- +{"jsonrpc":"2.0","id":1,"method":"clangd/inlayHints","params":{ + "textDocument":{"uri":"test:///main.cpp"}, + "range":{ + "start": {"line":1,"character":0}, + "end": {"line":2,"character":0} + } +}} +# CHECK: "id": 1, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "kind": "parameter", +# CHECK-NEXT: "label": "bar: ", +# CHECK-NEXT: "position": { +# CHECK-NEXT: "character": 12, +# CHECK-NEXT: "line": 1 +# CHECK-NEXT: }, +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 14, +# CHECK-NEXT: "line": 1 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 12, +# CHECK-NEXT: "line": 1 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT:} +--- +{"jsonrpc":"2.0","id":100,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} + diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -327,8 +327,14 @@ Hidden, }; -opt InlayHints{"inlay-hints", cat(Features), - desc("Enable preview of InlayHints feature"), init(false)}; +opt InlayHints{ + "inlay-hints", + cat(Features), + desc("Enable InlayHints feature"), + init(ClangdLSPServer::Options().InlayHints), + // FIXME: allow inlayHints to be disabled in Config and remove this option. + Hidden, +}; opt WorkerThreadsCount{ "j", diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -23,20 +23,23 @@ namespace { -using ::testing::UnorderedElementsAre; +using ::testing::ElementsAre; std::vector hintsOfKind(ParsedAST &AST, InlayHintKind Kind) { std::vector Result; - for (auto &Hint : inlayHints(AST)) { + for (auto &Hint : inlayHints(AST, /*RestrictRange=*/llvm::None)) { if (Hint.kind == Kind) Result.push_back(Hint); } return Result; } +enum HintSide { Left, Right }; + struct ExpectedHint { std::string Label; std::string RangeName; + HintSide Side = Left; friend std::ostream &operator<<(std::ostream &Stream, const ExpectedHint &Hint) { @@ -46,9 +49,13 @@ MATCHER_P2(HintMatcher, Expected, Code, "") { return arg.label == Expected.Label && - arg.range == Code.range(Expected.RangeName); + arg.range == Code.range(Expected.RangeName) && + arg.position == + ((Expected.Side == Left) ? arg.range.start : arg.range.end); } +MATCHER_P(labelIs, Label, "") { return arg.label == Label; } + template void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource, ExpectedHints... Expected) { @@ -58,18 +65,23 @@ auto AST = TU.build(); EXPECT_THAT(hintsOfKind(AST, Kind), - UnorderedElementsAre(HintMatcher(Expected, Source)...)); + ElementsAre(HintMatcher(Expected, Source)...)); } +// Hack to allow expression-statements operating on parameter packs in C++14. +template void ignore(T &&...) {} + template void assertParameterHints(llvm::StringRef AnnotatedSource, ExpectedHints... Expected) { + ignore(Expected.Side = Left...); assertHints(InlayHintKind::ParameterHint, AnnotatedSource, Expected...); } template void assertTypeHints(llvm::StringRef AnnotatedSource, ExpectedHints... Expected) { + ignore(Expected.Side = Right...); assertHints(InlayHintKind::TypeHint, AnnotatedSource, Expected...); } @@ -631,6 +643,18 @@ ExpectedHint{": int", "var"}); } +TEST(InlayHints, RestrictRange) { + Annotations Code(R"cpp( + auto a = false; + [[auto b = 1; + auto c = '2';]] + auto d = 3.f; + )cpp"); + auto AST = TestTU::withCode(Code.code()).build(); + EXPECT_THAT(inlayHints(AST, Code.range()), + ElementsAre(labelIs(": int"), labelIs(": char"))); +} + // FIXME: Low-hanging fruit where we could omit a type hint: // - auto x = TypeName(...); // - auto x = (TypeName) (...);