diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -76,6 +76,7 @@ HeuristicResolver.cpp Hover.cpp IncludeFixer.cpp + InlayHints.cpp JSONTransport.cpp PathMapping.cpp Protocol.cpp 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}, + {"clangdInlayHintsProvider", 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. @@ -1471,6 +1477,7 @@ Bind.method("textDocument/documentLink", this, &ClangdLSPServer::onDocumentLink); Bind.method("textDocument/semanticTokens/full", this, &ClangdLSPServer::onSemanticTokens); Bind.method("textDocument/semanticTokens/full/delta", this, &ClangdLSPServer::onSemanticTokensDelta); + Bind.method("clangd/inlayHints", this, &ClangdLSPServer::onInlayHints); Bind.method("$/memoryUsage", this, &ClangdLSPServer::onMemoryUsage); if (Opts.FoldingRanges) Bind.method("textDocument/foldingRange", this, &ClangdLSPServer::onFoldingRange); 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 @@ -15,6 +15,7 @@ #include "Format.h" #include "HeaderSourceSwitch.h" #include "Headers.h" +#include "InlayHints.h" #include "ParsedAST.h" #include "Preamble.h" #include "Protocol.h" @@ -749,6 +750,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/InlayHints.h b/clang-tools-extra/clangd/InlayHints.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/InlayHints.h @@ -0,0 +1,29 @@ +//===--- InlayHints.h --------------------------------------------*- 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 +// +//===----------------------------------------------------------------------===// +// +// Support for the proposed "inlay hints" LSP feature. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INLAY_HINTS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INLAY_HINTS_H + +#include "Protocol.h" +#include + +namespace clang { +namespace clangd { +class ParsedAST; + +// Compute and return all inlay hints for a file. +std::vector inlayHints(ParsedAST &AST); + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INLAY_HINTS_H diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -0,0 +1,169 @@ +//===--- InlayHints.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 "InlayHints.h" +#include "ParsedAST.h" +#include "clang/AST/DeclarationName.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/RecursiveASTVisitor.h" + +namespace clang { +namespace clangd { + +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 constructor calls that don't look like a function call with + // an argument list, by checking the validity of getParenOrBraceRange(). + // Also weed out std::initializer_list constructors as there are no names + // for the individual arguments. + if (E->getParenOrBraceRange().isValid() && + !E->isStdInitListInitialization()) { + 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, + llvm::ArrayRef Args) { + if (Args.size() == 0 || !Callee) + return; + + if (Callee->getNumParams() < Args.size()) + return; + + std::vector ParameterNames = + chooseParameterNames(Callee, Args.size()); + + for (size_t I = 0; I < Args.size(); ++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(), 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 chooseParameterNames(const FunctionDecl *Callee, + size_t ArgCount) { + // The current strategy here is to use all the parameter names from the + // canonical declaration, unless they're all empty, in which case we + // use all the parameter names from the definition (in present in the + // translation unit). + // We could try a bit harder, e.g.: + // - try all re-declarations, not just canonical + definition + // - fall back arg-by-arg rather than wholesale + + std::vector ParameterNames = + getParameterNamesForDecl(Callee, ArgCount); + + if (llvm::all_of(ParameterNames, std::mem_fn(&std::string::empty))) { + if (const FunctionDecl *Def = Callee->getDefinition()) { + ParameterNames = getParameterNamesForDecl(Def, ArgCount); + } + } + assert(ParameterNames.size() == ArgCount); + + // Standard library functions often have parameter names that start + // with underscores, which makes the hints noisy, so strip them out. + for (auto &Name : ParameterNames) + stripLeadingUnderscores(Name); + + return ParameterNames; + } + + static void stripLeadingUnderscores(std::string &Name) { + while (!Name.empty() && Name[0] == '_') + Name = Name.substr(1); + } + + std::vector + getParameterNamesForDecl(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, 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())}, + 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; +} + +} // namespace clangd +} // namespace clang 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,48 @@ }; 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 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, + + /// Other ideas for hints that are not currently implemented: + /// + /// * Type hints, showing deduced types. + /// * Chaining hints, showing the types of intermediate expressions + /// in a chain of function calls. + /// * Hints indicating implicit conversions or implicit constructor calls. +}; +llvm::json::Value toJSON(InlayHintKind); + +/// An annotation to be displayed inline next to a range of source code. +struct InlayHint { + /// 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. + Range range; + + /// 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,25 @@ 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"; + } + llvm_unreachable("Unknown clang.clangd.InlayHintKind"); +} + +llvm::json::Value toJSON(const InlayHint &H) { + return llvm::json::Object{ + {"range", H.range}, {"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.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" @@ -1980,5 +1981,6 @@ AST.getHeuristicResolver()); 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 @@ -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/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,237 @@ +//===-- 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 "InlayHints.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()); + TU.ExtraArgs.push_back("-std=c++11"); + 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, ConstructorStdInitList) { + // Do not show hints for std::initializer_list constructors. + // FIXME: TestTU doesn't seem to be able to resolve standard library + // includes like for some reason. + assertParameterHints(R"cpp( + #include + struct S { + S(std::initializer_list param); + }; + void bar() { + S obj{42, 43}; + } + )cpp"); +} + +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, LeadingUnderscore) { + assertParameterHints(R"cpp( + void foo(int p1, int _p2, int __p3); + void bar() { + foo($p1[[41]], $p2[[42]], $p3[[43]]); + } + )cpp", + ExpectedHint{"p1: ", "p1"}, ExpectedHint{"p2: ", "p2"}, + ExpectedHint{"p3: ", "p3"}); +} + +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"); +} + +// FIXME: Tests to write and get working: +// - Variadic function (don't show duplicate hints for variadic args) +// - C-style varargs function (do show hints for non-vararg args) +// - Improved macro behaviour + +} // namespace +} // namespace clangd +} // namespace clang