diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -88,11 +88,12 @@ bool StandardLibrary = true; } Index; - enum UnusedIncludesPolicy { - /// Diagnose unused includes. + enum IncludesPolicy { + /// Diagnose missing and unused includes. Strict, None, - /// The same as Strict, but using the include-cleaner library. + /// The same as Strict, but using the include-cleaner library for + /// unused includes. Experiment, }; /// Controls warnings and errors when parsing code. @@ -107,7 +108,8 @@ llvm::StringMap CheckOptions; } ClangTidy; - UnusedIncludesPolicy UnusedIncludes = None; + IncludesPolicy UnusedIncludes = None; + IncludesPolicy MissingIncludes = None; /// IncludeCleaner will not diagnose usages of these headers matched by /// these regexes. diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -432,15 +432,27 @@ if (F.UnusedIncludes) if (auto Val = - compileEnum("UnusedIncludes", + compileEnum("UnusedIncludes", **F.UnusedIncludes) - .map("Strict", Config::UnusedIncludesPolicy::Strict) - .map("Experiment", Config::UnusedIncludesPolicy::Experiment) - .map("None", Config::UnusedIncludesPolicy::None) + .map("Strict", Config::IncludesPolicy::Strict) + .map("Experiment", Config::IncludesPolicy::Experiment) + .map("None", Config::IncludesPolicy::None) .value()) Out.Apply.push_back([Val](const Params &, Config &C) { C.Diagnostics.UnusedIncludes = *Val; }); + + if (F.MissingIncludes) + if (auto Val = + compileEnum("MissingIncludes", + **F.MissingIncludes) + .map("Strict", Config::IncludesPolicy::Strict) + .map("None", Config::IncludesPolicy::None) + .value()) + Out.Apply.push_back([Val](const Params &, Config &C) { + C.Diagnostics.MissingIncludes = *Val; + }); + compile(std::move(F.Includes)); compile(std::move(F.ClangTidy)); diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -221,7 +221,7 @@ /// This often has other advantages, such as skipping some analysis. std::vector> Suppress; - /// Controls how clangd will correct "unnecessary #include directives. + /// Controls how clangd will correct "unnecessary" #include directives. /// clangd can warn if a header is `#include`d but not used, and suggest /// removing it. // @@ -235,6 +235,19 @@ /// - None std::optional> UnusedIncludes; + /// Controls if clangd should analyze missing #include directives. + /// clangd will warn if no header providing a symbol is `#include`d + /// (missing) directly, and suggest adding it. + /// + /// Strict means a header providing a symbol is missing if it is not + /// *directly #include'd. The file might still compile if the header is + /// included transitively. + /// + /// Valid values are: + /// - Strict + /// - None + std::optional> MissingIncludes; + /// Controls IncludeCleaner diagnostics. struct IncludesBlock { /// Regexes that will be used to avoid diagnosing certain includes as diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -128,6 +128,9 @@ Dict.handle("UnusedIncludes", [&](Node &N) { F.UnusedIncludes = scalarValue(N, "UnusedIncludes"); }); + Dict.handle("MissingIncludes", [&](Node &N) { + F.MissingIncludes = scalarValue(N, "MissingIncludes"); + }); Dict.handle("Includes", [&](Node &N) { parse(F.Includes, N); }); Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); }); Dict.parse(N); 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 @@ -32,6 +32,11 @@ namespace clang { namespace clangd { +struct IncludeCleanerFindings { + std::vector UnusedIncludes; + llvm::StringMap> MissingIncludes; +}; + struct ReferencedLocations { llvm::DenseSet User; llvm::DenseSet Stdlib; @@ -96,13 +101,10 @@ const llvm::DenseSet &ReferencedFiles, const llvm::StringSet<> &ReferencedPublicHeaders); +IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST); std::vector computeUnusedIncludes(ParsedAST &AST); -// The same as computeUnusedIncludes, but it is an experimental and -// include-cleaner-lib-based implementation. -std::vector -computeUnusedIncludesExperimental(ParsedAST &AST); -std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, +std::vector issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code); /// Affects whether standard library includes should be considered for 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 @@ -8,32 +8,44 @@ #include "IncludeCleaner.h" #include "Config.h" +#include "Diagnostics.h" #include "Headers.h" #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" +#include "URI.h" #include "clang-include-cleaner/Analysis.h" #include "clang-include-cleaner/Types.h" -#include "index/CanonicalIncludes.h" -#include "support/Logger.h" -#include "support/Trace.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/FileEntry.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Inclusions/HeaderIncludes.h" +#include "clang/Tooling/Inclusions/IncludeStyle.h" #include "clang/Tooling/Syntax/Tokens.h" +#include "index/CanonicalIncludes.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" -#include +#include "support/Logger.h" +#include "support/Trace.h" +#include #include +#include namespace clang { namespace clangd { @@ -85,7 +97,7 @@ // Using templateName case is handled by the override TraverseTemplateName. if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate) return true; - add(TST->getAsCXXRecordDecl()); // Specialization + add(TST->getAsCXXRecordDecl()); // Specialization return true; } @@ -460,6 +472,27 @@ return TranslatedHeaderIDs; } +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; +} + // This is the original clangd-own implementation for computing unused // #includes. Eventually it will be deprecated and replaced by the // include-cleaner-lib-based implementation. @@ -474,104 +507,229 @@ translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM); return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas); } -std::vector computeUnusedIncludesExperimental(ParsedAST &AST) { - const auto &SM = AST.getSourceManager(); - const auto &Includes = AST.getIncludeStructure(); - // FIXME: this map should probably be in IncludeStructure. - llvm::StringMap> BySpelling; - for (const auto &Inc : Includes.MainFileIncludes) { + +include_cleaner::Includes +convertIncludes(const SourceManager &SM, + const std::vector &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; + TransformedInc.Angled = WrittenRef.starts_with("<"); + if (auto FE = SM.getFileManager().getFile(Inc.Resolved)) + TransformedInc.Resolved = *FE; + Includes.add(std::move(TransformedInc)); + } + return Includes; +} + +Diag computeMissingIncludesDiagnostic(ParsedAST &AST, llvm::StringRef Spelling, + llvm::ArrayRef Tokens, + llvm::StringRef Code, + std::string FileName) { + Diag D; + D.Message = llvm::formatv("header {0} is missing", Spelling); + D.Name = "missing-includes"; + D.Source = Diag::DiagSource::Clangd; + D.File = FileName; + D.Severity = DiagnosticsEngine::Warning; + + syntax::FileRange Range = syntax::Token::range( + AST.getSourceManager(), Tokens[0], Tokens[Tokens.size() - 1]); + D.Range = clangd::Range{offsetToPosition(Code, Range.beginOffset()), + offsetToPosition(Code, Range.endOffset())}; + tooling::HeaderIncludes HeaderIncludes(FileName, Code, + tooling::IncludeStyle{}); + llvm::StringRef HeaderRef{Spelling}; + bool Angled = HeaderRef.starts_with("<"); + std::optional Replacement = HeaderIncludes.insert( + HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include); + if (Replacement.has_value()) { + D.Fixes.emplace_back(); + D.Fixes.back().Message = "#include " + Spelling.str(); + TextEdit Edit = replacementToEdit(Code, *Replacement); + D.Fixes.back().Edits.emplace_back(std::move(Edit)); + } + D.InsideMainFile = true; + return D; +} + +Diag computeUnusedIncludesDiagnostic(std::string FileName, const Inclusion *Inc, + llvm::StringRef Code) { + Diag D; + D.Message = llvm::formatv( + "included header {0} is not used directly", + llvm::sys::path::filename(Inc->Written.substr(1, Inc->Written.size() - 2), + llvm::sys::path::Style::posix)); + D.Name = "unused-includes"; + D.Source = Diag::DiagSource::Clangd; + D.File = FileName; + D.Severity = DiagnosticsEngine::Warning; + D.Tags.push_back(Unnecessary); + D.Range = getDiagnosticRange(Code, Inc->HashOffset); + // FIXME(kirillbobyrev): Removing inclusion might break the code if the + // used headers are only reachable transitively through this one. Suggest + // including them directly instead. + // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas + // (keep/export) remove the warning once we support IWYU pragmas. + D.Fixes.emplace_back(); + D.Fixes.back().Message = "remove #include directive"; + D.Fixes.back().Edits.emplace_back(); + D.Fixes.back().Edits.back().range.start.line = Inc->HashLine; + D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1; + D.InsideMainFile = true; + return D; +} + +std::string resolveSpelledHeader(ParsedAST &AST, const FileEntry *MainFile, + include_cleaner::Header Provider) { + std::string SpelledHeader; + if (Provider.kind() == include_cleaner::Header::Physical) { + std::optional CanonicalPath = + getCanonicalPath(Provider.physical(), AST.getSourceManager()); + if (CanonicalPath.has_value()) { + llvm::Expected Uri = URI::create(*CanonicalPath); + if (!Uri.takeError()) { + llvm::Expected ExpectedIncludeSpelling = + URI::includeSpelling(*Uri); + if (!ExpectedIncludeSpelling.takeError()) + SpelledHeader = *ExpectedIncludeSpelling; + } + } + } + if (SpelledHeader.empty()) + SpelledHeader = include_cleaner::spellHeader( + Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile); + return SpelledHeader; +} + +IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { + const auto &SM = AST.getSourceManager(); + const auto &Includes = AST.getIncludeStructure(); + include_cleaner::Includes IncludeCleanerIncludes = + convertIncludes(AST.getSourceManager(), Includes.MainFileIncludes); + const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); + + // FIXME: this map should probably be in IncludeStructure. + llvm::StringMap> BySpelling; + for (const auto &Inc : Includes.MainFileIncludes) { if (Inc.HeaderID) BySpelling.try_emplace(Inc.Written) .first->second.push_back( static_cast(*Inc.HeaderID)); - } - // 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}); - } - llvm::DenseSet Used; - include_cleaner::walkUsed( - AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, - AST.getPragmaIncludes(), SM, - [&](const include_cleaner::SymbolReference &Ref, - llvm::ArrayRef Providers) { - for (const auto &H : Providers) { - switch (H.kind()) { - case include_cleaner::Header::Physical: - if (auto HeaderID = Includes.getID(H.physical())) - Used.insert(*HeaderID); - break; - case include_cleaner::Header::Standard: - for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard())) - Used.insert(HeaderID); - break; - case include_cleaner::Header::Verbatim: - for (auto HeaderID : BySpelling.lookup(H.verbatim())) - Used.insert(HeaderID); - break; - } - } - }); - return getUnused(AST, Used, /*ReferencedPublicHeaders*/{}); + } + + std::vector Macros = + collectMacroReferences(AST); + llvm::StringMap> MissingIncludes; + llvm::DenseSet Used; + include_cleaner::walkUsed( + AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, + AST.getPragmaIncludes(), SM, + [&](const include_cleaner::SymbolReference &Ref, + llvm::ArrayRef Providers) { + bool Satisfied = false; + for (const auto &H : Providers) { + if (!IncludeCleanerIncludes.match(H).empty()) { + Satisfied = true; + } + switch (H.kind()) { + case include_cleaner::Header::Physical: + if (auto HeaderID = Includes.getID(H.physical())) + Used.insert(*HeaderID); + if (H.physical() == MainFile) + Satisfied = true; + break; + case include_cleaner::Header::Standard: + for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard())) + Used.insert(HeaderID); + break; + case include_cleaner::Header::Verbatim: + for (auto HeaderID : BySpelling.lookup(H.verbatim())) + Used.insert(HeaderID); + break; + } + } + + if (!Satisfied && !Providers.empty() && + Ref.RT == include_cleaner::RefType::Explicit) { + + std::string SpelledHeader = + resolveSpelledHeader(AST, MainFile, Providers.front()); + + const syntax::TokenBuffer &Tokens = AST.getTokens(); + std::optional> SpelledForExpanded = + Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation)); + if (SpelledForExpanded.has_value()) + MissingIncludes.insert( + {SpelledHeader, std::move(SpelledForExpanded.value())}); + } + }); + std::vector UnusedIncludes = + getUnused(AST, Used, /*ReferencedPublicHeaders*/ {}); + return {UnusedIncludes, MissingIncludes}; } -std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, +std::vector issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code) { - const Config &Cfg = Config::current(); - if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None || - Cfg.Diagnostics.SuppressAll || - Cfg.Diagnostics.Suppress.contains("unused-includes")) - return {}; // Interaction is only polished for C/CPP. if (AST.getLangOpts().ObjC) return {}; - trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics"); - std::vector Result; + const Config &Cfg = Config::current(); + if (Cfg.Diagnostics.SuppressAll || + (Cfg.Diagnostics.Suppress.contains("unused-includes") && + Cfg.Diagnostics.Suppress.contains("missing-includes"))) { + return {}; + } + + trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics"); + + std::vector UnusedIncludes; + llvm::StringMap> MissingIncludes; + + IncludeCleanerFindings Findings; + if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict || + Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Experiment) { + // will need include-cleaner results, call it once + Findings = computeIncludeCleanerFindings(AST); + } + + if (!Cfg.Diagnostics.Suppress.contains("unused-includes")) { + switch (Cfg.Diagnostics.UnusedIncludes) { + case Config::IncludesPolicy::Experiment: + UnusedIncludes = std::move(Findings.UnusedIncludes); + break; + case Config::IncludesPolicy::Strict: + UnusedIncludes = computeUnusedIncludes(AST); + break; + case Config::IncludesPolicy::None: + // do nothing + break; + } + } + + if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict && + !Cfg.Diagnostics.Suppress.contains("missing-includes")) { + MissingIncludes = std::move(Findings.MissingIncludes); + } + std::string FileName = AST.getSourceManager() .getFileEntryRefForID(AST.getSourceManager().getMainFileID()) ->getName() .str(); - const auto &UnusedIncludes = - Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment - ? computeUnusedIncludesExperimental(AST) - : computeUnusedIncludes(AST); - for (const auto *Inc : UnusedIncludes) { - Diag D; - D.Message = - llvm::formatv("included header {0} is not used directly", - llvm::sys::path::filename( - Inc->Written.substr(1, Inc->Written.size() - 2), - llvm::sys::path::Style::posix)); - D.Name = "unused-includes"; - D.Source = Diag::DiagSource::Clangd; - D.File = FileName; - D.Severity = DiagnosticsEngine::Warning; - D.Tags.push_back(Unnecessary); - D.Range = getDiagnosticRange(Code, Inc->HashOffset); - // FIXME(kirillbobyrev): Removing inclusion might break the code if the - // used headers are only reachable transitively through this one. Suggest - // including them directly instead. - // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas - // (keep/export) remove the warning once we support IWYU pragmas. - D.Fixes.emplace_back(); - D.Fixes.back().Message = "remove #include directive"; - D.Fixes.back().Edits.emplace_back(); - D.Fixes.back().Edits.back().range.start.line = Inc->HashLine; - D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1; - D.InsideMainFile = true; - Result.push_back(std::move(D)); + std::vector Result; + for (const auto &Inc : UnusedIncludes) { + Result.push_back(computeUnusedIncludesDiagnostic(FileName, Inc, Code)); + } + for (const llvm::StringMapEntry> &Missing : + MissingIncludes) { + Result.push_back(computeMissingIncludesDiagnostic( + AST, Missing.getKey(), Missing.getValue(), Code, FileName)); } return Result; } diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -689,11 +689,11 @@ std::move(Diags), std::move(Includes), std::move(CanonIncludes)); if (Result.Diags) { - auto UnusedHeadersDiags = - issueUnusedIncludesDiagnostics(Result, Inputs.Contents); + auto IncludeCleanerDiags = + issueIncludeCleanerDiagnostics(Result, Inputs.Contents); Result.Diags->insert(Result.Diags->end(), - make_move_iterator(UnusedHeadersDiags.begin()), - make_move_iterator(UnusedHeadersDiags.end())); + make_move_iterator(IncludeCleanerDiags.begin()), + make_move_iterator(IncludeCleanerDiags.end())); } return std::move(Result); } 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 @@ -123,7 +123,7 @@ SourceMgr = &CI.getSourceManager(); Includes.collect(CI); if (Config::current().Diagnostics.UnusedIncludes == - Config::UnusedIncludesPolicy::Experiment) + Config::IncludesPolicy::Experiment) Pragmas.record(CI); if (BeforeExecuteCallback) BeforeExecuteCallback(CI); diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -249,19 +249,19 @@ // Defaults to None. EXPECT_TRUE(compileAndApply()); EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::UnusedIncludesPolicy::None); + Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("None"); EXPECT_TRUE(compileAndApply()); EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::UnusedIncludesPolicy::None); + Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("Strict"); EXPECT_TRUE(compileAndApply()); EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::UnusedIncludesPolicy::Strict); + Config::IncludesPolicy::Strict); Frag = {}; EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty()) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -1897,7 +1897,7 @@ // Off by default. EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); Config Cfg; - Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict; + Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; // Set filtering. Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back( [](llvm::StringRef Header) { return Header.endswith("ignore.h"); }); diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -342,7 +342,8 @@ auto AST = TU.build(); EXPECT_THAT(computeUnusedIncludes(AST), ElementsAre(Pointee(writtenInclusion("")))); - EXPECT_THAT(computeUnusedIncludesExperimental(AST), + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.UnusedIncludes, ElementsAre(Pointee(writtenInclusion("")))); } @@ -379,12 +380,58 @@ computeUnusedIncludes(AST), UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")), Pointee(writtenInclusion("\"dir/unused.h\"")))); + + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); EXPECT_THAT( - computeUnusedIncludesExperimental(AST), + Findings.UnusedIncludes, UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")), Pointee(writtenInclusion("\"dir/unused.h\"")))); } +TEST(IncludeCleaner, GetMissingHeaders) { + llvm::StringLiteral MainFile = R"cpp( + #include "a.h" + #include "dir/c.h" + #include + + void foo() { + b(); + d(); + f(); + })cpp"; + // Build expected ast with symbols coming from headers. + TestTU TU; + TU.Filename = "foo.cpp"; + TU.AdditionalFiles["foo.h"] = guard("void foo();"); + TU.AdditionalFiles["a.h"] = guard("#include \"b.h\""); + TU.AdditionalFiles["b.h"] = guard("void b();"); + + TU.AdditionalFiles["dir/c.h"] = guard("#include \"d.h\""); + TU.AdditionalFiles["dir/d.h"] = guard("void d();"); + + TU.AdditionalFiles["system/e.h"] = guard("#include "); + TU.AdditionalFiles["system/f.h"] = guard("void f();"); + TU.ExtraArgs.push_back("-isystem" + testPath("system")); + + TU.Code = MainFile.str(); + ParsedAST AST = TU.build(); + + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + std::vector Names; + std::vector> TokenTexts; + for (const auto &MissingInclude : Findings.MissingIncludes) { + std::vector TokenText; + for (const auto &Token : MissingInclude.getValue()) + TokenText.push_back(std::string{Token.text(AST.getSourceManager())}); + Names.push_back(MissingInclude.getKey()); + TokenTexts.push_back(TokenText); + } + EXPECT_THAT(Names, UnorderedElementsAre("\"b.h\"", "\"dir/d.h\"", "")); + EXPECT_THAT(TokenTexts, UnorderedElementsAre(UnorderedElementsAre("b"), + UnorderedElementsAre("d"), + UnorderedElementsAre("f"))); +} + TEST(IncludeCleaner, VirtualBuffers) { TestTU TU; TU.Code = R"cpp( @@ -554,7 +601,8 @@ ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID())); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); - EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty()); + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } TEST(IncludeCleaner, RecursiveInclusion) { @@ -583,7 +631,8 @@ EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); - EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty()); + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } TEST(IncludeCleaner, IWYUPragmaExport) { @@ -608,7 +657,8 @@ // FIXME: This is not correct: foo.h is unused but is not diagnosed as such // because we ignore headers with IWYU export pragmas for now. EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); - EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty()); + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } TEST(IncludeCleaner, NoDiagsForObjC) { @@ -627,7 +677,8 @@ TU.ExtraArgs.emplace_back("-xobjective-c"); Config Cfg; - Cfg.Diagnostics.UnusedIncludes = Config::Strict; + Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; + Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict; WithContextValue Ctx(Config::Key, std::move(Cfg)); ParsedAST AST = TU.build(); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); 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 @@ -73,6 +73,8 @@ std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code, const format::FormatStyle &IncludeStyle); +std::string spellHeader(const Header &H, HeaderSearch &HS, + const FileEntry *Main); } // namespace include_cleaner } // namespace clang diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -49,8 +49,8 @@ } } -static std::string spellHeader(const Header &H, HeaderSearch &HS, - const FileEntry *Main) { +std::string spellHeader(const Header &H, HeaderSearch &HS, + const FileEntry *Main) { switch (H.kind()) { case Header::Physical: { bool IsSystem = false;