diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h --- 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 +#include namespace clang { namespace clangd { @@ -67,6 +68,8 @@ 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 SymRange; index::SymbolKind Kind = index::SymbolKind::Unknown; std::string Documentation; diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- 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 #include +#include namespace clang { namespace clangd { @@ -1084,6 +1091,49 @@ 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 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 getHover(ParsedAST &AST, Position Pos, @@ -1131,6 +1181,12 @@ 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 @@ 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()) { HI = getHoverContents(N, E, AST, PP, Index); } else if (const Attr *A = N->ASTNode.get()) { @@ -1217,6 +1274,14 @@ 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/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -68,6 +68,16 @@ /// 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 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/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -136,45 +136,6 @@ return true; } -include_cleaner::Includes -convertIncludes(const SourceManager &SM, - const llvm::ArrayRef 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 collectMacroReferences(ParsedAST &AST) { const auto &SM = AST.getSourceManager(); @@ -327,6 +288,44 @@ } } // namespace +include_cleaner::Includes +convertIncludes(const SourceManager &SM, + const llvm::ArrayRef 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 getUnused(ParsedAST &AST, diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -135,11 +135,7 @@ 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 --- 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 #include #include @@ -28,6 +29,10 @@ 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, Providers) { + struct { + const char *Code; + const std::function 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 = ""; + struct Case { + HoverInfo HI; + llvm::StringRef ExpectedMarkdown; + } Cases[] = {{HIFoo, "### `foo` \nprovided by `\"foo.h\"`"}, + {HIFooBar, "### `foo` \nprovided by ``"}}; + + for (const auto &Case : Cases) + EXPECT_EQ(Case.HI.present().asMarkdown(), Case.ExpectedMarkdown); +} + TEST(Hover, DocsFromIndex) { Annotations T(R"cpp( template class X {}; @@ -3359,8 +3457,8 @@ } } -// This is a separate test as headings don't create any differences in plaintext -// mode. +// This is a separate test as headings don't create any differences 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 --- 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 namespace clang { class SourceLocation; +class SourceManager; class Decl; class FileEntry; class HeaderSearch; @@ -75,6 +77,14 @@ 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
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 --- 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 @@ /// A set of locations that provides the declaration. std::vector> 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
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 Roots,