diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/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 { diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -414,6 +414,18 @@ C.Diagnostics.Suppress.insert(N); }); + if (F.IncludeCleaner) { + if (auto Val = compileEnum( + "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)); } diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -210,6 +210,11 @@ /// This often has other advantages, such as skipping some analysis. std::vector> Suppress; + /// Valid values are: + /// - UnusedHeaders + /// - None + llvm::Optional> IncludeCleaner; + /// Controls how clang-tidy will run over the code base. /// /// The settings are merged with any settings found in .clang-tidy diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/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); diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -63,6 +63,8 @@ std::vector computeUnusedIncludes(ParsedAST &AST); +std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -206,5 +206,30 @@ return getUnused(AST.getIncludeStructure(), ReferencedFiles); } +std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST) { + std::vector 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; + D.Fixes.emplace_back(); + D.Fixes.back().Message = "Remove unused include"; + D.Fixes.back().Edits.emplace_back(); + D.Fixes.back().Edits.back().range = D.Range; + 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 diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/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 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,15 @@ 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)) + Result.Diags->push_back(D); + return Result; } ParsedAST::ParsedAST(ParsedAST &&Other) = default; diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/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"); diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/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) {