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 @@ -145,6 +145,7 @@ void onCallHierarchyOutgoingCalls( const CallHierarchyOutgoingCallsParams &, Callback>); + void onInlayHints(const InlayHintsParams &, Callback>); void onChangeConfiguration(const DidChangeConfigurationParams &); void onSymbolInfo(const TextDocumentPositionParams &, Callback>); 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 @@ -571,6 +571,7 @@ {"referencesProvider", true}, {"astProvider", true}, // clangd extension {"typeHierarchyProvider", true}, + {"inlayHintsProvider", true}, {"memoryUsageProvider", true}, // clangd extension {"compilationDatabase", // clangd extension llvm::json::Object{{"automaticReload", true}}}, @@ -1208,6 +1209,11 @@ Reply(std::vector{}); } +void ClangdLSPServer::onInlayHints(const InlayHintsParams &Params, + Callback> Reply) { + Server->inlayHints(Params.textDocument.uri.file(), std::move(Reply)); +} + void ClangdLSPServer::applyConfiguration( const ConfigurationSettings &Settings) { // Per-file update to the compilation database. @@ -1469,6 +1475,7 @@ Bind.method("callHierarchy/outgoingCalls", this, &ClangdLSPServer::onCallHierarchyOutgoingCalls); Bind.method("textDocument/selectionRange", this, &ClangdLSPServer::onSelectionRange); Bind.method("textDocument/documentLink", this, &ClangdLSPServer::onDocumentLink); + Bind.method("textDocument/inlayHints", this, &ClangdLSPServer::onInlayHints); Bind.method("textDocument/semanticTokens/full", this, &ClangdLSPServer::onSemanticTokens); Bind.method("textDocument/semanticTokens/full/delta", this, &ClangdLSPServer::onSemanticTokensDelta); Bind.method("$/memoryUsage", this, &ClangdLSPServer::onMemoryUsage); 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 @@ -261,6 +261,9 @@ void incomingCalls(const CallHierarchyItem &Item, Callback>); + /// Resolve inlay hints for a given document. + void inlayHints(PathRef File, Callback>); + /// Retrieve the top symbols from the workspace matching a query. void workspaceSymbols(StringRef Query, int Limit, Callback> CB); 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 @@ -749,6 +749,17 @@ }); } +void ClangdServer::inlayHints(PathRef File, + Callback> CB) { + auto Action = [File = File.str(), + CB = std::move(CB)](Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + CB(clangd::inlayHints(InpAST->AST)); + }; + WorkScheduler->runWithAST("InlayHints", File, std::move(Action)); +} + void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { // FIXME: Do nothing for now. This will be used for indexing and potentially // invalidating other caches. 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 @@ -1478,6 +1478,68 @@ }; llvm::json::Value toJSON(const CallHierarchyOutgoingCall &); +/// The parameter of a `textDocument/inlayHints` request. +struct InlayHintsParams { + /// The text document for which inlay hints are requested. + TextDocumentIdentifier textDocument; +}; +bool fromJSON(const llvm::json::Value &, InlayHintsParams &, llvm::json::Path); + +/// A set of predefined hint kinds. +enum class InlayHintKind { + /// The hint is for type information. + /// An example of a type hint is a hint in this position: + /// auto var^ = expr; + /// which shows the deduced type of `var`. + TypeHint, + + /// The hint corresponds to parameter information. + /// An example of a parameter hint is a hint in this position: + /// func(^arg); + /// which shows the name of the corresponding parameter. + ParameterHint, + + /// The hint corresponds to method chaining. + /// An example of a chaining hint is a hint in this position: + /// a.foo()^ + /// .bar(); + /// which shows the type of the intermediate expression `a.foo()`. + ChainingHint +}; +llvm::json::Value toJSON(InlayHintKind); + +/// Defines possible positions of an inlay hint relevant to the token +/// to which it applies. +enum class InlayHintPosition { + /// The inlay hint should precede the token. + Before, + + /// The inlay hint should follow the token. + After +}; +llvm::json::Value toJSON(InlayHintPosition); + +/// An annotation to be displayed inline next to a source code token. +struct InlayHint { + /// The source range of the token 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 token, rather than displaying it all the time. + Range range; + + /// The position of the hint relative to the token. + InlayHintPosition position; + + /// The type of hint. + InlayHintKind kind; + + /// The label that is displayed in the editor. + std::string label; +}; +llvm::json::Value toJSON(const InlayHint &); + struct ReferenceContext { /// Include the declaration of the current symbol. bool includeDeclaration = false; 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 @@ -1296,6 +1296,35 @@ return llvm::json::Object{{"to", C.to}, {"fromRanges", C.fromRanges}}; } +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); +} + +llvm::json::Value toJSON(InlayHintKind K) { + switch (K) { + case InlayHintKind::ParameterHint: + return "parameter"; + case InlayHintKind::TypeHint: + return "type"; + case InlayHintKind::ChainingHint: + return "chaining"; + } + llvm_unreachable("Unknown clang.clangd.InlayHintKind"); +} + +llvm::json::Value toJSON(InlayHintPosition P) { + return llvm::json::Value(static_cast(P)); +} + +llvm::json::Value toJSON(const InlayHint &H) { + return llvm::json::Object{{"range", H.range}, + {"position", H.position}, + {"kind", H.kind}, + {"label", H.label}}; +} + static const char *toString(OffsetEncoding OE) { switch (OE) { case OffsetEncoding::UTF8: diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h --- a/clang-tools-extra/clangd/XRefs.h +++ b/clang-tools-extra/clangd/XRefs.h @@ -132,6 +132,8 @@ std::vector incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index); +std::vector inlayHints(ParsedAST &AST); + /// Returns all decls that are referenced in the \p FD except local symbols. llvm::DenseSet getNonLocalDeclRefs(ParsedAST &AST, const FunctionDecl *FD); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -30,6 +30,7 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/ExternalASTSource.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtCXX.h" @@ -1963,6 +1964,130 @@ return Results; } +static bool isAnonymous(const DeclarationName &N) { + return N.isIdentifier() && !N.getAsIdentifierInfo(); +} + +class InlayHintVisitor : public RecursiveASTVisitor { +public: + InlayHintVisitor(std::vector &Results, ParsedAST &AST) + : Results(Results), AST(AST.getASTContext()) {} + + bool VisitCXXConstructExpr(CXXConstructExpr *E) { + // Weed out implicit constructor calls by checking the validity of + // getParenOrBraceRange(). (FIXME: Is there a better way to check this?) + if (E->getParenOrBraceRange().isValid()) { + processCall(E->getConstructor(), E->getArgs(), E->getNumArgs()); + } + return true; + } + + bool VisitCallExpr(CallExpr *E) { + // Do not show parameter hints for operator calls written using operator + // syntax. The resulting hints can look awkard (e.g. the operator expression + // can itself be a function argument and then we'd get two hints side by + // side). + if (isa(E)) + return true; + + processCall(dyn_cast_or_null(E->getCalleeDecl()), + E->getArgs(), E->getNumArgs()); + return true; + } + +private: + void processCall(const FunctionDecl *Callee, const Expr *const *Args, + size_t ArgCount) { + if (ArgCount == 0 || !Callee) + return; + + if (Callee->getNumParams() < ArgCount) + return; + + std::vector ParameterNames = + getParameterNames(Callee, ArgCount); + + // If all parameter names are empty, see if the function's definition is + // present in the translation unit, maybe it has names. + if (llvm::all_of(ParameterNames, std::mem_fn(&std::string::empty))) { + if (const FunctionDecl *Def = Callee->getDefinition()) { + ParameterNames = getParameterNames(Def, ArgCount); + } + } + assert(ParameterNames.size() == ArgCount); + for (size_t I = 0; I < ArgCount; ++I) { + std::string Name = ParameterNames[I]; + if (Name.empty()) + continue; + + // If the argument expression is a single name and it matches the + // parameter name exactly, omit the hint. + if (auto *DRE = getDeclRefExpr(Args[I])) { + if (!DRE->getQualifier() && Name == printName(AST, *DRE->getDecl())) + continue; + } + + addInlayHint(Args[I]->getSourceRange(), InlayHintPosition::Before, + InlayHintKind::ParameterHint, Name + ": "); + } + } + + static const DeclRefExpr *getDeclRefExpr(const Expr *E) { + // FIXME: Are there other implicit wrappers we should handle? + if (auto *ICE = dyn_cast(E)) { + E = ICE->getSubExpr(); + } + if (auto *DRE = dyn_cast(E)) { + return DRE; + } + return nullptr; + } + + std::vector getParameterNames(const FunctionDecl *Function, + size_t ArgCount) { + std::vector Result; + for (size_t I = 0; I < ArgCount; ++I) { + const ParmVarDecl *Parm = Function->getParamDecl(I); + assert(Parm); + + // Rather than showing "(anonymous)", just don't show a hint. + if (isAnonymous(Parm->getDeclName())) { + Result.emplace_back(); + continue; + } + + Result.emplace_back(printName(AST, *Parm)); + } + return Result; + } + + void addInlayHint(SourceRange R, InlayHintPosition Pos, InlayHintKind Kind, + llvm::StringRef Label) { + // Do not annotate calls in macro expansions. + if (!R.getBegin().isFileID()) + return; + auto FileRange = + toHalfOpenFileRange(AST.getSourceManager(), AST.getLangOpts(), R); + if (!FileRange) + return; + Results.push_back(InlayHint{ + Range{ + sourceLocToPosition(AST.getSourceManager(), FileRange->getBegin()), + sourceLocToPosition(AST.getSourceManager(), FileRange->getEnd())}, + Pos, Kind, Label.str()}); + } + + std::vector &Results; + ASTContext &AST; +}; + +std::vector inlayHints(ParsedAST &AST) { + std::vector Results; + InlayHintVisitor Visitor(Results, AST); + Visitor.TraverseAST(AST.getASTContext()); + return Results; +} + llvm::DenseSet getNonLocalDeclRefs(ParsedAST &AST, const FunctionDecl *FD) { if (!FD->hasBody()) @@ -1981,4 +2106,4 @@ return DeclRefs; } } // namespace clangd -} // namespace clang +} // namespace clang \ No newline at end of file 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 @@ -72,6 +72,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, # CHECK-NEXT: "implementationProvider": true, +# CHECK-NEXT: "inlayHintsProvider": true, # CHECK-NEXT: "memoryUsageProvider": true, # CHECK-NEXT: "referencesProvider": true, # CHECK-NEXT: "renameProvider": true, diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -68,6 +68,7 @@ HoverTests.cpp IndexActionTests.cpp IndexTests.cpp + InlayHintTests.cpp JSONTransportTests.cpp LoggerTests.cpp LSPBinderTests.cpp diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -0,0 +1,204 @@ +//===-- InlayHintTests.cpp -------------------------------*- C++ -*-------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +#include "Annotations.h" +#include "Protocol.h" +#include "TestTU.h" +#include "XRefs.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +using ::testing::UnorderedElementsAre; + +std::vector parameterHints(ParsedAST &AST) { + std::vector Result; + for (auto &Hint : inlayHints(AST)) { + if (Hint.kind == InlayHintKind::ParameterHint) + Result.push_back(Hint); + } + return Result; +} + +struct ExpectedHint { + std::string Label; + std::string RangeName; +}; + +MATCHER_P2(HintMatcher, Expected, Code, "") { + return arg.label == Expected.Label && + arg.range == Code.range(Expected.RangeName); +} + +template +void assertParameterHints(llvm::StringRef AnnotatedSource, + ExpectedHints... Expected) { + Annotations Source(AnnotatedSource); + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + + EXPECT_THAT(parameterHints(AST), + UnorderedElementsAre(HintMatcher(Expected, Source)...)); +} + +TEST(ParameterHints, Smoke) { + assertParameterHints(R"cpp( + void foo(int param); + void bar() { + foo($param[[42]]); + } + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, NoName) { + // No hint for anonymous parameter. + assertParameterHints(R"cpp( + void foo(int); + void bar() { + foo(42); + } + )cpp"); +} + +TEST(ParameterHints, NameInDefinition) { + // Parameter name picked up from definition if necessary. + assertParameterHints(R"cpp( + void foo(int); + void bar() { + foo($param[[42]]); + } + void foo(int param) {}; + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, NameMismatch) { + // Prefer name from declaration. + assertParameterHints(R"cpp( + void foo(int good); + void bar() { + foo($good[[42]]); + } + void foo(int bad) {}; + )cpp", + ExpectedHint{"good: ", "good"}); +} + +TEST(ParameterHints, Operator) { + // No hint for operator call with operator syntax. + assertParameterHints(R"cpp( + struct S {}; + void operator+(S lhs, S rhs); + void bar() { + S a, b; + a + b; + } + )cpp"); +} + +TEST(ParameterHints, Macro) { + // No hint for call in macro expansion. + assertParameterHints(R"cpp( + void foo(int param); + #define ExpandsToCall() foo(42) + void bar() { + ExpandsToCall(); + } + )cpp"); +} + +TEST(ParameterHints, ConstructorParens) { + assertParameterHints(R"cpp( + struct S { + S(int param); + }; + void bar() { + S obj($param[[42]]); + } + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, ConstructorBraces) { + assertParameterHints(R"cpp( + struct S { + S(int param); + }; + void bar() { + S obj{$param[[42]]}; + } + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, MemberInit) { + assertParameterHints(R"cpp( + struct S { + S(int param); + }; + struct T { + S member; + T() : member($param[[42]]) {} + }; + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, ImplicitConstructor) { + assertParameterHints(R"cpp( + struct S { + S(int param); + }; + void bar(S); + S foo() { + // Do not show hint for implicit constructor call in argument. + bar(42); + // Do not show hint for implicit constructor call in return. + return 42; + } + )cpp"); +} + +TEST(ParameterHints, ArgMatchesParam) { + assertParameterHints(R"cpp( + void foo(int param); + struct S { + static const int param = 42; + }; + void bar() { + int param = 42; + // Do not show redundant "param: param". + foo(param); + // But show it if the argument is qualified. + foo($param[[S::param]]); + } + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, DependentCall) { + // FIXME: This doesn't currently produce a hint but should. + assertParameterHints(R"cpp( + template + void foo(T param); + + template + struct S { + void bar(T par) { + foo($param[[par]]); + } + }; + )cpp"); +} + +} // namespace +} // namespace clangd +} // namespace clang