Fix

Author: zeyi2

clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp

@@ -36,17 +36,7 @@ class DuplicateIncludeCallbacks : public PPCallbacks {
 public:
   DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check,
                             const SourceManager &SM,
-                            const std::vector<StringRef> &IgnoredList)
-      : Check(Check), SM(SM) {
-    // The main file doesn't participate in the FileChanged notification.
-    Files.emplace_back();
-
-    AllowedRegexes.reserve(IgnoredList.size());
-    for (const StringRef &It : IgnoredList) {
-      if (!It.empty())
-        AllowedRegexes.emplace_back(It);
-    }
-  }
+                            const std::vector<StringRef> &IgnoredList);
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
                    SrcMgr::CharacteristicKind FileType,
@@ -67,26 +57,12 @@ class DuplicateIncludeCallbacks : public PPCallbacks {
                       const MacroDirective *Undef) override;
 
 private:
+  bool isAllowedDuplicate(StringRef FileName, OptionalFileEntryRef File) const;
   // A list of included files is kept for each file we enter.
   SmallVector<FileList> Files;
   DuplicateIncludeCheck &Check;
   const SourceManager &SM;
   std::vector<llvm::Regex> AllowedRegexes;
-
-  bool isAllowedDuplicate(StringRef FileName, OptionalFileEntryRef File) const {
-    if (llvm::any_of(AllowedRegexes,
-                     [&](const llvm::Regex &R) { return R.match(FileName); }))
-      return true;
-
-    if (File) {
-      const StringRef Resolved = File->getName();
-      if (llvm::any_of(AllowedRegexes,
-                       [&](const llvm::Regex &R) { return R.match(Resolved); }))
-        return true;
-    }
-
-    return false;
-  }
 };
 
 } // namespace
@@ -97,6 +73,37 @@ DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name,
       IgnoredFilesList(utils::options::parseStringList(
           Options.get("IgnoredFilesList", ""))) {}
 
+DuplicateIncludeCallbacks::DuplicateIncludeCallbacks(
+    DuplicateIncludeCheck &Check, const SourceManager &SM,
+    const std::vector<StringRef> &IgnoredList)
+    : Check(Check), SM(SM) {
+  // The main file doesn't participate in the FileChanged notification.
+  Files.emplace_back();
+
+  AllowedRegexes.reserve(IgnoredList.size());
+  for (const StringRef &It : IgnoredList) {
+    if (!It.empty())
+      AllowedRegexes.emplace_back(It);
+  }
+}
+
+bool DuplicateIncludeCallbacks::isAllowedDuplicate(
+    StringRef FileName, OptionalFileEntryRef File) const {
+  if (llvm::any_of(AllowedRegexes, [&FileName](const llvm::Regex &R) {
+        return R.match(FileName);
+      }))
+    return true;
+
+  if (File) {
+    const StringRef Resolved = File->getName();
+    return llvm::any_of(AllowedRegexes, [&Resolved](const llvm::Regex &R) {
+      return R.match(Resolved);
+    });
+  }
+
+  return false;
+}
+
 void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc,
                                             FileChangeReason Reason,
                                             SrcMgr::CharacteristicKind FileType,
@@ -117,9 +124,8 @@ void DuplicateIncludeCallbacks::InclusionDirective(
       FilenameRange.getEnd().isMacroID())
     return;
   if (llvm::is_contained(Files.back(), FileName)) {
-    if (isAllowedDuplicate(FileName, File)) {
+    if (isAllowedDuplicate(FileName, File))
       return;
-    }
     // We want to delete the entire line, so make sure that [Start,End] covers
     // everything.
     const SourceLocation Start =

clang-tools-extra/docs/clang-tidy/checks/readability/duplicate-include.rst

@@ -7,17 +7,6 @@ Looks for duplicate includes and removes them.  The check maintains a list of
 included files and looks for duplicates.  If a macro is defined or undefined
 then the list of included files is cleared.
 
-Options
--------
-
-.. option:: IgnoredFilesList
-
-  A semicolon-separated list of regular expressions or filenames that are
-  allowed to be included multiple times without diagnostics. Matching is
-  performed against the textual include name. If the header can be
-  resolved, its full path is also matched against the patterns.
-  Default is empty.
-
 Examples:
 
 .. code-block:: c++
@@ -44,3 +33,12 @@ Because of the intervening macro definitions, this code remains unchanged:
   #define NDEBUG
   #include "assertion.h"
   // ...code with assertions disabled
+
+Options
+-------
+
+.. option:: IgnoredFilesList
+
+  A semicolon-separated list of regular expressions or filenames that are
+  allowed to be included multiple times without diagnostics. Matching is
+  performed against the textual include name. Default is an empty string.

clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-ignored-files.cpp

@@ -1,9 +1,24 @@
 // RUN: %check_clang_tidy %s readability-duplicate-include %t -- \
-// RUN:   -config="{CheckOptions: {readability-duplicate-include.IgnoredFilesList: 'pack_.*\\.h'}}" \
+// RUN:   -config="{CheckOptions: {readability-duplicate-include.IgnoredFilesList: 'pack_.*\\.h'}}" -header-filter='' \
 // RUN:   -- -I %S/Inputs/duplicate-include
 
+int g;
+#include "duplicate-include.h"
+int h;
+#include "duplicate-include.h"
+int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include]
+// CHECK-FIXES:      int g;
+// CHECK-FIXES-NEXT: #include "duplicate-include.h"
+// CHECK-FIXES-NEXT: int h;
+// CHECK-FIXES-NEXT: int i;
+
 #include "pack_begin.h"
 struct A { int x; };
 #include "pack_end.h"
 
-// no warning
+#include "pack_begin.h"
+struct B { int x; };
+#include "pack_end.h"
+
+// No warning here.