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

[Congcong Cai via cfe-commits]

https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/65431:

>From 2b727285edb91a4a88add118745eabc08da9c6fd Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 6 Sep 2023 09:55:20 +0800
Subject: [PATCH 1/2] [clang-tidy][misc-include-cleaner]Avoid fixes insert same
 include header multiple times

Fixed: #65285
---
 .../clang-tidy/misc/IncludeCleanerCheck.cpp   | 21 +++++++-----
 clang-tools-extra/docs/ReleaseNotes.rst       |  3 +-
 .../clang-tidy/IncludeCleanerTest.cpp         | 32 +++++++++++++++++++
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index 2658c4b38702ca8..d8afe451c99bb7c 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -199,6 +199,9 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
 
   tooling::HeaderIncludes HeaderIncludes(getCurrentMainFile(), Code,
                                          FileStyle->IncludeStyle);
+  // `tooling::HeaderIncludes::insert` will not modify `ExistingIncludes`. We
+  // should handle repeat include here
+  std::set<const std::string> InsertedHeader{};
   for (const auto &Inc : Missing) {
     std::string Spelling = include_cleaner::spellHeader(
         {Inc.Missing, PP->getHeaderSearchInfo(), MainFile});
@@ -209,14 +212,16 @@ 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 (InsertedHeader.insert(Replacement->getReplacementText().str()).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..571a50e75bc9b0e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,7 +224,8 @@ Changes in existing checks
 
 - Improved :doc:`misc-include-cleaner
   <clang-tidy/checks/misc/include-cleaner>` check by adding option
-  `DeduplicateFindings` to output one finding per symbol occurrence.
+  `DeduplicateFindings` to output one finding per symbol occurrence 
+  and avoid fixes insert same include header multiple times.
 
 - Improved :doc:`misc-redundant-expression
   <clang-tidy/checks/misc/redundant-expression>` check to ignore
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>

>From 403b03e2ced5b70fc10aa003b9f72e269ed61dad Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 6 Sep 2023 13:38:24 +0800
Subject: [PATCH 2/2] add areDiagsSelfContained check

---
 clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index d8afe451c99bb7c..bbb244a5c31bfe7 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"
@@ -201,7 +202,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
                                          FileStyle->IncludeStyle);
   // `tooling::HeaderIncludes::insert` will not modify `ExistingIncludes`. We
   // should handle repeat include here
-  std::set<const std::string> InsertedHeader{};
+  llvm::StringSet<> InsertedHeaders{};
   for (const auto &Inc : Missing) {
     std::string Spelling = include_cleaner::spellHeader(
         {Inc.Missing, PP->getHeaderSearchInfo(), MainFile});
@@ -217,7 +218,8 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
           diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
                "no header providing \"%0\" is directly included")
           << Inc.SymRef.Target.name();
-      if (InsertedHeader.insert(Replacement->getReplacementText().str()).second)
+      if (areDiagsSelfContained() ||
+          InsertedHeaders.insert(Replacement->getReplacementText()).second)
         DB << FixItHint::CreateInsertion(
             SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()),
             Replacement->getReplacementText());