[include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

[kadircet]

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

61

A comma-separated list of regexes to match against suffix of a header, and disable analysis if matched.

161–162

we actually want to perform filtering inside analyze. it's not enough to filter only when printing, we should also filter before applying fixes :D. but we should also be applying filtering to absolute paths in all cases, for missing includes this is being applied to spelling instead.

162

erroring out only after we performed all of our analysis and build a TU feels unfortunate. can we build these regexes and verify them at main and pass onto action instead? (in case the action is run on multiple files at once, we shouldn't compile/verify regexes over and over again)

[hokein]

address comments:

• move the filtering in analysis API
• add an actual -edit lit test

[hokein]

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

161–162

oops, good catch. Added an actual --edit test.

[kadircet]

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h

67–68

can you add some comments about how header filter works? A predicate that receives absolute path or spelling without quotes/brackets, when a phyiscal file doesn't exist. No analysis will be performed for headers that satisfy the predicate.

72

rather than a default here, can we just do no filtering when HeaderFilter is null ?

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

137

again some comments here could be useful

clang-tools-extra/include-cleaner/lib/Analysis.cpp

65

can you drop Path or put it in comments

clang-tools-extra/include-cleaner/test/tool.cpp

17

can you ignore one but keep other?

it'd be useful to also test the regex behaviour

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

112

same as above, can you either drop or comment out Path

119

same here for Path

172

nit: braces

263

nit: might be better to extract this into a function like: std::function<bool(StringRef)> headerFilter() { /* parses flags, returns a null function on failure */ }

265

s/KeepEmpty/KeepEmpty=/

[hokein]

address review comments.

[hokein]

clang-tools-extra/include-cleaner/test/tool.cpp

17

this tests aims to test filtering logic for both missing-includes and unused-includes cases.

added a new test.

[kadircet]

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

138

i think name and the comment are somewhat confusing, what about:

/// Absolute path for the header, when it's a physical file. Otherwise just the spelling without surrounding quotes/brackets.

clang-tools-extra/include-cleaner/lib/Analysis.cpp

84

nit: rather than checking this at every use, might be easier to have something like:

if (!HeaderFilter)
  HeaderFilter = +[](llvm::StringRef) { return false; };

clang-tools-extra/include-cleaner/test/tool.cpp

17

we're still lacking a test for regex behaviour, maybe change next one to foob.*\.h ?

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

221

nit: Drop Path

240

FilterRegs=std::move(FilterRegs) and drop the shared_ptr?

241

looks like debugging artifact

[hokein]

address comments

[hokein]

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

240

This was my first attempt, it didn't work, we're returning a lamdba function, which invokes a delete constructor of llvm::Regex.

clang-tools-extra/clangd/IncludeCleaner.cpp

if (!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
           &FE->getFileEntry())) {
     dlog("{0} doesn't have header guard and will not be considered unused",
          FE->getName());
     return false;
   }
   return true;
 }
 
-llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) {
-  switch (SymProvider.kind()) {
-  case include_cleaner::Header::Physical:
-    return SymProvider.physical()->tryGetRealPathName();
-  case include_cleaner::Header::Standard:
-    return SymProvider.standard().name().trim("<>\"");
-  case include_cleaner::Header::Verbatim:
-    return SymProvider.verbatim().trim("<>\"");
-  }
-  llvm_unreachable("Unknown header kind");
-}
-
 std::vector<Diag> generateMissingIncludeDiagnostics(
     ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes,
     llvm::StringRef Code, HeaderFilter IgnoreHeaders) {
   std::vector<Diag> Result;
   const SourceManager &SM = AST.getSourceManager();
   const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
 
   auto FileStyle = format::getStyle(
       format::DefaultFormatStyle, AST.tuPath(), format::DefaultFallbackStyle,
       Code, &SM.getFileManager().getVirtualFileSystem());
   if (!FileStyle) {
     elog("Couldn't infer style", FileStyle.takeError());
     FileStyle = format::getLLVMStyle();
   }
 
   tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code,
                                          FileStyle->IncludeStyle);
   for (const auto &SymbolWithMissingInclude : MissingIncludes) {
     llvm::StringRef ResolvedPath =
-        getResolvedPath(SymbolWithMissingInclude.Providers.front());
+        SymbolWithMissingInclude.Providers.front().resolvedPath();
     if (isIgnored(ResolvedPath, IgnoreHeaders)) {
       dlog("IncludeCleaner: not diagnosing missing include {0}, filtered by "
            "config",
            ResolvedPath);
       continue;
     }
 
     std::string Spelling = include_cleaner::spellHeader(

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h

void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
               const PragmaIncludes *PI, const SourceManager &, UsedSymbolCB CB);
 
 struct AnalysisResults {
   std::vector<const Include *> Unused;
   std::vector<std::string> Missing; // Spellings, like "<vector>"
 };
 
 /// Determine which headers should be inserted or removed from the main file.
 /// This exposes conclusions but not reasons: use lower-level walkUsed for that.
-AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
-                        llvm::ArrayRef<SymbolReference> MacroRefs,
-                        const Includes &I, const PragmaIncludes *PI,
-                        const SourceManager &SM, const HeaderSearch &HS);
+///
+/// The HeaderFilter is a predicate that receives absolute path or spelling
+/// without quotes/brackets, when a phyiscal file doesn't exist.
+/// No analysis will be performed for headers that satisfy the predicate.
+AnalysisResults
+analyze(llvm::ArrayRef<Decl *> ASTRoots,
+        llvm::ArrayRef<SymbolReference> MacroRefs, const Includes &I,
+        const PragmaIncludes *PI, const SourceManager &SM,
+        const HeaderSearch &HS,
+        llvm::function_ref<bool(llvm::StringRef)> HeaderFilter = nullptr);
 
 /// Removes unused includes and inserts missing ones in the main file.
 /// Returns the modified main-file code.
 /// The FormatStyle must be C++ or ObjC (to support include ordering).
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
                         const format::FormatStyle &IncludeStyle);
 
 /// Gets all the providers for a symbol by traversing each location.

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseMapInfoVariant.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include <memory>
 #include <utility>
 #include <variant>
 #include <vector>
 
 namespace llvm {
 class raw_ostream;
 } // namespace llvm
struct Header {
   bool operator<(const Header &RHS) const;
 
   const FileEntry *physical() const { return std::get<Physical>(Storage); }
   tooling::stdlib::Header standard() const {
     return std::get<Standard>(Storage);
   }
   StringRef verbatim() const { return std::get<Verbatim>(Storage); }
 
+  /// Absolute path for the header when it's a physical file. Otherwise just
+  /// the spelling without surrounding quotes/brackets.
+  llvm::StringRef resolvedPath() const;
+
 private:
   // Order must match Kind enum!
   std::variant<const FileEntry *, tooling::stdlib::Header, StringRef> Storage;
 
   // Disambiguation tag to make sure we can call the right constructor from
   // DenseMapInfo methods.
   struct SentinelTag {};
   Header(SentinelTag, decltype(Storage) Sentinel)

clang-tools-extra/include-cleaner/lib/Analysis.cpp

void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
   for (const SymbolReference &MacroRef : MacroRefs) {
     assert(MacroRef.Target.kind() == Symbol::Macro);
     if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)))
       continue;
     CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI));
   }
 }
 
-AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
-                        llvm::ArrayRef<SymbolReference> MacroRefs,
-                        const Includes &Inc, const PragmaIncludes *PI,
-                        const SourceManager &SM, const HeaderSearch &HS) {
+AnalysisResults
+analyze(llvm::ArrayRef<Decl *> ASTRoots,
+        llvm::ArrayRef<SymbolReference> MacroRefs, const Includes &Inc,
+        const PragmaIncludes *PI, const SourceManager &SM,
+        const HeaderSearch &HS,
+        llvm::function_ref<bool(llvm::StringRef)> HeaderFilter) {
   const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
   llvm::DenseSet<const Include *> Used;
   llvm::StringSet<> Missing;
+  if (!HeaderFilter)
+    HeaderFilter = [](llvm::StringRef) { return false; };
   walkUsed(ASTRoots, MacroRefs, PI, SM,
            [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
              bool Satisfied = false;
              for (const Header &H : Providers) {
                if (H.kind() == Header::Physical && H.physical() == MainFile)
                  Satisfied = true;
                for (const Include *I : Inc.match(H)) {
                  Used.insert(I);
                  Satisfied = true;
                }
              }
              if (!Satisfied && !Providers.empty() &&
-                 Ref.RT == RefType::Explicit)
+                 Ref.RT == RefType::Explicit &&
+                 !HeaderFilter(Providers.front().resolvedPath()))
                Missing.insert(spellHeader({Providers.front(), HS, MainFile}));
            });
 
   AnalysisResults Results;
   for (const Include &I : Inc.all()) {
-    if (Used.contains(&I) || !I.Resolved)
+    if (Used.contains(&I) || !I.Resolved ||
+        HeaderFilter(I.Resolved->tryGetRealPathName()))
       continue;
     if (PI) {
       if (PI->shouldKeep(I.Line))
         continue;
       // Check if main file is the public interface for a private header. If so
       // we shouldn't diagnose it as unused.
       if (auto PHeader = PI->getPublic(I.Resolved); !PHeader.empty()) {
         PHeader = PHeader.trim("<>\"");

clang-tools-extra/include-cleaner/lib/Types.cpp

if (const auto *ND = llvm::dyn_cast<NamedDecl>(&S.declaration()))
       return OS << ND->getQualifiedNameAsString();
     return OS << S.declaration().getDeclKindName();
   case Symbol::Macro:
     return OS << S.macro().Name->getName();
   }
   llvm_unreachable("Unhandled Symbol kind");
 }
 
+llvm::StringRef Header::resolvedPath() const {
+  switch (kind()) {
+  case include_cleaner::Header::Physical:
+    return physical()->tryGetRealPathName();
+  case include_cleaner::Header::Standard:
+    return standard().name().trim("<>\"");
+  case include_cleaner::Header::Verbatim:
+    return verbatim().trim("<>\"");
+  }
+  llvm_unreachable("Unknown header kind");
+}
+
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Header &H) {
   switch (H.kind()) {
   case Header::Physical:
     return OS << H.physical()->getName();
   case Header::Standard:
     return OS << H.standard().name();
   case Header::Verbatim:
     return OS << H.verbatim();

clang-tools-extra/include-cleaner/test/tool.cpp

 #include "foobar.h"
 
 int x = foo();
 
 //         RUN: clang-include-cleaner -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=CHANGE %s
 //      CHANGE: - "foobar.h"
 // CHANGE-NEXT: + "foo.h"
 
 //         RUN: clang-include-cleaner -remove=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
 //  INSERT-NOT: - "foobar.h"
 //      INSERT: + "foo.h"
 
 //         RUN: clang-include-cleaner -insert=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
 //      REMOVE: - "foobar.h"
 //  REMOVE-NOT: + "foo.h"
 
+//        RUN: clang-include-cleaner -print=changes %s --ignore-headers="foobar\.h,foo\.h" -- -I%S/Inputs/ | FileCheck --match-full-lines --allow-empty --check-prefix=IGNORE %s
+// IGNORE-NOT: - "foobar.h"
+// IGNORE-NOT: + "foo.h"
+
+//        RUN: clang-include-cleaner -print=changes %s --ignore-headers="foobar.*\.h" -- -I%S/Inputs/ | FileCheck --match-full-lines --allow-empty --check-prefix=IGNORE2 %s
+// IGNORE2-NOT: - "foobar.h"
+//     IGNORE2: + "foo.h"
+
 //        RUN: clang-include-cleaner -print %s -- -I%S/Inputs/ | FileCheck --match-full-lines --check-prefix=PRINT %s
 //      PRINT: #include "foo.h"
 //  PRINT-NOT: {{^}}#include "foobar.h"{{$}}
 
 //        RUN: cp %s %t.cpp
 //        RUN: clang-include-cleaner -edit %t.cpp -- -I%S/Inputs/
 //        RUN: FileCheck --match-full-lines --check-prefix=EDIT %s < %t.cpp
 //       EDIT: #include "foo.h"
 //   EDIT-NOT: {{^}}#include "foobar.h"{{$}}
+
+//        RUN: cp %s %t.cpp
+//        RUN: clang-include-cleaner -edit --ignore-headers="foobar\.h,foo\.h" %t.cpp -- -I%S/Inputs/ 
+//        RUN: FileCheck --match-full-lines --check-prefix=EDIT2 %s < %t.cpp
+//  EDIT2-NOT: {{^}}#include "foo.h"{{$}}

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

 //===--- IncludeCleaner.cpp - standalone tool for include analysis --------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
+#include <functional>
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
 
 namespace clang {
 namespace include_cleaner {
 namespace {
 namespace cl = llvm::cl;
 
 llvm::StringRef Overview = llvm::StringLiteral(R"(
 clang-include-cleaner analyzes the #include directives in source code.

 
 cl::opt<std::string> HTMLReportPath{
     "html",
     cl::desc("Specify an output filename for an HTML report. "
              "This describes both recommendations and reasons for changes."),
     cl::cat(IncludeCleaner),
 };
 
+cl::opt<std::string> IgnoreHeaders{
+    "ignore-headers",
+    cl::desc("A comma-separated list of regexes to match against suffix of a "
+             "header, and disable analysis if matched."),
+    cl::init(""),
+    cl::cat(IncludeCleaner),
+};
+
 enum class PrintStyle { Changes, Final };
 cl::opt<PrintStyle> Print{
     "print",
     cl::values(
         clEnumValN(PrintStyle::Changes, "changes", "Print symbolic changes"),
         clEnumValN(PrintStyle::Final, "", "Print final code")),
     cl::ValueOptional,
     cl::init(PrintStyle::Final),
format::FormatStyle getStyle(llvm::StringRef Filename) {
   if (!S || !S->isCpp()) {
     consumeError(S.takeError());
     return format::getLLVMStyle();
   }
   return std::move(*S);
 }
 
 class Action : public clang::ASTFrontendAction {
+public:
+  Action(llvm::function_ref<bool(llvm::StringRef)> HeaderFilter)
+      : HeaderFilter(HeaderFilter){};
+
+private:
   RecordedAST AST;
   RecordedPP PP;
   PragmaIncludes PI;
+  llvm::function_ref<bool(llvm::StringRef)> HeaderFilter;
 
   bool BeginInvocation(CompilerInstance &CI) override {
     // We only perform include-cleaner analysis. So we disable diagnostics that
     // won't affect our analysis to make the tool more robust against
     // in-development code.
     CI.getLangOpts().ModulesDeclUse = false;
     CI.getLangOpts().ModulesStrictDeclUse = false;
     return true;
if (!HTMLReportPath.empty())
       writeHTML();
 
     auto &HS = getCompilerInstance().getPreprocessor().getHeaderSearchInfo();
     llvm::StringRef Path =
         SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
     assert(!Path.empty() && "Main file path not known?");
     llvm::StringRef Code = SM.getBufferData(SM.getMainFileID());
 
-    auto Results =
-        analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS);
+    auto Results = analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM,
+                           HS, HeaderFilter);
     if (!Insert)
       Results.Missing.clear();
     if (!Remove)
       Results.Unused.clear();
     std::string Final = fixIncludes(Results, Code, getStyle(Path));
 
     if (Print.getNumOccurrences()) {
       switch (Print) {
       case PrintStyle::Changes:
         for (const Include *I : Results.Unused)
           llvm::outs() << "- " << I->quote() << " @Line:" << I->Line << "\n";
         for (const auto &I : Results.Missing)
           llvm::outs() << "+ " << I << "\n";
         break;
       case PrintStyle::Final:
         llvm::outs() << Final;
         break;
       }
if (EC) {
       return;
     }
     writeHTMLReport(
         AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
         PP.MacroReferences, *AST.Ctx,
         getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, OS);
   }
 };
+class ActionFactory : public tooling::FrontendActionFactory {
+public:
+  ActionFactory(llvm::function_ref<bool(llvm::StringRef Path)> HeaderFilter)
+      : HeaderFilter(HeaderFilter) {}
+
+  std::unique_ptr<clang::FrontendAction> create() override {
+    return std::make_unique<Action>(HeaderFilter);
+  }
+
+private:
+  llvm::function_ref<bool(llvm::StringRef)> HeaderFilter;
+};
+
+std::function<bool(llvm::StringRef)> headerFilter() {
+  auto FilterRegs = std::make_shared<std::vector<llvm::Regex>>();
+
+  llvm::SmallVector<llvm::StringRef> Headers;
+  llvm::StringRef(IgnoreHeaders).split(Headers, ',', -1, /*KeepEmpty=*/false);
+  for (auto HeaderPattern : Headers) {
+    std::string AnchoredPattern = "(" + HeaderPattern.str() + ")$";
+    llvm::Regex CompiledRegex(AnchoredPattern);
+    std::string RegexError;
+    if (!CompiledRegex.isValid(RegexError)) {
+      llvm::errs() << llvm::formatv("Invalid regular expression '{0}': {1}\n",
+                                    HeaderPattern, RegexError);
+      return nullptr;
+    }
+    FilterRegs->push_back(std::move(CompiledRegex));
+  }
+  return [FilterRegs](llvm::StringRef Path) {
+    for (const auto &F : *FilterRegs) {
+      if (F.match(Path))
+        return true;
+    }
+    return false;
+  };
+}
 
 } // namespace
 } // namespace include_cleaner
 } // namespace clang
 
 int main(int argc, const char **argv) {
   using namespace clang::include_cleaner;
 
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   auto OptionsParser =
       clang::tooling::CommonOptionsParser::create(argc, argv, IncludeCleaner);
   if (!OptionsParser) {
     llvm::errs() << toString(OptionsParser.takeError());
     return 1;
   }
-
   if (OptionsParser->getSourcePathList().size() != 1) {
     std::vector<cl::Option *> IncompatibleFlags = {&HTMLReportPath, &Print};
     for (const auto *Flag : IncompatibleFlags) {
       if (Flag->getNumOccurrences()) {
         llvm::errs() << "-" << Flag->ArgStr << " requires a single input file";
         return 1;
       }
     }
   }
-  auto Factory = clang::tooling::newFrontendActionFactory<Action>();
+  auto HeaderFilter = headerFilter();
+  ActionFactory Factory(HeaderFilter);
   return clang::tooling::ClangTool(OptionsParser->getCompilations(),
                                    OptionsParser->getSourcePathList())
-             .run(Factory.get()) ||
+             .run(&Factory) ||
          Errors != 0;
 }