[clang-tools-extra] 2bececb - [clangd] Add provider info on symbol hover.
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 23 09:23:24 PDT 2023
Author: Viktoriia Bakalova
Date: 2023-03-23T16:21:51Z
New Revision: 2bececb8bed1f8fcd8d54dba831ceb117717bfcc
URL: https://github.com/llvm/llvm-project/commit/2bececb8bed1f8fcd8d54dba831ceb117717bfcc
DIFF: https://github.com/llvm/llvm-project/commit/2bececb8bed1f8fcd8d54dba831ceb117717bfcc.diff
LOG: [clangd] Add provider info on symbol hover.
Differential Revision: https://reviews.llvm.org/D144976
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index c5436141adbf..e240c22259f3 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -12,11 +12,16 @@
#include "CodeCompletionStrings.h"
#include "Config.h"
#include "FindTarget.h"
+#include "IncludeCleaner.h"
#include "ParsedAST.h"
#include "Selection.h"
#include "SourceCode.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
#include "index/SymbolCollector.h"
+#include "support/Logger.h"
#include "support/Markup.h"
+#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTTypeTraits.h"
@@ -43,11 +48,13 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/raw_ostream.h"
#include <optional>
#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -1084,6 +1091,49 @@ const NamedDecl *pickDeclToUse(llvm::ArrayRef<const NamedDecl *> Candidates) {
return Candidates.front();
}
+void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo &HI,
+ include_cleaner::Symbol Sym) {
+ trace::Span Tracer("Hover::maybeAddSymbolProviders");
+
+ const SourceManager &SM = AST.getSourceManager();
+ llvm::SmallVector<include_cleaner::Header> RankedProviders =
+ include_cleaner::headersForSymbol(Sym, SM, AST.getPragmaIncludes());
+ if (RankedProviders.empty())
+ return;
+
+ std::string Result;
+ include_cleaner::Includes ConvertedIncludes =
+ convertIncludes(SM, AST.getIncludeStructure().MainFileIncludes);
+ for (const auto &P : RankedProviders) {
+ if (P.kind() == include_cleaner::Header::Physical &&
+ P.physical() == SM.getFileEntryForID(SM.getMainFileID()))
+ // Main file ranked higher than any #include'd file
+ break;
+
+ // Pick the best-ranked #include'd provider
+ auto Matches = ConvertedIncludes.match(P);
+ if (!Matches.empty()) {
+ Result = Matches[0]->quote();
+ break;
+ }
+ }
+
+ if (!Result.empty()) {
+ HI.Provider = std::move(Result);
+ return;
+ }
+
+ // Pick the best-ranked non-#include'd provider
+ const auto &H = RankedProviders.front();
+ if (H.kind() == include_cleaner::Header::Physical &&
+ H.physical() == SM.getFileEntryForID(SM.getMainFileID()))
+ // Do not show main file as provider, otherwise we'll show provider info
+ // on local variables, etc.
+ return;
+
+ HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H);
+}
+
} // namespace
std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -1131,6 +1181,12 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
HighlightRange = Tok.range(SM).toCharRange(SM);
if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
HI = getHoverContents(*M, Tok, AST);
+ if (auto DefLoc = M->Info->getDefinitionLoc(); DefLoc.isValid()) {
+ include_cleaner::Macro IncludeCleanerMacro{
+ AST.getPreprocessor().getIdentifierInfo(Tok.text(SM)), DefLoc};
+ maybeAddSymbolProviders(AST, *HI,
+ include_cleaner::Symbol{IncludeCleanerMacro});
+ }
break;
}
} else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
@@ -1168,6 +1224,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
if (!HI->Value)
HI->Value = printExprValue(N, AST.getASTContext());
maybeAddCalleeArgInfo(N, *HI, PP);
+ maybeAddSymbolProviders(AST, *HI, include_cleaner::Symbol{*DeclToUse});
} else if (const Expr *E = N->ASTNode.get<Expr>()) {
HI = getHoverContents(N, E, AST, PP, Index);
} else if (const Attr *A = N->ASTNode.get<Attr>()) {
@@ -1217,6 +1274,14 @@ markup::Document HoverInfo::present() const {
assert(!Name.empty() && "hover triggered on a nameless symbol");
Header.appendCode(Name);
+ if (!Provider.empty()) {
+ markup::Paragraph &DI = Output.addParagraph();
+ DI.appendText("provided by");
+ DI.appendSpace();
+ DI.appendCode(Provider);
+ Output.addRuler();
+ }
+
// Put a linebreak after header to increase readability.
Output.addRuler();
// Print Types on their own lines to reduce chances of getting line-wrapped by
diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h
index e63ff95b400b..7ade177f89cc 100644
--- a/clang-tools-extra/clangd/Hover.h
+++ b/clang-tools-extra/clangd/Hover.h
@@ -14,6 +14,7 @@
#include "support/Markup.h"
#include "clang/Index/IndexSymbol.h"
#include <optional>
+#include <string>
namespace clang {
namespace clangd {
@@ -67,6 +68,8 @@ struct HoverInfo {
std::string LocalScope;
/// Name of the symbol, does not contain any "::".
std::string Name;
+ /// Header providing the symbol (best match). Contains ""<>.
+ std::string Provider;
std::optional<Range> SymRange;
index::SymbolKind Kind = index::SymbolKind::Unknown;
std::string Documentation;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index ee470bd8b963..ab7c05eb834c 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -136,45 +136,6 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
return true;
}
-include_cleaner::Includes
-convertIncludes(const SourceManager &SM,
- const llvm::ArrayRef<Inclusion> MainFileIncludes) {
- include_cleaner::Includes Includes;
- for (const Inclusion &Inc : MainFileIncludes) {
- include_cleaner::Include TransformedInc;
- llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
- TransformedInc.Spelled = WrittenRef.trim("\"<>");
- TransformedInc.HashLocation =
- SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
- TransformedInc.Line = Inc.HashLine + 1;
- TransformedInc.Angled = WrittenRef.starts_with("<");
- auto FE = SM.getFileManager().getFile(Inc.Resolved);
- if (!FE) {
- elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
- Inc.Resolved, FE.getError().message());
- continue;
- }
- TransformedInc.Resolved = *FE;
- Includes.add(std::move(TransformedInc));
- }
- return Includes;
-}
-
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
- include_cleaner::Header Provider) {
- if (Provider.kind() == include_cleaner::Header::Physical) {
- if (auto CanonicalPath =
- getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
- std::string SpelledHeader =
- llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
- if (!SpelledHeader.empty())
- return SpelledHeader;
- }
- }
- return include_cleaner::spellHeader(
- Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
-}
-
std::vector<include_cleaner::SymbolReference>
collectMacroReferences(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
@@ -327,6 +288,44 @@ std::vector<Diag> generateUnusedIncludeDiagnostics(
}
} // namespace
+include_cleaner::Includes
+convertIncludes(const SourceManager &SM,
+ const llvm::ArrayRef<Inclusion> Includes) {
+ include_cleaner::Includes ConvertedIncludes;
+ for (const Inclusion &Inc : Includes) {
+ include_cleaner::Include TransformedInc;
+ llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
+ TransformedInc.Spelled = WrittenRef.trim("\"<>");
+ TransformedInc.HashLocation =
+ SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
+ TransformedInc.Line = Inc.HashLine + 1;
+ TransformedInc.Angled = WrittenRef.starts_with("<");
+ auto FE = SM.getFileManager().getFile(Inc.Resolved);
+ if (!FE) {
+ elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
+ Inc.Resolved, FE.getError().message());
+ continue;
+ }
+ TransformedInc.Resolved = *FE;
+ ConvertedIncludes.add(std::move(TransformedInc));
+ }
+ return ConvertedIncludes;
+}
+
+std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
+ include_cleaner::Header Provider) {
+ if (Provider.kind() == include_cleaner::Header::Physical) {
+ if (auto CanonicalPath =
+ getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
+ std::string SpelledHeader =
+ llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
+ if (!SpelledHeader.empty())
+ return SpelledHeader;
+ }
+ }
+ return include_cleaner::spellHeader(
+ Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
+}
std::vector<const Inclusion *>
getUnused(ParsedAST &AST,
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index d7edca035c96..1a5f07869d56 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -68,6 +68,16 @@ std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
/// FIXME: remove this hack once the implementation is good enough.
void setIncludeCleanerAnalyzesStdlib(bool B);
+/// Converts the clangd include representation to include-cleaner
+/// include representation.
+include_cleaner::Includes
+convertIncludes(const SourceManager &SM,
+ const llvm::ArrayRef<Inclusion> Includes);
+
+/// Determines the header spelling of an include-cleaner header
+/// representation. The spelling contains the ""<> characters.
+std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
+ include_cleaner::Header Provider);
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 061c67d65f7d..08662697a4a5 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -135,11 +135,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
SourceMgr = &CI.getSourceManager();
PP = &CI.getPreprocessor();
Includes.collect(CI);
- if (Config::current().Diagnostics.UnusedIncludes ==
- Config::IncludesPolicy::Strict ||
- Config::current().Diagnostics.MissingIncludes ==
- Config::IncludesPolicy::Strict)
- Pragmas.record(CI);
+ Pragmas.record(CI);
if (BeforeExecuteCallback)
BeforeExecuteCallback(CI);
}
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 211fd1311c98..6ee938420403 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -14,11 +14,12 @@
#include "TestTU.h"
#include "index/MemIndex.h"
#include "clang/AST/Attr.h"
+#include "clang/Format/Format.h"
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/StringRef.h"
-#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <functional>
#include <string>
#include <vector>
@@ -28,6 +29,10 @@ namespace {
using PassMode = HoverInfo::PassType::PassMode;
+std::string guard(llvm::StringRef Code) {
+ return "#pragma once\n" + Code.str();
+}
+
TEST(Hover, Structured) {
struct {
const char *const Code;
@@ -2882,6 +2887,99 @@ TEST(Hover, All) {
}
}
+TEST(Hover, Providers) {
+ struct {
+ const char *Code;
+ const std::function<void(HoverInfo &)> ExpectedBuilder;
+ } Cases[] = {{R"cpp(
+ struct Foo {};
+ Foo F = Fo^o{};
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = ""; }},
+ {R"cpp(
+ #include "foo.h"
+ Foo F = Fo^o{};
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {R"cpp(
+ #include "all.h"
+ Foo F = Fo^o{};
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {R"cpp(
+ #define FOO 5
+ int F = ^FOO;
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = ""; }},
+ {R"cpp(
+ #include "foo.h"
+ int F = ^FOO;
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {R"cpp(
+ #include "all.h"
+ int F = ^FOO;
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {R"cpp(
+ #include "foo.h"
+ Foo A;
+ Foo B;
+ Foo C = A ^+ B;
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ // Hover selects the underlying decl of the using decl
+ {R"cpp(
+ #include "foo.h"
+ namespace ns {
+ using ::Foo;
+ }
+ ns::F^oo d;
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}};
+
+ for (const auto &Case : Cases) {
+ Annotations Code{Case.Code};
+ SCOPED_TRACE(Code.code());
+
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ TU.Code = Code.code();
+ TU.AdditionalFiles["foo.h"] = guard(R"cpp(
+ #define FOO 1
+ class Foo {};
+ Foo& operator+(const Foo, const Foo);
+ )cpp");
+ TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\"");
+
+ auto AST = TU.build();
+ auto H = getHover(AST, Code.point(), format::getLLVMStyle(), nullptr);
+ ASSERT_TRUE(H);
+ HoverInfo Expected;
+ Case.ExpectedBuilder(Expected);
+ SCOPED_TRACE(H->present().asMarkdown());
+ EXPECT_EQ(H->Provider, Expected.Provider);
+ }
+}
+
+TEST(Hover, ParseProviderInfo) {
+ HoverInfo HIFoo;
+ HIFoo.Name = "foo";
+ HIFoo.Provider = "\"foo.h\"";
+
+ HoverInfo HIFooBar;
+ HIFooBar.Name = "foo";
+ HIFooBar.Provider = "<bar.h>";
+ struct Case {
+ HoverInfo HI;
+ llvm::StringRef ExpectedMarkdown;
+ } Cases[] = {{HIFoo, "### `foo` \nprovided by `\"foo.h\"`"},
+ {HIFooBar, "### `foo` \nprovided by `<bar.h>`"}};
+
+ for (const auto &Case : Cases)
+ EXPECT_EQ(Case.HI.present().asMarkdown(), Case.ExpectedMarkdown);
+}
+
TEST(Hover, DocsFromIndex) {
Annotations T(R"cpp(
template <typename T> class X {};
@@ -3359,8 +3457,8 @@ TEST(Hover, ParseDocumentation) {
}
}
-// This is a separate test as headings don't create any
diff erences in plaintext
-// mode.
+// This is a separate test as headings don't create any
diff erences in
+// plaintext mode.
TEST(Hover, PresentHeadings) {
HoverInfo HI;
HI.Kind = index::SymbolKind::Variable;
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index cd1170054807..66916a52046c 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -16,11 +16,13 @@
#include "clang/Format/Format.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/MemoryBufferRef.h"
#include <variant>
namespace clang {
class SourceLocation;
+class SourceManager;
class Decl;
class FileEntry;
class HeaderSearch;
@@ -75,6 +77,14 @@ std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
std::string spellHeader(const Header &H, HeaderSearch &HS,
const FileEntry *Main);
+
+/// Gets all the providers for a symbol by traversing each location.
+/// Returned headers are sorted by relevance, first element is the most
+/// likely provider for the symbol.
+llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
+ const SourceManager &SM,
+ const PragmaIncludes *PI);
+
} // namespace include_cleaner
} // namespace clang
diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
index acf462919344..6bfed91b584b 100644
--- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -22,6 +22,7 @@
#define CLANG_INCLUDE_CLEANER_ANALYSISINTERNAL_H
#include "TypesInternal.h"
+#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "llvm/ADT/STLFunctionalExtras.h"
@@ -58,13 +59,6 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
/// A set of locations that provides the declaration.
std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S);
-/// Gets all the providers for a symbol by traversing each location.
-/// Returned headers are sorted by relevance, first element is the most
-/// likely provider for the symbol.
-llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
- const SourceManager &SM,
- const PragmaIncludes *PI);
-
/// Write an HTML summary of the analysis to the given stream.
void writeHTMLReport(FileID File, const Includes &,
llvm::ArrayRef<Decl *> Roots,
More information about the cfe-commits
mailing list