llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Daan De Meyer (DaanDeMeyer) <details> <summary>Changes</summary> These mimick the same options from clangd and allow using the check to only check for unused includes or missing includes. --- Full diff: https://github.com/llvm/llvm-project/pull/140600.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp (+40-32) - (modified) clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h (+4) - (modified) clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst (+12-1) - (modified) clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp (+58) ``````````diff diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp index 7638bbc103d16..67129e87be22a 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp @@ -59,7 +59,9 @@ IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name, : ClangTidyCheck(Name, Context), IgnoreHeaders( utils::options::parseStringList(Options.get("IgnoreHeaders", ""))), - DeduplicateFindings(Options.get("DeduplicateFindings", true)) { + DeduplicateFindings(Options.get("DeduplicateFindings", true)), + UnusedIncludes(Options.get("UnusedIncludes", true)), + MissingIncludes(Options.get("MissingIncludes", true)) { for (const auto &Header : IgnoreHeaders) { if (!llvm::Regex{Header}.isValid()) configurationDiag("Invalid ignore headers regex '%0'") << Header; @@ -74,6 +76,8 @@ void IncludeCleanerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreHeaders", utils::options::serializeStringList(IgnoreHeaders)); Options.store(Opts, "DeduplicateFindings", DeduplicateFindings); + Options.store(Opts, "UnusedIncludes", UnusedIncludes); + Options.store(Opts, "MissingIncludes", MissingIncludes); } bool IncludeCleanerCheck::isLanguageVersionSupported( @@ -200,39 +204,43 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) { if (!FileStyle) FileStyle = format::getLLVMStyle(); - for (const auto *Inc : Unused) { - diag(Inc->HashLocation, "included header %0 is not used directly") - << llvm::sys::path::filename(Inc->Spelled, - llvm::sys::path::Style::posix) - << FixItHint::CreateRemoval(CharSourceRange::getCharRange( - SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1), - SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1))); + if (UnusedIncludes) { + for (const auto *Inc : Unused) { + diag(Inc->HashLocation, "included header %0 is not used directly") + << llvm::sys::path::filename(Inc->Spelled, + llvm::sys::path::Style::posix) + << FixItHint::CreateRemoval(CharSourceRange::getCharRange( + SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1), + SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1))); + } } - 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}); - bool Angled = llvm::StringRef{Spelling}.starts_with("<"); - // We might suggest insertion of an existing include in edge cases, e.g., - // include is present in a PP-disabled region, or spelling of the header - // turns out to be the same as one of the unresolved includes in the - // main file. - if (auto Replacement = - HeaderIncludes.insert(llvm::StringRef{Spelling}.trim("\"<>"), - 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()); + if (MissingIncludes) { + 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}); + bool Angled = llvm::StringRef{Spelling}.starts_with("<"); + // We might suggest insertion of an existing include in edge cases, e.g., + // include is present in a PP-disabled region, or spelling of the header + // turns out to be the same as one of the unresolved includes in the + // main file. + if (auto Replacement = HeaderIncludes.insert( + llvm::StringRef{Spelling}.trim("\"<>"), 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/clang-tidy/misc/IncludeCleanerCheck.h b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h index b46e409bd6f6a..8f05887efb776 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h @@ -47,6 +47,10 @@ class IncludeCleanerCheck : public ClangTidyCheck { std::vector<StringRef> IgnoreHeaders; // Whether emit only one finding per usage of a symbol. const bool DeduplicateFindings; + // Whether to report unused includes. + const bool UnusedIncludes; + // Whether to report missing includes. + const bool MissingIncludes; llvm::SmallVector<llvm::Regex> IgnoreHeadersRegex; bool shouldIgnore(const include_cleaner::Header &H); }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst index d112f01cbc0b1..4364610787058 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst @@ -10,7 +10,7 @@ Findings correspond to https://clangd.llvm.org/design/include-cleaner. Example: .. code-block:: c++ - + // foo.h class Foo{}; // bar.h @@ -38,3 +38,14 @@ Options A boolean that controls whether the check should deduplicate findings for the same symbol. Defaults to `true`. + +.. option:: UnusedIncludes + + A boolean that controls whether the check should report unused includes + (includes that are not used directly). Defaults to `true`. + +.. option:: MissingIncludes + + A boolean that controls whether the check should report missing includes + (header files from which symbols are used but which are not directly included). + Defaults to `true`. diff --git a/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp b/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp index 3d6ec995e443d..00576916492e1 100644 --- a/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp @@ -316,6 +316,64 @@ DECLARE { )"}})); } +TEST(IncludeCleanerCheckTest, UnusedIncludes) { + const char *PreCode = R"( +#include "bar.h")"; + + { + std::vector<ClangTidyError> Errors; + runCheckOnCode<IncludeCleanerCheck>(PreCode, &Errors, "file.cpp", {}, + ClangTidyOptions(), + {{"bar.h", "#pragma once"}}); + ASSERT_THAT(Errors.size(), testing::Eq(1U)); + EXPECT_EQ(Errors.front().Message.Message, + "included header bar.h is not used directly"); + } + { + std::vector<ClangTidyError> Errors; + ClangTidyOptions Opts; + Opts.CheckOptions["test-check-0.UnusedIncludes"] = "false"; + runCheckOnCode<IncludeCleanerCheck>(PreCode, &Errors, "file.cpp", {}, Opts, + {{"bar.h", "#pragma once"}}); + ASSERT_THAT(Errors.size(), testing::Eq(0U)); + } +} + +TEST(IncludeCleanerCheckTest, MissingIncludes) { + const char *PreCode = R"( +#include "baz.h" // IWYU pragma: keep + +int BarResult1 = bar();)"; + + { + std::vector<ClangTidyError> Errors; + runCheckOnCode<IncludeCleanerCheck>(PreCode, &Errors, "file.cpp", {}, + ClangTidyOptions(), + {{"baz.h", R"(#pragma once + #include "bar.h" + )"}, + {"bar.h", R"(#pragma once + int bar(); + )"}}); + ASSERT_THAT(Errors.size(), testing::Eq(1U)); + EXPECT_EQ(Errors.front().Message.Message, + "no header providing \"bar\" is directly included"); + } + { + std::vector<ClangTidyError> Errors; + ClangTidyOptions Opts; + Opts.CheckOptions["test-check-0.MissingIncludes"] = "false"; + runCheckOnCode<IncludeCleanerCheck>(PreCode, &Errors, "file.cpp", {}, Opts, + {{"baz.h", R"(#pragma once + #include "bar.h" + )"}, + {"bar.h", R"(#pragma once + int bar(); + )"}}); + ASSERT_THAT(Errors.size(), testing::Eq(0U)); + } +} + } // namespace } // namespace test } // namespace tidy `````````` </details> https://github.com/llvm/llvm-project/pull/140600 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
