[clang-tools-extra] 8c5a7a1 - [clangd] Add config option to allow detection of unused angled includes (#87208)
via cfe-commits
cfe-commits at lists.llvm.org
Wed May 29 10:30:01 PDT 2024
Author: Vadim D
Date: 2024-05-29T13:29:57-04:00
New Revision: 8c5a7a1fc4890fcae50f8e8a61d5a2e2b1ebd7e5
URL: https://github.com/llvm/llvm-project/commit/8c5a7a1fc4890fcae50f8e8a61d5a2e2b1ebd7e5
DIFF: https://github.com/llvm/llvm-project/commit/8c5a7a1fc4890fcae50f8e8a61d5a2e2b1ebd7e5.diff
LOG: [clangd] Add config option to allow detection of unused angled includes (#87208)
This PR adds a new `AnalyzeAngledIncludes` option to `Includes` section
of clangd config. This option enables unused include checks for all includes
that use the `<>` syntax, not just standard library includes.
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/docs/ReleaseNotes.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 4371c80a6c587..41143b9ebc8d2 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -110,10 +110,11 @@ struct Config {
IncludesPolicy UnusedIncludes = IncludesPolicy::Strict;
IncludesPolicy MissingIncludes = IncludesPolicy::None;
- /// IncludeCleaner will not diagnose usages of these headers matched by
- /// these regexes.
struct {
+ /// IncludeCleaner will not diagnose usages of these headers matched by
+ /// these regexes.
std::vector<std::function<bool(llvm::StringRef)>> IgnoreHeader;
+ bool AnalyzeAngledIncludes = false;
} Includes;
} Diagnostics;
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 5bb2eb4a9f803..f32f674443ffe 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -572,32 +572,46 @@ struct FragmentCompiler {
#else
static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
#endif
- auto Filters = std::make_shared<std::vector<llvm::Regex>>();
- for (auto &HeaderPattern : F.IgnoreHeader) {
- // Anchor on the right.
- std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
- llvm::Regex CompiledRegex(AnchoredPattern, Flags);
- std::string RegexError;
- if (!CompiledRegex.isValid(RegexError)) {
- diag(Warning,
- llvm::formatv("Invalid regular expression '{0}': {1}",
- *HeaderPattern, RegexError)
- .str(),
- HeaderPattern.Range);
- continue;
+ std::shared_ptr<std::vector<llvm::Regex>> Filters;
+ if (!F.IgnoreHeader.empty()) {
+ Filters = std::make_shared<std::vector<llvm::Regex>>();
+ for (auto &HeaderPattern : F.IgnoreHeader) {
+ // Anchor on the right.
+ std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
+ llvm::Regex CompiledRegex(AnchoredPattern, Flags);
+ std::string RegexError;
+ if (!CompiledRegex.isValid(RegexError)) {
+ diag(Warning,
+ llvm::formatv("Invalid regular expression '{0}': {1}",
+ *HeaderPattern, RegexError)
+ .str(),
+ HeaderPattern.Range);
+ continue;
+ }
+ Filters->push_back(std::move(CompiledRegex));
}
- Filters->push_back(std::move(CompiledRegex));
}
- if (Filters->empty())
+ // Optional to override the resulting AnalyzeAngledIncludes
+ // only if it's explicitly set in the current fragment.
+ // Otherwise it's inherited from parent fragment.
+ std::optional<bool> AnalyzeAngledIncludes;
+ if (F.AnalyzeAngledIncludes.has_value())
+ AnalyzeAngledIncludes = **F.AnalyzeAngledIncludes;
+ if (!Filters && !AnalyzeAngledIncludes.has_value())
return;
- auto Filter = [Filters](llvm::StringRef Path) {
- for (auto &Regex : *Filters)
- if (Regex.match(Path))
- return true;
- return false;
- };
- Out.Apply.push_back([Filter](const Params &, Config &C) {
- C.Diagnostics.Includes.IgnoreHeader.emplace_back(Filter);
+ Out.Apply.push_back([Filters = std::move(Filters),
+ AnalyzeAngledIncludes](const Params &, Config &C) {
+ if (Filters) {
+ auto Filter = [Filters](llvm::StringRef Path) {
+ for (auto &Regex : *Filters)
+ if (Regex.match(Path))
+ return true;
+ return false;
+ };
+ C.Diagnostics.Includes.IgnoreHeader.emplace_back(std::move(Filter));
+ }
+ if (AnalyzeAngledIncludes.has_value())
+ C.Diagnostics.Includes.AnalyzeAngledIncludes = *AnalyzeAngledIncludes;
});
}
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 7fa61108c78a0..f3e51a9b6dbc4 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -254,6 +254,10 @@ struct Fragment {
/// unused or missing. These can match any suffix of the header file in
/// question.
std::vector<Located<std::string>> IgnoreHeader;
+
+ /// If false (default), unused system headers will be ignored.
+ /// Standard library headers are analyzed regardless of this option.
+ std::optional<Located<bool>> AnalyzeAngledIncludes;
};
IncludesBlock Includes;
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index ce09af819247a..3e9b6a07d3b32 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -169,6 +169,10 @@ class Parser {
if (auto Values = scalarValues(N))
F.IgnoreHeader = std::move(*Values);
});
+ Dict.handle("AnalyzeAngledIncludes", [&](Node &N) {
+ if (auto Value = boolValue(N, "AnalyzeAngledIncludes"))
+ F.AnalyzeAngledIncludes = *Value;
+ });
Dict.parse(N);
}
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 8e48f546d94e7..01b47679790f1 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -68,24 +68,30 @@ bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter IgnoreHeaders) {
}
bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
- const include_cleaner::PragmaIncludes *PI) {
+ const include_cleaner::PragmaIncludes *PI,
+ bool AnalyzeAngledIncludes) {
assert(Inc.HeaderID);
auto HID = static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID);
auto FE = AST.getSourceManager().getFileManager().getFileRef(
AST.getIncludeStructure().getRealPath(HID));
assert(FE);
if (FE->getDir() == AST.getPreprocessor()
- .getHeaderSearchInfo()
- .getModuleMap()
- .getBuiltinDir())
+ .getHeaderSearchInfo()
+ .getModuleMap()
+ .getBuiltinDir())
return false;
if (PI && PI->shouldKeep(*FE))
return false;
// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
// System headers are likely to be standard library headers.
- // Until we have good support for umbrella headers, don't warn about them.
- if (Inc.Written.front() == '<')
- return tooling::stdlib::Header::named(Inc.Written).has_value();
+ // Until we have good support for umbrella headers, don't warn about them
+ // (unless analysis is explicitly enabled).
+ if (Inc.Written.front() == '<') {
+ if (tooling::stdlib::Header::named(Inc.Written))
+ return true;
+ if (!AnalyzeAngledIncludes)
+ return false;
+ }
if (PI) {
// Check if main file is the public interface for a private header. If so we
// shouldn't diagnose it as unused.
@@ -266,7 +272,8 @@ Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) {
std::vector<const Inclusion *>
getUnused(ParsedAST &AST,
- const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles) {
+ const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
+ bool AnalyzeAngledIncludes) {
trace::Span Tracer("IncludeCleaner::getUnused");
std::vector<const Inclusion *> Unused;
for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
@@ -275,7 +282,8 @@ getUnused(ParsedAST &AST,
auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID);
if (ReferencedFiles.contains(IncludeID))
continue;
- if (!mayConsiderUnused(MFI, AST, &AST.getPragmaIncludes())) {
+ if (!mayConsiderUnused(MFI, AST, &AST.getPragmaIncludes(),
+ AnalyzeAngledIncludes)) {
dlog("{0} was not used, but is not eligible to be diagnosed as unused",
MFI.Written);
continue;
@@ -347,7 +355,8 @@ include_cleaner::Includes convertIncludes(const ParsedAST &AST) {
return ConvertedIncludes;
}
-IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
+IncludeCleanerFindings
+computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeAngledIncludes) {
// Interaction is only polished for C/CPP.
if (AST.getLangOpts().ObjC)
return {};
@@ -432,7 +441,8 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
MapInfo::getHashValue(RHS.Symbol);
});
MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
- std::vector<const Inclusion *> UnusedIncludes = getUnused(AST, Used);
+ std::vector<const Inclusion *> UnusedIncludes =
+ getUnused(AST, Used, AnalyzeAngledIncludes);
return {std::move(UnusedIncludes), std::move(MissingIncludes)};
}
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index 624e2116be7da..a01146d14e3c1 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -53,7 +53,9 @@ struct IncludeCleanerFindings {
std::vector<MissingIncludeDiagInfo> MissingIncludes;
};
-IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST);
+IncludeCleanerFindings
+computeIncludeCleanerFindings(ParsedAST &AST,
+ bool AnalyzeAngledIncludes = false);
using HeaderFilter = llvm::ArrayRef<std::function<bool(llvm::StringRef)>>;
std::vector<Diag>
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 3ff759415f7c8..2bd1fbcad2ada 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -373,7 +373,8 @@ std::vector<Diag> getIncludeCleanerDiags(ParsedAST &AST, llvm::StringRef Code,
Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::None;
if (SuppressMissing && SuppressUnused)
return {};
- auto Findings = computeIncludeCleanerFindings(AST);
+ auto Findings = computeIncludeCleanerFindings(
+ AST, Cfg.Diagnostics.Includes.AnalyzeAngledIncludes);
if (SuppressMissing)
Findings.MissingIncludes.clear();
if (SuppressUnused)
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index f0ffc429c0ca9..4ecfdf0184ab4 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -277,6 +277,12 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) {
};
EXPECT_TRUE(HeaderFilter("foo.h"));
EXPECT_FALSE(HeaderFilter("bar.h"));
+
+ Frag = {};
+ EXPECT_FALSE(Conf.Diagnostics.Includes.AnalyzeAngledIncludes);
+ Frag.Diagnostics.Includes.AnalyzeAngledIncludes = true;
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_TRUE(Conf.Diagnostics.Includes.AnalyzeAngledIncludes);
}
TEST_F(ConfigCompileTests, DiagnosticSuppression) {
diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
index 44a6647d4c0a8..10d67dead342c 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -278,6 +278,21 @@ TEST(ParseYAML, IncludesIgnoreHeader) {
ElementsAre(val("foo"), val("bar")));
}
+TEST(ParseYAML, IncludesAnalyzeAngledIncludes) {
+ CapturedDiags Diags;
+ Annotations YAML(R"yaml(
+Diagnostics:
+ Includes:
+ AnalyzeAngledIncludes: true
+ )yaml");
+ auto Results =
+ Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ ASSERT_EQ(Results.size(), 1u);
+ EXPECT_THAT(Results[0].Diagnostics.Includes.AnalyzeAngledIncludes,
+ llvm::ValueIs(val(true)));
+}
+
TEST(ParseYAML, Style) {
CapturedDiags Diags;
Annotations YAML(R"yaml(
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 142310837bd9c..7027232460354 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -108,6 +108,7 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
#include "unguarded.h"
#include "unused.h"
#include <system_header.h>
+ #include <non_system_angled_header.h>
void foo() {
a();
b();
@@ -122,6 +123,7 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
TU.AdditionalFiles["dir/c.h"] = guard("void c();");
TU.AdditionalFiles["unused.h"] = guard("void unused();");
TU.AdditionalFiles["dir/unused.h"] = guard("void dirUnused();");
+ TU.AdditionalFiles["dir/non_system_angled_header.h"] = guard("");
TU.AdditionalFiles["system/system_header.h"] = guard("");
TU.AdditionalFiles["unguarded.h"] = "";
TU.ExtraArgs.push_back("-I" + testPath("dir"));
@@ -135,6 +137,48 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
Pointee(writtenInclusion("\"dir/unused.h\""))));
}
+TEST(IncludeCleaner, IgnoredAngledHeaders) {
+ // Currently the default behavior is to ignore unused angled includes
+ auto TU = TestTU::withCode(R"cpp(
+ #include <system_header.h>
+ #include <system_unused.h>
+ #include <non_system_angled_unused.h>
+ SystemClass x;
+ )cpp");
+ TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass {};");
+ TU.AdditionalFiles["system/system_unused.h"] = guard("");
+ TU.AdditionalFiles["dir/non_system_angled_unused.h"] = guard("");
+ TU.ExtraArgs = {
+ "-isystem" + testPath("system"),
+ "-I" + testPath("dir"),
+ };
+ auto AST = TU.build();
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+}
+
+TEST(IncludeCleaner, UnusedAngledHeaders) {
+ auto TU = TestTU::withCode(R"cpp(
+ #include <system_header.h>
+ #include <system_unused.h>
+ #include <non_system_angled_unused.h>
+ SystemClass x;
+ )cpp");
+ TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass {};");
+ TU.AdditionalFiles["system/system_unused.h"] = guard("");
+ TU.AdditionalFiles["dir/non_system_angled_unused.h"] = guard("");
+ TU.ExtraArgs = {
+ "-isystem" + testPath("system"),
+ "-I" + testPath("dir"),
+ };
+ auto AST = TU.build();
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST, true);
+ EXPECT_THAT(Findings.UnusedIncludes,
+ UnorderedElementsAre(
+ Pointee(writtenInclusion("<system_unused.h>")),
+ Pointee(writtenInclusion("<non_system_angled_unused.h>"))));
+}
+
TEST(IncludeCleaner, ComputeMissingHeaders) {
Annotations MainFile(R"cpp(
#include "a.h"
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3e3195f6f6813..a5e87d26d96c3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -84,6 +84,11 @@ Objective-C
Miscellaneous
^^^^^^^^^^^^^
+- Added a boolean option `AnalyzeAngledIncludes` to `Includes` config section,
+ which allows to enable unused includes detection for all angled ("system") headers.
+ At this moment umbrella headers are not supported, so enabling this option
+ may result in false-positives.
+
Improvements to clang-doc
-------------------------
More information about the cfe-commits
mailing list