[clang-tidy] Add UnusedIncludes/MissingIncludes options to misc-include-cleaner #140600
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-clang-tools-extra Author: Daan De Meyer (DaanDeMeyer) ChangesThese 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:
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
|
Member
|
@llvm/pr-subscribers-clang-tidy Author: Daan De Meyer (DaanDeMeyer) ChangesThese 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:
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
|
kadircet
approved these changes
May 20, 2025
Contributor
Author
|
@kadircet Can you land this one as well? (Feel free to give me back the commit bit if that's in your power if you want me to land stuff myself after review) |
vbvictor
reviewed
May 20, 2025
Contributor
vbvictor
left a comment
vbvictor
left a comment
There was a problem hiding this comment.
Please add an entry in ReleaseNotes.rst.
DaanDeMeyer
force-pushed
the
misc-include-checker
branch
from
16d4523 to
e68b81b
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
DaanDeMeyer
force-pushed
the
misc-include-checker
branch
2 times, most recently
from
371cbcd to
0e6f4f4
Compare
vbvictor
reviewed
May 20, 2025
DaanDeMeyer
force-pushed
the
misc-include-checker
branch
from
0e6f4f4 to
5f4601e
Compare
vbvictor
reviewed
May 20, 2025
DaanDeMeyer
force-pushed
the
misc-include-checker
branch
from
5f4601e to
900e6b2
Compare
vbvictor
reviewed
May 20, 2025
clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner-wrong-config.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
May 20, 2025
clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner-wrong-config.cpp
Outdated
Show resolved
Hide resolved
DaanDeMeyer
force-pushed
the
misc-include-checker
branch
from
900e6b2 to
60663b3
Compare
Contributor
|
Apart from failing tests, LGTM |
…de-cleaner These mimick the same options from clangd and allow using the check to only check for unused includes or missing includes.
DaanDeMeyer
force-pushed
the
misc-include-checker
branch
from
60663b3 to
e8916f6
Compare
Contributor
Author
|
@vbvictor Could you merge this one for me now that CI is green? Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These mimick the same options from clangd and allow using the check to only check for unused includes or missing includes.