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 <congcongcai0...@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 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 2535224078f9abc579c795652ae366f496c091b2 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@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 d8afe451c99bb7..75c0aef7f6571f 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<> InsertedHeader{}; 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() || + InsertedHeader.insert(Replacement->getReplacementText()).second) DB << FixItHint::CreateInsertion( SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()), Replacement->getReplacementText()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits