kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This is useful for dogfooding the feature and spotting bugs.


Repository:
  rG LLVM Github Monorepo

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,20 @@
       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));
+  log("Config detected: {0}",
+      (Cfg.Diagnostics.IncludeCleaner ==
+               Config::IncludeCleanerPolicy::UnusedHeaders
+           ? "UnusedHeaders"
+           : "None"));
+  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;
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -63,6 +63,8 @@
 
 std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
 
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST);
+
 } // 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,30 @@
   return getUnused(AST.getIncludeStructure(), ReferencedFiles);
 }
 
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST) {
+  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;
+    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
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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to