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). + 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,41 @@ 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 Headers = + include_cleaner::headersForSymbol(Sym, SM, AST.getPragmaIncludes()); + std::string Result; + include_cleaner::Includes ConvertedIncludes = + convertIncludes(SM, AST.getIncludeStructure().MainFileIncludes); + for (const auto &H : Headers) { + auto Matches = ConvertedIncludes.match(H); + if (!Matches.empty()) { + // Pick the best-ranked included provider + Result = Matches[0]->quote(); + break; + } + } + + if (!Result.empty()) { + HI.Provider = std::move(Result); + return; + } + + for (const auto &H : Headers) { + if (H.kind() == include_cleaner::Header::Physical && + H.physical() == SM.getFileEntryForID(SM.getMainFileID())) + break; + + // Pick the best-ranked non-included provider + HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H); + break; + } +} + } // namespace std::optional getHover(ParsedAST &AST, Position Pos, @@ -1131,6 +1173,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 +1216,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 +1266,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,12 @@ /// FIXME: remove this hack once the implementation is good enough. void setIncludeCleanerAnalyzesStdlib(bool B); +include_cleaner::Includes +convertIncludes(const SourceManager &SM, + const llvm::ArrayRef MainFileIncludes); + +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 @@ -124,66 +124,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(); - // FIXME: !!this is a hacky way to collect macro references. - std::vector Macros; - auto &PP = AST.getPreprocessor(); - for (const syntax::Token &Tok : - AST.getTokens().spelledTokens(SM.getMainFileID())) { - auto Macro = locateMacroAt(Tok, PP); - if (!Macro) - continue; - if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid()) - Macros.push_back( - {Tok.location(), - include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)), - DefLoc}, - include_cleaner::RefType::Explicit}); - } - return Macros; -} - llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) { switch (SymProvider.kind()) { case include_cleaner::Header::Physical: @@ -313,8 +253,67 @@ } return Result; } + +std::vector +collectMacroReferences(ParsedAST &AST) { + const auto &SM = AST.getSourceManager(); + // FIXME: !!this is a hacky way to collect macro references. + std::vector Macros; + auto &PP = AST.getPreprocessor(); + for (const syntax::Token &Tok : + AST.getTokens().spelledTokens(SM.getMainFileID())) { + auto Macro = locateMacroAt(Tok, PP); + if (!Macro) + continue; + if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid()) + Macros.push_back( + {Tok.location(), + include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)), + DefLoc}, + include_cleaner::RefType::Explicit}); + } + return Macros; +} } // namespace +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 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 @@ -127,11 +127,7 @@ LangOpts = &CI.getLangOpts(); SourceMgr = &CI.getSourceManager(); 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,280 @@ } } +TEST(Hover, Providers) { + struct { + Annotations Code; + llvm::StringLiteral FooHeader; + llvm::StringLiteral BarHeader; + llvm::StringLiteral FooBarHeader; + const std::function ExpectedBuilder; + } Cases[] = { + {Annotations( + R"cpp( + struct Foo {}; + Foo F = [[Fo^o]]{}; + )cpp"), + "int foo();", "", "", [](HoverInfo &HI) { HI.Provider = ""; }}, + {Annotations( + R"cpp( + #include "foo.h" + int F = [[fo^o]](); + )cpp"), + "int foo();", "", "", [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + #include "foo.h" + int F = [[f^oo]](); + )cpp"), + "int foo();", "int foo();", "", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + #include "foo.h" + int F = [[^foo]](); + )cpp"), + "int foo();", "#include \"foobar.h\"", "int foo();", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + #include "foo.h" + #include "foobar.h" + int F = [[^foo]](); + )cpp"), + "int foo();", "int foo();", "#include \"bar.h\"", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + #include "foo.h" + #include "foobar.h" + int F = [[foo^]](); + )cpp"), + "int foo();", "#include \"foo.h\"", "#include \"foo.h\"", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "foo.h" + + Foo [[f^oo]] = 42; + )cpp"), + R"cpp( + class Foo { + public: + Foo(int val): val(val) {} + private: + int val; + }; + )cpp", "", "", [](HoverInfo &HI) { HI.Provider = ""; }}, + {Annotations( + R"cpp( + #include "bar.h" + + int F = [[f^oo]](); + )cpp"), + R"cpp( + int foo(); + )cpp", "#include \"foo.h\"", "", [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + + int F = [[f^oo]](); + )cpp"), + "int foo();", + R"cpp( + #include "foo.h" + #include "foobar.h" + + )cpp", "int foo();", [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + + int F = [[f^oo]](); + )cpp"), + R"cpp( + #include "foobar.h" + )cpp", R"cpp( + #include "foo.h" + #include "foobar.h" + + )cpp", "int foo();", [](HoverInfo &HI) { HI.Provider = "\"foobar.h\""; }}, + {Annotations( + R"cpp( + #include "foo.h" + int F = [[^MACRO]]; + )cpp"), + "#define MACRO 5", + "", "", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + #include "foo.h" + + int F = [[M^ACRO]]; + )cpp"), + "#define MACRO 5", + "#define MACRO 10", "", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + #include "foo.h" + + int F = [[MA^CRO]](5); + )cpp"), + "#define MACRO(X) X+1", + "#include \"foobar.h\"", "#define MACRO(X) X+3", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "foo.h" + int [[^F]] = MACRO(5); + )cpp"), + "#define MACRO(X) (X+1)", + "", "", + [](HoverInfo &HI) { HI.Provider = ""; }}, + {Annotations( + R"cpp( + #include "bar.h" + int F = [[MACR^O]]; + )cpp"), + "#define MACRO 5", "#include \"foo.h\"", "", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + int F = [[MACR^O]]; + )cpp"), + "#define MACRO 5", R"cpp( + #include "foo.h" + #include "foobar.h" + )cpp", "#define MACRO 10", + [](HoverInfo &HI) { HI.Provider = "\"foobar.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + + int F = [[fo^o]](); + )cpp"), + "", R"cpp( + // IWYU pragma: private, include "foo.h" + int foo(); + )cpp", "", + [](HoverInfo &HI) { HI.Provider = "\"bar.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + + int F = [[fo^o]](); + )cpp"), + "", "#include \"foobar.h\"", + R"cpp( + // IWYU pragma: private, include "public.h" + int foo(); + )cpp", + [](HoverInfo &HI) { HI.Provider = "\"public.h\""; }}, + {Annotations( + R"cpp( + #include "bar.h" + + std::[[vect^or]] v; + )cpp"), + R"cpp( + namespace std { + class vector {}; + } + )cpp", "#include \"foo.h\"", + "", + [](HoverInfo &HI) { HI.Provider = ""; }}, + {Annotations( + R"cpp( + #include "foo.h" + + Foo A; + Foo B; + Foo C = A [[^+]] B; + + )cpp"), + R"cpp( + class Foo {}; + Foo& operator+(const Foo &lhs, const Foo &rhs); + )cpp", "#include \"foo.h\"", + "", + [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }} + }; + + for (const auto &Case : Cases) { + SCOPED_TRACE(Case.Code.code()); + + TestTU TU; + TU.Filename = "foo.cpp"; + TU.Code = Case.Code.code(); + TU.AdditionalFiles["foo.h"] = guard(Case.FooHeader); + TU.AdditionalFiles["bar.h"] = guard(Case.BarHeader); + TU.AdditionalFiles["foobar.h"] = guard(Case.FooBarHeader); + + auto AST = TU.build(); + auto H = getHover(AST, Case.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, NoProviderIfOtherDeclUsedForHoverCard) { + Annotations Code(R"cpp( + #include "absl_string_view.h" + absl::str^ing_view abc; // hover on string_view + )cpp"); + llvm::StringLiteral AbslHeader = R"cpp( + #include "std_string_view.h" + namespace absl { + using std::string_view; + })cpp"; + llvm::StringLiteral StdHeader = R"cpp( + namespace std { + class string_view {}; + })cpp"; + + TestTU TU; + TU.Filename = "main.cpp"; + TU.Code = Code.code(); + TU.AdditionalFiles["absl_string_view.h"] = guard(AbslHeader); + TU.AdditionalFiles["std_string_view.h"] = guard(StdHeader); + + auto AST = TU.build(); + auto H = getHover(AST, Code.point(), format::getLLVMStyle(), nullptr); + ASSERT_TRUE(H); + EXPECT_EQ(H->Provider, ""); +} + + TEST(Hover, DocsFromIndex) { Annotations T(R"cpp( template class X {}; @@ -3359,8 +3638,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 @@ -13,9 +13,11 @@ #include "clang-include-cleaner/Record.h" #include "clang-include-cleaner/Types.h" +#include "clang/Basic/SourceLocation.h" #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 @@ -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