[clang-tidy][IncludeCleaner] Fix analysis supression in presence of verbatim spellings (llvm#68185)

Author: kadircet

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

@@ -30,6 +30,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
+#include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
@@ -97,9 +98,12 @@ bool IncludeCleanerCheck::shouldIgnore(const include_cleaner::Header &H) {
   return llvm::any_of(IgnoreHeadersRegex, [&H](const llvm::Regex &R) {
     switch (H.kind()) {
     case include_cleaner::Header::Standard:
+      // We don't trim angle brackets around standard library headers
+      // deliberately, so that they are only matched as <vector>, otherwise
+      // having just `.*/vector` might yield false positives.
       return R.match(H.standard().name());
     case include_cleaner::Header::Verbatim:
-      return R.match(H.verbatim());
+      return R.match(H.verbatim().trim("<>\""));
     case include_cleaner::Header::Physical:
       return R.match(H.physical().getFileEntry().tryGetRealPathName());
     }
@@ -179,12 +183,14 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
       if (getCurrentMainFile().endswith(PHeader))
         continue;
     }
-
-    if (llvm::none_of(
-            IgnoreHeadersRegex,
-            [Resolved = (*I.Resolved).getFileEntry().tryGetRealPathName()](
-                const llvm::Regex &R) { return R.match(Resolved); }))
-      Unused.push_back(&I);
+    auto StdHeader = tooling::stdlib::Header::named(
+        I.quote(), PP->getLangOpts().CPlusPlus ? tooling::stdlib::Lang::CXX
+                                               : tooling::stdlib::Lang::C);
+    if (StdHeader && shouldIgnore(*StdHeader))
+      continue;
+    if (shouldIgnore(*I.Resolved))
+      continue;
+    Unused.push_back(&I);
   }
 
   llvm::StringRef Code = SM->getBufferData(SM->getMainFileID());

clang-tools-extra/docs/ReleaseNotes.rst

@@ -249,14 +249,12 @@ Changes in existing checks
 - Improved :doc:`misc-const-correctness
   <clang-tidy/checks/misc/const-correctness>` check to avoid false positive when
   using pointer to member function.
-  
-- Improved :doc:`misc-include-cleaner
-  <clang-tidy/checks/misc/include-cleaner>` check by adding option
-  `DeduplicateFindings` to output one finding per symbol occurrence.
 
 - Improved :doc:`misc-include-cleaner
-  <clang-tidy/checks/misc/include-cleaner>` check to avoid fixes insert
-  same include header multiple times.
+  <clang-tidy/checks/misc/include-cleaner>` check by adding option
+  `DeduplicateFindings` to output one finding per symbol occurrence, avoid
+  inserting the same header multiple times, fix a bug where `IgnoreHeaders`
+  option won't work with verbatim/std headers.
 
 - Improved :doc:`misc-redundant-expression
   <clang-tidy/checks/misc/redundant-expression>` check to ignore

clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp

@@ -59,18 +59,20 @@ TEST(IncludeCleanerCheckTest, SuppressUnusedIncludes) {
 #include "foo/qux.h"
 #include "baz/qux/qux.h"
 #include <vector>
+#include <list>
 )";
 
   const char *PostCode = R"(
 #include "bar.h"
 #include "foo/qux.h"
 #include <vector>
+#include <list>
 )";
 
   std::vector<ClangTidyError> Errors;
   ClangTidyOptions Opts;
   Opts.CheckOptions["IgnoreHeaders"] = llvm::StringRef{llvm::formatv(
-      "bar.h;{0};{1};vector",
+      "bar.h;{0};{1};vector;<list>;",
       llvm::Regex::escape(appendPathFileSystemIndependent({"foo", "qux.h"})),
       llvm::Regex::escape(appendPathFileSystemIndependent({"baz", "qux"})))};
   EXPECT_EQ(
@@ -79,6 +81,7 @@ TEST(IncludeCleanerCheckTest, SuppressUnusedIncludes) {
           PreCode, &Errors, "file.cpp", std::nullopt, Opts,
           {{"bar.h", "#pragma once"},
            {"vector", "#pragma once"},
+           {"list", "#pragma once"},
            {appendPathFileSystemIndependent({"foo", "qux.h"}), "#pragma once"},
            {appendPathFileSystemIndependent({"baz", "qux", "qux.h"}),
             "#pragma once"}}));
@@ -163,30 +166,37 @@ TEST(IncludeCleanerCheckTest, SuppressMissingIncludes) {
 int BarResult = bar();
 int BazResult = baz();
 int QuxResult = qux();
+int PrivResult = test();
+std::vector x;
 )";
 
   ClangTidyOptions Opts;
   Opts.CheckOptions["IgnoreHeaders"] = llvm::StringRef{
-      "baz.h;" +
+      "public.h;<vector>;baz.h;" +
       llvm::Regex::escape(appendPathFileSystemIndependent({"foo", "qux.h"}))};
   std::vector<ClangTidyError> Errors;
   EXPECT_EQ(PreCode, runCheckOnCode<IncludeCleanerCheck>(
                          PreCode, &Errors, "file.cpp", std::nullopt, Opts,
                          {{"bar.h", R"(#pragma once
                               #include "baz.h"
                               #include "foo/qux.h"
+                              #include "private.h"
                               int bar();
+                              namespace std { struct vector {}; }
                            )"},
                           {"baz.h", R"(#pragma once
                               int baz();
                            )"},
+                          {"private.h", R"(#pragma once
+                              // IWYU pragma: private, include "public.h"
+                              int test();
+                           )"},
                           {appendPathFileSystemIndependent({"foo", "qux.h"}),
                            R"(#pragma once
                               int qux();
                            )"}}));
 }
 
-
 TEST(IncludeCleanerCheckTest, MultipleTimeMissingInclude) {
   const char *PreCode = R"(
 #include "bar.h"