[clangd] Add a way to enable IncludeCleaner through config

[kbobyrev]

Summary

This is useful for dogfooding the feature and spotting bugs.

[kbobyrev]

Delete debugging leftovers.

[kbobyrev]

Fix the warning range.

[sammccall]

clang-tools-extra/clangd/Config.h

90

you've accidentally split this comment from its decl

91

structure, as discussed offline...

"include cleaner" refers to a set of features to do with header hygiene, related but I think we still want to configure them individually.

"Unused headers" is the feature we're trying to configure, so I think it's a key rather than a value. Its value could be a boolean, (and configure other aspects separately), or we could make it an enum so we can choose a policy at the same time.
My suspicion is that the major knob here is strictness: do we warn when a header doesn't *directly* provide any used symbols, or also indirectly?

So I'd probably call this UnusedInclude: Strict

94

don't group with the generic suppression bits

clang-tools-extra/clangd/ConfigFragment.h

213

This is where the documentation for the feature goes :-)

clang-tools-extra/clangd/IncludeCleaner.cpp

250

Source should be Clangd I think (our first?).

Name should be something like "unused-include": this doesn't need to echo the tool name.

251

remove FIXEDME

256

This doesn't handle hitting EOF correctly, and also gets encodings wrong (should be UTF-16 code units, unless we're in another mode).

I think you want:

D.range.end.character = lspLength(Code.drop_front(Inc->HashOffset).take_until([](char C) { return c == '\n' || c == '\r'; }));

You could make this a function in SourceCode.h if you like.
(I'm fairly sure we're not clean when it comes to using \r being a line separator in the rest of the code BTW)

261

No need to explain the ranges thing, this is ubiquitous and documented on the structs.

264

no-op

265

don't do this in the loop

270

D.Tags.push_back(Unneccesary)

This produces nice rendering in vscode

clang-tools-extra/clangd/IncludeCleaner.h

67

nit: llvm::StringRef

clang-tools-extra/clangd/ParsedAST.cpp

380

You're bypassing this logic, which is OK, but you'll need to duplicate it somewhere. (Probably wherever is looking at cfg anyway)

519

You're missing tests for this functionality

524

If/when we support other policies, the issueUnusedIncludesDiagnostics function will need to look at the current policy, so you might want to move this check there already. (We also need to check for suppression, as mentioned above)

525

you're copying all the diags here

I don't mind the temp vector to keep the signature nice, but Result.Diags->insert(Result.Diags->end(), make_move_iterator(NewDiags.begin()), etc)?

[kbobyrev]

Address review comments.

[kbobyrev]

Rebase on top of main, revert unwanted formatting change.

[kbobyrev]

Elaborate a bit more on Strict mode for UnusedHeaders.

[kbobyrev]

[clangd] IncludeCleaner: Implement more complicated rules for considering enums forward declarations "used"

[kbobyrev]

Prevent accidental changes from getting into this patch.

[sammccall]

Mostly dumb nitpicks because this is user-facing magic strings... let's chat before doing another round so I'm not wasting your time.

---

You've done some renaming from UnusedInclude -> UnusedHeaders.

I feel bad because I did say in the last round

"Unused headers" is the feature we're trying to configure

but I wasn't intending to be precise and actually think the old name is better (some discussion below)

clang-tools-extra/clangd/ConfigFragment.h

215

I'm not sure we need a separate block for this. The main cost is an extra line in the config file, and it makes the "include cleaner" name user-facing, so it's an extra concept people have to learn.

Do you think we'll need a lot more than UnusedHeaders/MissingHeaders?

216

s/IncludeCleaner/clangd

225

I wonder whether "UnusedIncludes" would be clearer, especially given the pairing with "MissingHeaders" (consider confusion with pp_file_not_found) vs "MissingIncludes".

Conceptually it's an unused edge rather than node, but mostly i'm just thinking "MissingIncludes" more clearly describes the nature of the fix.

clang-tools-extra/clangd/IncludeCleaner.cpp

157

maybe worth a comment that we don't bother to handle escaped newlines etc

164

This code is making me a bit dizzy. I get the idea that we're using the precise HashOffset while exploiting the fact that we know the line number to avoid counting lines. But I'm not sure it's worthwhile, we're near the top of the file after all...

What about just

Result.end = result.start = offsetToPosition(code, HashOffset)
Result.end.character += lspLength(Code.drop_front(HashOffset).take_until(...));

245

config is usually passed by TLS rather than explicitly

250

clangd-unused-header -> unused-header

(there's a test, but it's stale)

260

include at least the basename in the message - this also gets listed in e.g. diagnostics view

268

again, we're not removing the header, we're removing the inclusion
"remove #include directive"?

[kbobyrev]

clang-tools-extra/clangd/IncludeCleaner.cpp

250

what do you mean by "stale"?

[kbobyrev]

Resolve review comments.

[kbobyrev]

Use IncludeCleaner to get rid of unused include in IncludeCleaner.cpp 🆒

[sammccall]

Awesome, let's ship it!
Only nits on the text, feel free to ignore any you don't like

clang-tools-extra/clangd/ConfigFragment.h

213

brevity: consider "correct" or "diagnose" instead of "treat and warn users on".
specificity: consider "unnecessary #include directives" instead of "suboptimal include usage".

217

We're glossing over the key idea ("come from") here, and mixing what the Strict policy is about with what the feature is about. I think we can be a bit more precise.

I'd add a second sentence to the first paragraph, elaborating a little:

clangd can warn if a header is `#included` but not used, and suggest removing it.

And then we can define the Strict policy, and mention its limitations.

Strict means a header is unused if it does not *directly* provide any symbol used in the file.
Removing it may still break compilation if it transitively includes headers that are used.
This should be fixed by including those headers directly.

220

I think at least the "fixme" part isn't suitable for public docs.
We also don't mention what we want to do instead.

I'd suggest:

• move this to the part of the code that generates fixes
• mention that we if it's transitively used, we could suggest replacing it with the needed #includes instead (I think this is actually pretty feasible...)
• while there, maybe also add a FIXME that we should suggest adding IWYU pragma export/keep as an alternative fixes (once we honor that)

clang-tools-extra/clangd/IncludeCleaner.cpp

250

You'd updated the diagnostic name, but not the suppression check. I was going to suggest a test for suppression, and realized you had one, it was also testing the old name though.

LG now!

259

This is totally up to you, but wanted to be explicit with what I meant about basename...

If the path is long, this can be: included header this/is/a/long/path/to/some/header.h is not used
Depending how you feel about having to visually scan past the path to get to the end of the sentence, you might choose to trade precision for brevity: included header header.h is not used.

This is llvm::sys::path::filename(stripHeaderQuotes(Inc->Written), posix), with stripHeaderQuotes to be written.

Again, I don't feel strongly, your call

[kbobyrev]

Resolve the review comments.

clang-tools-extra/clangd/Config.h

struct Config {
   };
   /// Controls background-index behavior.
   struct {
     /// Whether this TU should be indexed.
     BackgroundPolicy Background = BackgroundPolicy::Build;
     ExternalIndexSpec External;
   } Index;
 
+  enum UnusedIncludesPolicy { Strict, None };
   /// Controls warnings and errors when parsing code.
   struct {
     bool SuppressAll = false;
     llvm::StringSet<> Suppress;
 
     /// Configures what clang-tidy checks to run and options to use with them.
     struct {
       // A comma-seperated list of globs specify which clang-tidy checks to run.
       std::string Checks;
       llvm::StringMap<std::string> CheckOptions;
     } ClangTidy;
+
+    UnusedIncludesPolicy UnusedIncludes = None;
   } Diagnostics;
 
   /// Style of the codebase.
   struct {
     // Namespaces that should always be fully qualified, meaning no "using"
     // declarations, always spell out the whole name (with or without leading
     // ::). All nested namespaces are affected as well.
     std::vector<std::string> FullyQualifiedNamespaces;

clang-tools-extra/clangd/ConfigCompile.cpp

if (!Normalized.empty())
       Out.Apply.push_back(
           [Normalized(std::move(Normalized))](const Params &, Config &C) {
             if (C.Diagnostics.SuppressAll)
               return;
             for (llvm::StringRef N : Normalized)
               C.Diagnostics.Suppress.insert(N);
           });
 
+    if (F.UnusedIncludes)
+      if (auto Val = compileEnum<Config::UnusedIncludesPolicy>(
+                         "UnusedIncludes", **F.UnusedIncludes)
+                         .map("Strict", Config::UnusedIncludesPolicy::Strict)
+                         .map("None", Config::UnusedIncludesPolicy::None)
+                         .value())
+        Out.Apply.push_back([Val](const Params &, Config &C) {
+          C.Diagnostics.UnusedIncludes = *Val;
+        });
+
     compile(std::move(F.ClangTidy));
   }
 
   void compile(Fragment::StyleBlock &&F) {
     if (!F.FullyQualifiedNamespaces.empty()) {
       std::vector<std::string> FullyQualifiedNamespaces;
       for (auto &N : F.FullyQualifiedNamespaces) {
         // Normalize the data by dropping both leading and trailing ::

clang-tools-extra/clangd/ConfigFragment.h

struct DiagnosticsBlock {
     /// - warning categories (e.g. unused-result)
     /// - clang-tidy check names (e.g. bugprone-narrowing-conversions)
     ///
     /// This is a simple filter. Diagnostics can be controlled in other ways
     /// (e.g. by disabling a clang-tidy check, or the -Wunused compile flag).
     /// This often has other advantages, such as skipping some analysis.
     std::vector<Located<std::string>> Suppress;
 
+    /// Controls how clangd will correct "unnecessary #include directives.
+    /// clangd can warn if a header is `#include`d but not used, and suggest
+    /// removing it.
+    //
+    /// Strict means a header is unused if it does not *directly* provide any
+    /// symbol used in the file. Removing it may still break compilation if it
+    /// transitively includes headers that are used. This should be fixed by
+    /// including those headers directly.
+    ///
+    /// Valid values are:
+    /// - Strict
+    /// - None
+    llvm::Optional<Located<std::string>> UnusedIncludes;
+
     /// Controls how clang-tidy will run over the code base.
     ///
     /// The settings are merged with any settings found in .clang-tidy
     /// configiration files with these ones taking precedence.
     struct ClangTidyBlock {
       std::vector<Located<std::string>> Add;
       /// List of checks to disable.
       /// Takes precedence over Add. To enable all llvm checks except include

clang-tools-extra/clangd/ConfigYAML.cpp

private:
   }
 
   void parse(Fragment::DiagnosticsBlock &F, Node &N) {
     DictParser Dict("Diagnostics", this);
     Dict.handle("Suppress", [&](Node &N) {
       if (auto Values = scalarValues(N))
         F.Suppress = std::move(*Values);
     });
+    Dict.handle("UnusedIncludes", [&](Node &N) {
+      F.UnusedIncludes = scalarValue(N, "UnusedIncludes");
+    });
     Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); });
     Dict.parse(N);
   }
 
   void parse(Fragment::DiagnosticsBlock::ClangTidyBlock &F, Node &N) {
     DictParser Dict("ClangTidy", this);
     Dict.handle("Add", [&](Node &N) {
       if (auto Values = scalarValues(N))

clang-tools-extra/clangd/Diagnostics.h

 /// A top-level diagnostic that may have Notes and Fixes.
 struct Diag : DiagBase {
   std::string Name; // if ID was recognized.
   // The source of this diagnostic.
   enum DiagSource {
     Unknown,
     Clang,
     ClangTidy,
+    Clangd,
     ClangdConfig,
   } Source = Unknown;
   /// Elaborate on the problem, usually pointing to a related piece of code.
   std::vector<Note> Notes;
   /// *Alternative* fixes for this diagnostic, one should be chosen.
   std::vector<Fix> Fixes;
   llvm::SmallVector<DiagnosticTag, 1> Tags;
 };

clang-tools-extra/clangd/Diagnostics.cpp

void toLSPDiags(
   Main.code = D.Name;
   switch (D.Source) {
   case Diag::Clang:
     Main.source = "clang";
     break;
   case Diag::ClangTidy:
     Main.source = "clang-tidy";
     break;
+  case Diag::Clangd:
+    Main.source = "clangd";
+    break;
   case Diag::ClangdConfig:
     Main.source = "clangd-config";
     break;
   case Diag::Unknown:
     break;
   }
   if (Opts.EmbedFixesInDiagnostics) {
     Main.codeActions.emplace();

clang-tools-extra/clangd/IncludeCleaner.h

 
 /// Retrieves headers that are referenced from the main file but not used.
 std::vector<const Inclusion *>
 getUnused(const IncludeStructure &Includes,
           const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles);
 
 std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
 
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
+                                                 llvm::StringRef Code);
+
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDE_CLEANER_H

clang-tools-extra/clangd/IncludeCleaner.cpp

 //===--- IncludeCleaner.cpp - Unused/Missing Headers Analysis ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "IncludeCleaner.h"
+#include "Config.h"
+#include "Protocol.h"
+#include "SourceCode.h"
 #include "support/Logger.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 /// Crawler traverses the AST and feeds in the locations of (sometimes
 /// implicitly) used symbols into \p Result.
 class ReferencedLocationCrawler
if (!SM.isWrittenInScratchSpace(Exp.getSpellingLoc()))
       add(Exp.getSpellingLoc());
     if (!SM.isWrittenInScratchSpace(Exp.getExpansionLocStart()))
       add(Exp.getExpansionLocStart());
     if (!SM.isWrittenInScratchSpace(Exp.getExpansionLocEnd()))
       add(Exp.getExpansionLocEnd());
   }
 };
 
+// Returns the range starting at '#' and ending at EOL. Escaped newlines are not
+// handled.
+clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) {
+  clangd::Range Result;
+  Result.end = Result.start = offsetToPosition(Code, HashOffset);
+
+  // Span the warning until the EOL or EOF.
+  Result.end.character +=
+      lspLength(Code.drop_front(HashOffset).take_until([](char C) {
+        return C == '\n' || C == '\r';
+      }));
+  return Result;
+}
+
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
   // FIXME(kirillbobyrev): Handle macros.
   return Result;
std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) {
   const auto &SM = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
   auto ReferencedFiles = translateToHeaderIDs(findReferencedFiles(Refs, SM),
                                               AST.getIncludeStructure(), SM);
   return getUnused(AST.getIncludeStructure(), ReferencedFiles);
 }
 
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
+                                                 llvm::StringRef Code) {
+  const Config &Cfg = Config::current();
+  if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
+      Cfg.Diagnostics.SuppressAll ||
+      Cfg.Diagnostics.Suppress.contains("unused-includes"))
+    return {};
+  std::vector<Diag> Result;
+  std::string FileName =
+      AST.getSourceManager()
+          .getFileEntryForID(AST.getSourceManager().getMainFileID())
+          ->getName()
+          .str();
+  for (const auto *Inc : computeUnusedIncludes(AST)) {
+    Diag D;
+    D.Message =
+        llvm::formatv("included header {0} is not used",
+                      llvm::sys::path::filename(
+                          Inc->Written.substr(1, Inc->Written.size() - 2),
+                          llvm::sys::path::Style::posix));
+    D.Name = "unused-includes";
+    D.Source = Diag::DiagSource::Clangd;
+    D.File = FileName;
+    D.Severity = DiagnosticsEngine::Warning;
+    D.Tags.push_back(Unnecessary);
+    D.Range = getDiagnosticRange(Code, Inc->HashOffset);
+    // FIXME(kirillbobyrev): Removing inclusion might break the code if the
+    // used headers are only reachable transitively through this one. Suggest
+    // including them directly instead.
+    // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
+    // (keep/export) remove the warning once we support IWYU pragmas.
+    D.Fixes.emplace_back();
+    D.Fixes.back().Message = "remove #include directive";
+    D.Fixes.back().Edits.emplace_back();
+    D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
+    D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
+    D.InsideMainFile = true;
+    Result.push_back(std::move(D));
+  }
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/ParsedAST.cpp

 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
 #include "Diagnostics.h"
 #include "Feature.h"
 #include "FeatureModule.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
 #include "TidyProvider.h"
 #include "index/CanonicalIncludes.h"
 #include "index/Index.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
                                   const clang::Diagnostic &Info) {
       if (Cfg.Diagnostics.SuppressAll ||
           isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress))
         return DiagnosticsEngine::Ignored;
       if (!CTChecks.empty()) {
         std::string CheckName = CTContext->getCheckName(Info.getID());
         bool IsClangTidyDiag = !CheckName.empty();
         if (IsClangTidyDiag) {
           if (Cfg.Diagnostics.Suppress.contains(CheckName))
             return DiagnosticsEngine::Ignored;
           // Check for suppression comment. Skip the check for diagnostics not
           // in the main file, because we don't want that function to query the
           // source buffer for preamble files. For the same reason, we ask
           // shouldSuppressDiagnostic to avoid I/O.
           // We let suppression comments take precedence over warning-as-error
           // to match clang-tidy's behaviour.
           bool IsInsideMainFile =
if (Preamble)
       Diags->insert(Diags->end(), Preamble->Diags.begin(),
                     Preamble->Diags.end());
     // Finally, add diagnostics coming from the AST.
     {
       std::vector<Diag> D = ASTDiags.take(CTContext.getPointer());
       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) {
+    auto UnusedHeadersDiags =
+        issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
+    Result.Diags->insert(Result.Diags->end(),
+                         make_move_iterator(UnusedHeadersDiags.begin()),
+                         make_move_iterator(UnusedHeadersDiags.end()));
+  }
+  return Result;
 }
 
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;
 
 ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
 
 ParsedAST::~ParsedAST() {
   if (Action) {

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

for (const auto &Case : Cases) {
     Frag = {};
     Frag.If.PathExclude.emplace_back(Case.PathSpec);
     Frag.Source.Directory = Case.Directory;
     EXPECT_NE(compileAndApply(), Case.ShouldMatch);
     ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   }
 }
 
+TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) {
+  // Defaults to None.
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
+            Config::UnusedIncludesPolicy::None);
+
+  Frag = {};
+  Frag.Diagnostics.UnusedIncludes.emplace("None");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
+            Config::UnusedIncludesPolicy::None);
+
+  Frag = {};
+  Frag.Diagnostics.UnusedIncludes.emplace("Strict");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
+            Config::UnusedIncludesPolicy::Strict);
+}
+
 TEST_F(ConfigCompileTests, DiagnosticSuppression) {
   Frag.Diagnostics.Suppress.emplace_back("bugprone-use-after-move");
   Frag.Diagnostics.Suppress.emplace_back("unreachable-code");
   Frag.Diagnostics.Suppress.emplace_back("-Wunused-variable");
   Frag.Diagnostics.Suppress.emplace_back("typecheck_bool_condition");
   Frag.Diagnostics.Suppress.emplace_back("err_unexpected_friend");
   Frag.Diagnostics.Suppress.emplace_back("warn_alloca");
   EXPECT_TRUE(compileAndApply());

clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

 Index:
   Background: Skip
 ---
 Diagnostics:
   ClangTidy:
     CheckOptions:
       IgnoreMacros: true
       example-check.ExampleOption: 0
+  UnusedIncludes: Strict
   )yaml";
   auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
   EXPECT_THAT(Diags.Files, ElementsAre("config.yaml"));
   ASSERT_EQ(Results.size(), 4u);
   EXPECT_FALSE(Results[0].If.HasUnrecognizedCondition);
   EXPECT_THAT(Results[0].If.PathMatch, ElementsAre(Val("abc")));
   EXPECT_THAT(Results[0].CompileFlags.Add, ElementsAre(Val("foo"), Val("bar")));
 
   EXPECT_THAT(Results[1].CompileFlags.Add, ElementsAre(Val("b\naz\n")));
 
   ASSERT_TRUE(Results[2].Index.Background);
   EXPECT_EQ("Skip", *Results[2].Index.Background.getValue());
   EXPECT_THAT(Results[3].Diagnostics.ClangTidy.CheckOptions,
               ElementsAre(PairVal("IgnoreMacros", "true"),
                           PairVal("example-check.ExampleOption", "0")));
+  EXPECT_TRUE(Results[3].Diagnostics.UnusedIncludes);
+  EXPECT_EQ("Strict", *Results[3].Diagnostics.UnusedIncludes.getValue());
 }
 
 TEST(ParseYAML, Locations) {
   CapturedDiags Diags;
   Annotations YAML(R"yaml(
 If:
   PathMatch: [['???bad***regex(((']]
   )yaml");

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

TEST(Diagnostics, Tags) {
   TU.Code = Test.code().str();
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   AllOf(Diag(Test.range("unused"), "unused variable 'x'"),
                         WithTag(DiagnosticTag::Unnecessary)),
                   AllOf(Diag(Test.range("deprecated"), "'bar' is deprecated"),
                         WithTag(DiagnosticTag::Deprecated))));
 }
+
+TEST(DiagnosticsTest, IncludeCleaner) {
+  Annotations Test(R"cpp(
+$fix[[  $diag[[#include "unused.h"]]
+]]  #include "used.h"
+
+  void foo() {
+    used();
+  }
+  )cpp");
+  TestTU TU;
+  TU.Code = Test.code().str();
+  TU.AdditionalFiles["unused.h"] = R"cpp(
+    void unused() {}
+  )cpp";
+  TU.AdditionalFiles["used.h"] = R"cpp(
+    void used() {}
+  )cpp";
+  // Off by default.
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+  Config Cfg;
+  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  WithContextValue WithCfg(Config::Key, std::move(Cfg));
+  EXPECT_THAT(
+      *TU.build().getDiagnostics(),
+      UnorderedElementsAre(AllOf(
+          Diag(Test.range("diag"), "included header unused.h is not used"),
+          WithTag(DiagnosticTag::Unnecessary), DiagSource(Diag::Clangd),
+          WithFix(Fix(Test.range("fix"), "", "remove #include directive")))));
+  Cfg.Diagnostics.SuppressAll = true;
+  WithContextValue SuppressAllWithCfg(Config::Key, std::move(Cfg));
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+  Cfg.Diagnostics.SuppressAll = false;
+  Cfg.Diagnostics.Suppress = {"unused-includes"};
+  WithContextValue SuppressFilterWithCfg(Config::Key, std::move(Cfg));
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang