[clang-tools-extra] b0831c3 - [clang-tidy][misc-include-cleaner]Avoid to insert same include header multiple times (#65431)

[via cfe-commits]

Author: Congcong Cai
Date: 2023-09-06T17:37:14+08:00
New Revision: b0831c3996752038c375796d8ebb4f471f1ea251

URL: https://github.com/llvm/llvm-project/commit/b0831c3996752038c375796d8ebb4f471f1ea251
DIFF: https://github.com/llvm/llvm-project/commit/b0831c3996752038c375796d8ebb4f471f1ea251.diff

LOG: [clang-tidy][misc-include-cleaner]Avoid to insert same include header multiple times (#65431)

`HeaderIncludes` won't update `ExistingIncludes` during inserting.
We need to manage it in tidy check.

Fixed: #65285

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index 2658c4b38702ca8..8d5f400acfef8bc 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -33,6 +33,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
@@ -199,6 +200,8 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
 
   tooling::HeaderIncludes HeaderIncludes(getCurrentMainFile(), Code,
                                          FileStyle->IncludeStyle);
+  // Deduplicate insertions when running in bulk fix mode.
+  llvm::StringSet<> InsertedHeaders{};
   for (const auto &Inc : Missing) {
     std::string Spelling = include_cleaner::spellHeader(
         {Inc.Missing, PP->getHeaderSearchInfo(), MainFile});
@@ -209,14 +212,18 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
     // main file.
     if (auto Replacement =
             HeaderIncludes.insert(llvm::StringRef{Spelling}.trim("\"<>"),
-                                  Angled, tooling::IncludeDirective::Include))
-      diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
-           "no header providing \"%0\" is directly included")
-          << Inc.SymRef.Target.name()
-          << FixItHint::CreateInsertion(
-                 SM->getComposedLoc(SM->getMainFileID(),
-                                    Replacement->getOffset()),
-                 Replacement->getReplacementText());
+                                  Angled, tooling::IncludeDirective::Include)) {
+      DiagnosticBuilder DB =
+          diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
+               "no header providing \"%0\" is directly included")
+          << Inc.SymRef.Target.name();
+      if (areDiagsSelfContained() ||
+          InsertedHeaders.insert(Replacement->getReplacementText()).second) {
+        DB << FixItHint::CreateInsertion(
+            SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()),
+            Replacement->getReplacementText());
+      }
+    }
   }
 }
 

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5dfda9928aca20b..a2cde526a8c04d9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -226,6 +226,10 @@ Changes in existing checks
   <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.
+
 - Improved :doc:`misc-redundant-expression
   <clang-tidy/checks/misc/redundant-expression>` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``).

diff  --git a/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp b/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp
index fe3e38958f8985a..f84133b01a3a49f 100644
--- a/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp
@@ -184,6 +184,38 @@ int QuxResult = qux();
                            )"}}));
 }
 
+
+TEST(IncludeCleanerCheckTest, MultipleTimeMissingInclude) {
+  const char *PreCode = R"(
+#include "bar.h"
+
+int BarResult = bar();
+int BazResult_0 = baz_0();
+int BazResult_1 = baz_1();
+)";
+  const char *PostCode = R"(
+#include "bar.h"
+#include "baz.h"
+
+int BarResult = bar();
+int BazResult_0 = baz_0();
+int BazResult_1 = baz_1();
+)";
+
+  std::vector<ClangTidyError> Errors;
+  EXPECT_EQ(PostCode,
+            runCheckOnCode<IncludeCleanerCheck>(
+                PreCode, &Errors, "file.cpp", std::nullopt, ClangTidyOptions(),
+                {{"bar.h", R"(#pragma once
+                              #include "baz.h"
+                              int bar();
+                           )"},
+                 {"baz.h", R"(#pragma once
+                              int baz_0();
+                              int baz_1();
+                           )"}}));
+}
+
 TEST(IncludeCleanerCheckTest, SystemMissingIncludes) {
   const char *PreCode = R"(
 #include <vector>