From 031ffc3e064525731914eff2ea0dfab1ff34cce1 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Fri, 9 Jun 2023 17:53:39 +0200 Subject: [PATCH] [clangd] Decouple IncludeCleaner implementation from Config This should help managing tests as we change defaults in configs. Differential Revision: https://reviews.llvm.org/D152685 --- clang-tools-extra/clangd/IncludeCleaner.cpp | 290 +++++++++------------ clang-tools-extra/clangd/IncludeCleaner.h | 23 +- clang-tools-extra/clangd/ParsedAST.cpp | 46 +++- .../clangd/unittests/IncludeCleanerTests.cpp | 59 ++--- 4 files changed, 204 insertions(+), 214 deletions(-) diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index fa30592..95585a6 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "IncludeCleaner.h" -#include "Config.h" #include "Diagnostics.h" #include "Headers.h" #include "ParsedAST.h" @@ -48,7 +47,6 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" -#include "llvm/ADT/StringSet.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" @@ -62,8 +60,7 @@ #include #include -namespace clang { -namespace clangd { +namespace clang::clangd { static bool AnalyzeStdlib = false; void setIncludeCleanerAnalyzesStdlib(bool B) { AnalyzeStdlib = B; } @@ -84,20 +81,19 @@ clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) { return Result; } -bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderPath) { +bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter IgnoreHeaders) { // Convert the path to Unix slashes and try to match against the filter. llvm::SmallString<64> NormalizedPath(HeaderPath); llvm::sys::path::native(NormalizedPath, llvm::sys::path::Style::posix); - for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) { + for (auto &Filter : IgnoreHeaders) { if (Filter(NormalizedPath)) return true; } return false; } -static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, - const Config &Cfg, - const include_cleaner::PragmaIncludes *PI) { +bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, + const include_cleaner::PragmaIncludes *PI) { // FIXME(kirillbobyrev): We currently do not support the umbrella headers. // System headers are likely to be standard library headers. // Until we have good support for umbrella headers, don't warn about them. @@ -133,11 +129,6 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, FE->getName()); return false; } - - if (isFilteredByConfig(Cfg, Inc.Resolved)) { - dlog("{0} header is filtered out by the configuration", FE->getName()); - return false; - } return true; } @@ -166,15 +157,8 @@ std::string getSymbolName(const include_cleaner::Symbol &Sym) { std::vector generateMissingIncludeDiagnostics( ParsedAST &AST, llvm::ArrayRef MissingIncludes, - llvm::StringRef Code) { + llvm::StringRef Code, HeaderFilter IgnoreHeaders) { std::vector Result; - const Config &Cfg = Config::current(); - if (Cfg.Diagnostics.MissingIncludes != Config::IncludesPolicy::Strict || - Cfg.Diagnostics.SuppressAll || - Cfg.Diagnostics.Suppress.contains("missing-includes")) { - return Result; - } - const SourceManager &SM = AST.getSourceManager(); const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); @@ -191,7 +175,7 @@ std::vector generateMissingIncludeDiagnostics( for (const auto &SymbolWithMissingInclude : MissingIncludes) { llvm::StringRef ResolvedPath = getResolvedPath(SymbolWithMissingInclude.Providers.front()); - if (isFilteredByConfig(Cfg, ResolvedPath)) { + if (isIgnored(ResolvedPath, IgnoreHeaders)) { dlog("IncludeCleaner: not diagnosing missing include {0}, filtered by " "config", ResolvedPath); @@ -252,15 +236,11 @@ std::vector generateMissingIncludeDiagnostics( std::vector generateUnusedIncludeDiagnostics( PathRef FileName, llvm::ArrayRef UnusedIncludes, - llvm::StringRef Code) { + llvm::StringRef Code, HeaderFilter IgnoreHeaders) { std::vector Result; - const Config &Cfg = Config::current(); - if (Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::None || - Cfg.Diagnostics.SuppressAll || - Cfg.Diagnostics.Suppress.contains("unused-includes")) { - return Result; - } for (const auto *Inc : UnusedIncludes) { + if (isIgnored(Inc->Resolved, IgnoreHeaders)) + continue; Diag &D = Result.emplace_back(); D.Message = llvm::formatv("included header {0} is not used directly", @@ -287,6 +267,104 @@ std::vector generateUnusedIncludeDiagnostics( } return Result; } + +std::optional +removeAllUnusedIncludes(llvm::ArrayRef UnusedIncludes) { + if (UnusedIncludes.empty()) + return std::nullopt; + + Fix RemoveAll; + RemoveAll.Message = "remove all unused includes"; + for (const auto &Diag : UnusedIncludes) { + assert(Diag.Fixes.size() == 1 && "Expected exactly one fix."); + RemoveAll.Edits.insert(RemoveAll.Edits.end(), + Diag.Fixes.front().Edits.begin(), + Diag.Fixes.front().Edits.end()); + } + + // TODO(hokein): emit a suitable text for the label. + ChangeAnnotation Annotation = {/*label=*/"", + /*needsConfirmation=*/true, + /*description=*/""}; + static const ChangeAnnotationIdentifier RemoveAllUnusedID = + "RemoveAllUnusedIncludes"; + for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) { + ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I); + RemoveAll.Edits[I].annotationId = ID; + RemoveAll.Annotations.push_back({ID, Annotation}); + } + return RemoveAll; +} + +std::optional +addAllMissingIncludes(llvm::ArrayRef MissingIncludeDiags) { + if (MissingIncludeDiags.empty()) + return std::nullopt; + + Fix AddAllMissing; + AddAllMissing.Message = "add all missing includes"; + // A map to deduplicate the edits with the same new text. + // newText (#include "my_missing_header.h") -> TextEdit. + llvm::StringMap Edits; + for (const auto &Diag : MissingIncludeDiags) { + assert(Diag.Fixes.size() == 1 && "Expected exactly one fix."); + for (const auto &Edit : Diag.Fixes.front().Edits) { + Edits.try_emplace(Edit.newText, Edit); + } + } + // FIXME(hokein): emit used symbol reference in the annotation. + ChangeAnnotation Annotation = {/*label=*/"", + /*needsConfirmation=*/true, + /*description=*/""}; + static const ChangeAnnotationIdentifier AddAllMissingID = + "AddAllMissingIncludes"; + unsigned I = 0; + for (auto &It : Edits) { + ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++); + AddAllMissing.Edits.push_back(std::move(It.getValue())); + AddAllMissing.Edits.back().annotationId = ID; + + AddAllMissing.Annotations.push_back({ID, Annotation}); + } + return AddAllMissing; +} +Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) { + Fix FixAll; + FixAll.Message = "fix all includes"; + + for (const auto &F : RemoveAllUnused.Edits) + FixAll.Edits.push_back(F); + for (const auto &F : AddAllMissing.Edits) + FixAll.Edits.push_back(F); + + for (const auto &A : RemoveAllUnused.Annotations) + FixAll.Annotations.push_back(A); + for (const auto &A : AddAllMissing.Annotations) + FixAll.Annotations.push_back(A); + return FixAll; +} + +std::vector +getUnused(ParsedAST &AST, + const llvm::DenseSet &ReferencedFiles) { + trace::Span Tracer("IncludeCleaner::getUnused"); + std::vector Unused; + for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) { + if (!MFI.HeaderID) + continue; + auto IncludeID = static_cast(*MFI.HeaderID); + if (ReferencedFiles.contains(IncludeID)) + continue; + if (!mayConsiderUnused(MFI, AST, AST.getPragmaIncludes())) { + dlog("{0} was not used, but is not eligible to be diagnosed as unused", + MFI.Written); + continue; + } + Unused.push_back(&MFI); + } + return Unused; +} + } // namespace std::vector @@ -350,33 +428,10 @@ std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile, {Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile}); } -std::vector -getUnused(ParsedAST &AST, - const llvm::DenseSet &ReferencedFiles, - const llvm::StringSet<> &ReferencedPublicHeaders) { - trace::Span Tracer("IncludeCleaner::getUnused"); - const Config &Cfg = Config::current(); - std::vector Unused; - for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) { - if (!MFI.HeaderID) - continue; - if (ReferencedPublicHeaders.contains(MFI.Written)) - continue; - auto IncludeID = static_cast(*MFI.HeaderID); - bool Used = ReferencedFiles.contains(IncludeID); - if (!Used && !mayConsiderUnused(MFI, AST, Cfg, AST.getPragmaIncludes())) { - dlog("{0} was not used, but is not eligible to be diagnosed as unused", - MFI.Written); - continue; - } - if (!Used) - Unused.push_back(&MFI); - dlog("{0} is {1}", MFI.Written, Used ? "USED" : "UNUSED"); - } - return Unused; -} - IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { + // Interaction is only polished for C/CPP. + if (AST.getLangOpts().ObjC) + return {}; const auto &SM = AST.getSourceManager(); const auto &Includes = AST.getIncludeStructure(); include_cleaner::Includes ConvertedIncludes = @@ -454,94 +509,21 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { MapInfo::getHashValue(RHS.Symbol); }); MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end()); - std::vector UnusedIncludes = - getUnused(AST, Used, /*ReferencedPublicHeaders*/ {}); + std::vector UnusedIncludes = getUnused(AST, Used); return {std::move(UnusedIncludes), std::move(MissingIncludes)}; } -std::optional removeAllUnusedIncludes(llvm::ArrayRef UnusedIncludes) { - if (UnusedIncludes.empty()) - return std::nullopt; - - Fix RemoveAll; - RemoveAll.Message = "remove all unused includes"; - for (const auto &Diag : UnusedIncludes) { - assert(Diag.Fixes.size() == 1 && "Expected exactly one fix."); - RemoveAll.Edits.insert(RemoveAll.Edits.end(), - Diag.Fixes.front().Edits.begin(), - Diag.Fixes.front().Edits.end()); - } - - // TODO(hokein): emit a suitable text for the label. - ChangeAnnotation Annotation = {/*label=*/"", - /*needsConfirmation=*/true, - /*description=*/""}; - static const ChangeAnnotationIdentifier RemoveAllUnusedID = - "RemoveAllUnusedIncludes"; - for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) { - ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I); - RemoveAll.Edits[I].annotationId = ID; - RemoveAll.Annotations.push_back({ID, Annotation}); - } - return RemoveAll; -} -std::optional -addAllMissingIncludes(llvm::ArrayRef MissingIncludeDiags) { - if (MissingIncludeDiags.empty()) - return std::nullopt; - - Fix AddAllMissing; - AddAllMissing.Message = "add all missing includes"; - // A map to deduplicate the edits with the same new text. - // newText (#include "my_missing_header.h") -> TextEdit. - llvm::StringMap Edits; - for (const auto &Diag : MissingIncludeDiags) { - assert(Diag.Fixes.size() == 1 && "Expected exactly one fix."); - for (const auto& Edit : Diag.Fixes.front().Edits) { - Edits.try_emplace(Edit.newText, Edit); - } - } - // FIXME(hokein): emit used symbol reference in the annotation. - ChangeAnnotation Annotation = {/*label=*/"", - /*needsConfirmation=*/true, - /*description=*/""}; - static const ChangeAnnotationIdentifier AddAllMissingID = - "AddAllMissingIncludes"; - unsigned I = 0; - for (auto &It : Edits) { - ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++); - AddAllMissing.Edits.push_back(std::move(It.getValue())); - AddAllMissing.Edits.back().annotationId = ID; - - AddAllMissing.Annotations.push_back({ID, Annotation}); - } - return AddAllMissing; -} -Fix fixAll(const Fix& RemoveAllUnused, const Fix& AddAllMissing) { - Fix FixAll; - FixAll.Message = "fix all includes"; - - for (const auto &F : RemoveAllUnused.Edits) - FixAll.Edits.push_back(F); - for (const auto &F : AddAllMissing.Edits) - FixAll.Edits.push_back(F); - - for (const auto& A : RemoveAllUnused.Annotations) - FixAll.Annotations.push_back(A); - for (const auto& A : AddAllMissing.Annotations) - FixAll.Annotations.push_back(A); - return FixAll; -} - -std::vector generateIncludeCleanerDiagnostic( - ParsedAST &AST, const IncludeCleanerFindings &Findings, - llvm::StringRef Code) { +std::vector +issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code, + const IncludeCleanerFindings &Findings, + HeaderFilter IgnoreHeaders) { + trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics"); std::vector UnusedIncludes = generateUnusedIncludeDiagnostics( - AST.tuPath(), Findings.UnusedIncludes, Code); + AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders); std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes); std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics( - AST, Findings.MissingIncludes, Code); + AST, Findings.MissingIncludes, Code, IgnoreHeaders); std::optional AddAllMissing = addAllMissingIncludes(MissingIncludeDiags); std::optional FixAll; @@ -549,47 +531,26 @@ std::vector generateIncludeCleanerDiagnostic( FixAll = fixAll(*RemoveAllUnused, *AddAllMissing); auto AddBatchFix = [](const std::optional &F, clang::clangd::Diag *Out) { - if (!F) return; + if (!F) + return; Out->Fixes.push_back(*F); }; for (auto &Diag : MissingIncludeDiags) { - AddBatchFix(MissingIncludeDiags.size() > 1 - ? AddAllMissing - : std::nullopt, + AddBatchFix(MissingIncludeDiags.size() > 1 ? AddAllMissing : std::nullopt, &Diag); AddBatchFix(FixAll, &Diag); } for (auto &Diag : UnusedIncludes) { - AddBatchFix(UnusedIncludes.size() > 1 ? RemoveAllUnused - : std::nullopt, + AddBatchFix(UnusedIncludes.size() > 1 ? RemoveAllUnused : std::nullopt, &Diag); AddBatchFix(FixAll, &Diag); } auto Result = std::move(MissingIncludeDiags); - llvm::move(UnusedIncludes, - std::back_inserter(Result)); + llvm::move(UnusedIncludes, std::back_inserter(Result)); return Result; } -std::vector issueIncludeCleanerDiagnostics(ParsedAST &AST, - llvm::StringRef Code) { - // Interaction is only polished for C/CPP. - if (AST.getLangOpts().ObjC) - return {}; - - trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics"); - - const Config &Cfg = Config::current(); - IncludeCleanerFindings Findings; - if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict || - Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict) { - // will need include-cleaner results, call it once - Findings = computeIncludeCleanerFindings(AST); - } - return generateIncludeCleanerDiagnostic(AST, Findings, Code); -} - std::optional firstMatchedProvider(const include_cleaner::Includes &Includes, llvm::ArrayRef Providers) { @@ -600,5 +561,4 @@ firstMatchedProvider(const include_cleaner::Includes &Includes, // No match for this provider in the includes list. return std::nullopt; } -} // namespace clangd -} // namespace clang +} // namespace clang::clangd diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h index 675c05a..5884984 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -18,12 +18,17 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDECLEANER_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDECLEANER_H +#include "Diagnostics.h" #include "Headers.h" #include "ParsedAST.h" #include "clang-include-cleaner/Types.h" +#include "clang/Basic/SourceManager.h" #include "clang/Tooling/Syntax/Tokens.h" -#include "llvm/ADT/DenseSet.h" -#include "llvm/ADT/StringSet.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringRef.h" +#include +#include +#include #include #include @@ -47,17 +52,13 @@ struct IncludeCleanerFindings { std::vector MissingIncludes; }; -/// Retrieves headers that are referenced from the main file but not used. -/// In unclear cases, headers are not marked as unused. -std::vector -getUnused(ParsedAST &AST, - const llvm::DenseSet &ReferencedFiles, - const llvm::StringSet<> &ReferencedPublicHeaders); - IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST); -std::vector issueIncludeCleanerDiagnostics(ParsedAST &AST, - llvm::StringRef Code); +using HeaderFilter = llvm::ArrayRef>; +std::vector +issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code, + const IncludeCleanerFindings &Findings, + HeaderFilter IgnoreHeader = {}); /// Affects whether standard library includes should be considered for /// removal. This is off by default for now due to implementation limitations: diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index e5a391a..05f0964 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -11,7 +11,9 @@ #include "../clang-tidy/ClangTidyDiagnosticConsumer.h" #include "../clang-tidy/ClangTidyModule.h" #include "../clang-tidy/ClangTidyModuleRegistry.h" +#include "../clang-tidy/ClangTidyOptions.h" #include "AST.h" +#include "CollectMacros.h" #include "Compiler.h" #include "Config.h" #include "Diagnostics.h" @@ -28,11 +30,18 @@ #include "index/CanonicalIncludes.h" #include "index/Symbol.h" #include "support/Logger.h" +#include "support/Path.h" #include "support/Trace.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclGroup.h" +#include "clang/AST/ExternalASTSource.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/FileEntry.h" +#include "clang/Basic/LLVM.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -40,18 +49,32 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" +#include "clang/Frontend/FrontendOptions.h" +#include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTWriter.h" #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Core/Diagnostic.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/MemoryBuffer.h" +#include +#include +#include #include #include +#include +#include +#include #include // Force the linker to link in Clang-tidy modules. @@ -338,6 +361,27 @@ void applyWarningOptions(llvm::ArrayRef ExtraArgs, } } +std::vector getIncludeCleanerDiags(ParsedAST &AST, llvm::StringRef Code) { + auto &Cfg = Config::current(); + if (Cfg.Diagnostics.SuppressAll) + return {}; + bool SuppressMissing = + Cfg.Diagnostics.Suppress.contains("missing-includes") || + Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::None; + bool SuppressUnused = + Cfg.Diagnostics.Suppress.contains("unused-includes") || + Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::None; + if (SuppressMissing && SuppressUnused) + return {}; + auto Findings = computeIncludeCleanerFindings(AST); + if (SuppressMissing) + Findings.MissingIncludes.clear(); + if (SuppressUnused) + Findings.UnusedIncludes.clear(); + return issueIncludeCleanerDiagnostics(AST, Code, Findings, + Cfg.Diagnostics.Includes.IgnoreHeader); +} + } // namespace std::optional @@ -688,7 +732,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, std::move(Diags), std::move(Includes), std::move(CanonIncludes)); if (Result.Diags) - llvm::move(issueIncludeCleanerDiagnostics(Result, Inputs.Contents), + llvm::move(getIncludeCleanerDiags(Result, Inputs.Contents), std::back_inserter(*Result.Diags)); return std::move(Result); } diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index c0831c2..33e3960 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" -#include "Config.h" #include "Diagnostics.h" #include "IncludeCleaner.h" #include "ParsedAST.h" @@ -45,7 +44,8 @@ using ::testing::Matcher; using ::testing::Pointee; using ::testing::UnorderedElementsAre; -Matcher withFix(std::vector<::testing::Matcher> FixMatcheres) { +Matcher +withFix(std::vector<::testing::Matcher> FixMatcheres) { return Field(&Diag::Fixes, testing::UnorderedElementsAreArray(FixMatcheres)); } @@ -62,7 +62,6 @@ MATCHER_P3(Fix, Range, Replacement, Message, } MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; } - std::string guard(llvm::StringRef Code) { return "#pragma once\n" + Code.str(); } @@ -174,11 +173,6 @@ TEST(IncludeCleaner, ComputeMissingHeaders) { } TEST(IncludeCleaner, GenerateMissingHeaderDiags) { - Config Cfg; - Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict; - Cfg.Diagnostics.Includes.IgnoreHeader = { - [](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }}; - WithContextValue Ctx(Config::Key, std::move(Cfg)); Annotations MainFile(R"cpp( #include "a.h" #include "all.h" @@ -250,8 +244,11 @@ $insert_f[[]]$insert_vector[[]] TU.Code = MainFile.code(); ParsedAST AST = TU.build(); - std::vector Diags = - issueIncludeCleanerDiagnostics(AST, TU.Code); + auto Findings = computeIncludeCleanerFindings(AST); + Findings.UnusedIncludes.clear(); + std::vector Diags = issueIncludeCleanerDiagnostics( + AST, TU.Code, Findings, + {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }}); EXPECT_THAT( Diags, UnorderedElementsAre( @@ -279,9 +276,11 @@ $insert_f[[]]$insert_vector[[]] AllOf( Diag(MainFile.range("vector"), "No header providing \"std::vector\" is directly included"), - withFix({Fix(MainFile.range("insert_vector"), - "#include \n", "#include "), - FixMessage("add all missing includes"),})), + withFix({ + Fix(MainFile.range("insert_vector"), "#include \n", + "#include "), + FixMessage("add all missing includes"), + })), AllOf(Diag(MainFile.range("FOO"), "No header providing \"FOO\" is directly included"), withFix({Fix(MainFile.range("insert_foo"), @@ -320,11 +319,7 @@ TEST(IncludeCleaner, IWYUPragmas) { // IWYU pragma: private, include "public.h" void foo() {} )cpp"); - Config Cfg; - Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; - WithContextValue Ctx(Config::Key, std::move(Cfg)); ParsedAST AST = TU.build(); - EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } @@ -347,7 +342,6 @@ TEST(IncludeCleaner, IWYUPragmaExport) { )cpp"); ParsedAST AST = TU.build(); - EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); EXPECT_THAT(Findings.UnusedIncludes, ElementsAre(Pointee(writtenInclusion("\"foo.h\"")))); @@ -368,13 +362,10 @@ TEST(IncludeCleaner, NoDiagsForObjC) { )cpp"; TU.ExtraArgs.emplace_back("-xobjective-c"); - Config Cfg; - - 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())); + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.MissingIncludes, IsEmpty()); + EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } TEST(IncludeCleaner, UmbrellaUsesPrivate) { @@ -387,11 +378,7 @@ TEST(IncludeCleaner, UmbrellaUsesPrivate) { void foo() {} )cpp"); TU.Filename = "public.h"; - Config Cfg; - Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; - WithContextValue Ctx(Config::Key, std::move(Cfg)); ParsedAST AST = TU.build(); - EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } @@ -511,11 +498,6 @@ TEST(IncludeCleaner, FirstMatchedProvider) { } TEST(IncludeCleaner, BatchFix) { - Config Cfg; - Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict; - Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; - WithContextValue Ctx(Config::Key, std::move(Cfg)); - TestTU TU; TU.Filename = "main.cpp"; TU.AdditionalFiles["foo.h"] = guard("class Foo;"); @@ -532,7 +514,8 @@ TEST(IncludeCleaner, BatchFix) { )cpp"; auto AST = TU.build(); EXPECT_THAT( - issueIncludeCleanerDiagnostics(AST, TU.Code), + issueIncludeCleanerDiagnostics(AST, TU.Code, + computeIncludeCleanerFindings(AST)), UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""), FixMessage("fix all includes")}), withFix({FixMessage("remove #include directive"), @@ -546,14 +529,15 @@ TEST(IncludeCleaner, BatchFix) { )cpp"; AST = TU.build(); EXPECT_THAT( - issueIncludeCleanerDiagnostics(AST, TU.Code), + issueIncludeCleanerDiagnostics(AST, TU.Code, + computeIncludeCleanerFindings(AST)), UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""), FixMessage("fix all includes")}), withFix({FixMessage("remove #include directive"), FixMessage("remove all unused includes"), FixMessage("fix all includes")}), withFix({FixMessage("remove #include directive"), - FixMessage("remove all unused includes"), + FixMessage("remove all unused includes"), FixMessage("fix all includes")}))); TU.Code = R"cpp( @@ -564,7 +548,8 @@ TEST(IncludeCleaner, BatchFix) { )cpp"; AST = TU.build(); EXPECT_THAT( - issueIncludeCleanerDiagnostics(AST, TU.Code), + issueIncludeCleanerDiagnostics(AST, TU.Code, + computeIncludeCleanerFindings(AST)), UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""), FixMessage("add all missing includes"), FixMessage("fix all includes")}), -- 2.7.4