[clang-tools-extra] [clang-tidy][misc-include-cleaner]Avoid to insert same include header multiple times (PR #65431)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 6 02:02:05 PDT 2023
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/3] [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 2658c4b38702ca..d8afe451c99bb7 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 5dfda9928aca20..571a50e75bc9b0 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 fe3e38958f8985..f84133b01a3a49 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/3] 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 d8afe451c99bb7..bbb244a5c31bfe 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());
>From b531a93307889863e03bfdea96fa92cd876ad2ff Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 6 Sep 2023 16:54:04 +0800
Subject: [PATCH 3/3] docs and format
---
clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp | 6 +++---
clang-tools-extra/docs/ReleaseNotes.rst | 7 +++++--
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index bbb244a5c31bfe..8d5f400acfef8b 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -200,8 +200,7 @@ 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
+ // Deduplicate insertions when running in bulk fix mode.
llvm::StringSet<> InsertedHeaders{};
for (const auto &Inc : Missing) {
std::string Spelling = include_cleaner::spellHeader(
@@ -219,10 +218,11 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
"no header providing \"%0\" is directly included")
<< Inc.SymRef.Target.name();
if (areDiagsSelfContained() ||
- InsertedHeaders.insert(Replacement->getReplacementText()).second)
+ 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 571a50e75bc9b0..a2cde526a8c04d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,8 +224,11 @@ 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
- and avoid fixes insert same include header multiple times.
+ `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
More information about the cfe-commits
mailing list