kbobyrev updated this revision to Diff 380426.
kbobyrev added a comment.
Fix the warning range.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111870/new/
https://reviews.llvm.org/D111870
Files:
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
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -67,6 +67,7 @@
CheckOptions:
IgnoreMacros: true
example-check.ExampleOption: 0
+ IncludeCleaner: UnusedHeaders
)yaml";
auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
EXPECT_THAT(Diags.Diagnostics, IsEmpty());
@@ -83,6 +84,8 @@
EXPECT_THAT(Results[3].Diagnostics.ClangTidy.CheckOptions,
ElementsAre(PairVal("IgnoreMacros", "true"),
PairVal("example-check.ExampleOption", "0")));
+ EXPECT_TRUE(Results[3].Diagnostics.IncludeCleaner);
+ EXPECT_EQ("UnusedHeaders", *Results[3].Diagnostics.IncludeCleaner.getValue());
}
TEST(ParseYAML, Locations) {
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -244,6 +244,25 @@
}
}
+TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) {
+ // Defaults to None.
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Diagnostics.IncludeCleaner,
+ Config::IncludeCleanerPolicy::None);
+
+ Frag = {};
+ Frag.Diagnostics.IncludeCleaner.emplace("None");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Diagnostics.IncludeCleaner,
+ Config::IncludeCleanerPolicy::None);
+
+ Frag = {};
+ Frag.Diagnostics.IncludeCleaner.emplace("UnusedHeaders");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Diagnostics.IncludeCleaner,
+ Config::IncludeCleanerPolicy::UnusedHeaders);
+}
+
TEST_F(ConfigCompileTests, DiagnosticSuppression) {
Frag.Diagnostics.Suppress.emplace_back("bugprone-use-after-move");
Frag.Diagnostics.Suppress.emplace_back("unreachable-code");
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -18,6 +18,7 @@
#include "FeatureModule.h"
#include "Headers.h"
#include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
#include "IncludeFixer.h"
#include "Preamble.h"
#include "SourceCode.h"
@@ -342,6 +343,7 @@
llvm::Optional<IncludeFixer> FixIncludes;
// No need to run clang-tidy or IncludeFixerif we are not going to surface
// diagnostics.
+ const Config &Cfg = Config::current();
if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
tidy::ClangTidyOptions ClangTidyOpts =
@@ -366,7 +368,6 @@
Check->registerMatchers(&CTFinder);
}
- const Config &Cfg = Config::current();
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
if (Cfg.Diagnostics.SuppressAll ||
@@ -515,10 +516,16 @@
Diags->insert(Diags->end(), D.begin(), D.end());
}
}
- return ParsedAST(Inputs.Version, std::move(Preamble), std::move(Clang),
+ ParsedAST Result(Inputs.Version, std::move(Preamble), std::move(Clang),
std::move(Action), std::move(Tokens), std::move(Macros),
std::move(Marks), std::move(ParsedDecls), std::move(Diags),
std::move(Includes), std::move(CanonIncludes));
+ if (Result.Diags && Cfg.Diagnostics.IncludeCleaner ==
+ Config::IncludeCleanerPolicy::UnusedHeaders)
+ for (const auto &D :
+ issueUnusedIncludesDiagnostics(Result, Inputs.Contents))
+ Result.Diags->push_back(D);
+ return Result;
}
ParsedAST::ParsedAST(ParsedAST &&Other) = default;
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -63,6 +63,9 @@
std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
+ StringRef Code);
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -206,5 +206,36 @@
return getUnused(AST.getIncludeStructure(), ReferencedFiles);
}
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
+ StringRef Code) {
+ std::vector<Diag> Result;
+ for (const auto *Inc : computeUnusedIncludes(AST)) {
+ Diag D;
+ D.Message = "Included header is unused";
+ D.Name = "clangd-include-cleaner";
+ // FIXME: This range should be the whole line with target #include.
+ D.Range.start.line = Inc->HashLine;
+ D.Range.end.line = Inc->HashLine;
+ // Span the warning until the end of line or EOF.
+ D.Range.end.character =
+ Code.drop_front(Inc->HashOffset).find_first_of("\n\1A");
+ D.Fixes.emplace_back();
+ D.Fixes.back().Message = "Remove unused include";
+ D.Fixes.back().Edits.emplace_back();
+ D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
+ // LSP uses [start; end) ranges: this will span the deletion range to EOL.
+ D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
+ D.InsideMainFile = true;
+ AST.getSourceManager();
+ D.File = AST.getSourceManager()
+ .getFileEntryForID(AST.getSourceManager().getMainFileID())
+ ->getName()
+ .str();
+ D.Severity = DiagnosticsEngine::Warning;
+ Result.push_back(std::move(D));
+ }
+ return Result;
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -114,6 +114,9 @@
void parse(Fragment::DiagnosticsBlock &F, Node &N) {
DictParser Dict("Diagnostics", this);
+ Dict.handle("IncludeCleaner", [&](Node &N) {
+ F.IncludeCleaner = scalarValue(N, "IncludeCleaner");
+ });
Dict.handle("Suppress", [&](Node &N) {
if (auto Values = scalarValues(N))
F.Suppress = std::move(*Values);
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -210,6 +210,11 @@
/// This often has other advantages, such as skipping some analysis.
std::vector<Located<std::string>> Suppress;
+ /// Valid values are:
+ /// - UnusedHeaders
+ /// - None
+ llvm::Optional<Located<std::string>> IncludeCleaner;
+
/// Controls how clang-tidy will run over the code base.
///
/// The settings are merged with any settings found in .clang-tidy
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -414,6 +414,18 @@
C.Diagnostics.Suppress.insert(N);
});
+ if (F.IncludeCleaner) {
+ if (auto Val = compileEnum<Config::IncludeCleanerPolicy>(
+ "IncludeCleaner", **F.IncludeCleaner)
+ .map("UnusedHeaders",
+ Config::IncludeCleanerPolicy::UnusedHeaders)
+ .map("None", Config::IncludeCleanerPolicy::None)
+ .value())
+ Out.Apply.push_back([Val](const Params &, Config &C) {
+ C.Diagnostics.IncludeCleaner = *Val;
+ });
+ }
+
compile(std::move(F.ClangTidy));
}
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -87,9 +87,11 @@
} Index;
/// Controls warnings and errors when parsing code.
+ enum IncludeCleanerPolicy { UnusedHeaders, None };
struct {
bool SuppressAll = false;
llvm::StringSet<> Suppress;
+ IncludeCleanerPolicy IncludeCleaner = None;
/// Configures what clang-tidy checks to run and options to use with them.
struct {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
