[clang-tools-extra] c3df9d5 - [clangd] Parse Diagnostics block, and nest ClangTidy block under it.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 27 16:38:46 PST 2021


Author: Sam McCall
Date: 2021-01-28T01:36:23+01:00
New Revision: c3df9d58c75e0f89ca95e947804d65e79a491adc

URL: https://github.com/llvm/llvm-project/commit/c3df9d58c75e0f89ca95e947804d65e79a491adc
DIFF: https://github.com/llvm/llvm-project/commit/c3df9d58c75e0f89ca95e947804d65e79a491adc.diff

LOG: [clangd] Parse Diagnostics block, and nest ClangTidy block under it.

(ClangTidy configuration block hasn't been in any release, so we should be OK
to move it around like this)

Differential Revision: https://reviews.llvm.org/D95362

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/TidyProvider.cpp
    clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
    clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 44ca283b6a0e..391632cb086a 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -90,6 +90,13 @@ struct Config {
   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;
   } Diagnostics;
 
   /// Style of the codebase.
@@ -99,14 +106,6 @@ struct Config {
     // ::). All nested namespaces are affected as well.
     std::vector<std::string> FullyQualifiedNamespaces;
   } Style;
-
-  /// Configures what clang-tidy checks to run and options to use with them.
-  struct {
-    // A comma-seperated list of globs to specify which clang-tidy checks to
-    // run.
-    std::string Checks;
-    llvm::StringMap<std::string> CheckOptions;
-  } ClangTidy;
 };
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index e82c6e159421..8682cae36f26 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -189,7 +189,6 @@ struct FragmentCompiler {
     compile(std::move(F.CompileFlags));
     compile(std::move(F.Index));
     compile(std::move(F.Diagnostics));
-    compile(std::move(F.ClangTidy));
   }
 
   void compile(Fragment::IfBlock &&F) {
@@ -379,6 +378,8 @@ struct FragmentCompiler {
         for (llvm::StringRef N : Normalized)
           C.Diagnostics.Suppress.insert(N);
       });
+
+    compile(std::move(F.ClangTidy));
   }
 
   void compile(Fragment::StyleBlock &&F) {
@@ -422,7 +423,7 @@ struct FragmentCompiler {
     CurSpec += Str;
   }
 
-  void compile(Fragment::ClangTidyBlock &&F) {
+  void compile(Fragment::DiagnosticsBlock::ClangTidyBlock &&F) {
     std::string Checks;
     for (auto &CheckGlob : F.Add)
       appendTidyCheckSpec(Checks, CheckGlob, true);
@@ -433,8 +434,9 @@ struct FragmentCompiler {
     if (!Checks.empty())
       Out.Apply.push_back(
           [Checks = std::move(Checks)](const Params &, Config &C) {
-            C.ClangTidy.Checks.append(
-                Checks, C.ClangTidy.Checks.empty() ? /*skip comma*/ 1 : 0,
+            C.Diagnostics.ClangTidy.Checks.append(
+                Checks,
+                C.Diagnostics.ClangTidy.Checks.empty() ? /*skip comma*/ 1 : 0,
                 std::string::npos);
           });
     if (!F.CheckOptions.empty()) {
@@ -445,8 +447,8 @@ struct FragmentCompiler {
       Out.Apply.push_back(
           [CheckOptions = std::move(CheckOptions)](const Params &, Config &C) {
             for (auto &StringPair : CheckOptions)
-              C.ClangTidy.CheckOptions.insert_or_assign(StringPair.first,
-                                                        StringPair.second);
+              C.Diagnostics.ClangTidy.CheckOptions.insert_or_assign(
+                  StringPair.first, StringPair.second);
           });
     }
   }

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 5b67c49fe154..c36b07f5e8e2 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -203,6 +203,29 @@ struct Fragment {
     /// (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 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
+      /// order:
+      ///   Add: llvm-*
+      ///   Remove: llvm-include-onder
+      std::vector<Located<std::string>> Remove;
+
+      /// A Key-Value pair list of options to pass to clang-tidy checks
+      /// These take precedence over options specified in clang-tidy
+      /// configuration files. Example:
+      ///   CheckOptions:
+      ///     readability-braces-around-statements.ShortStatementLines: 2
+      std::vector<std::pair<Located<std::string>, Located<std::string>>>
+          CheckOptions;
+    };
+    ClangTidyBlock ClangTidy;
   };
   DiagnosticsBlock Diagnostics;
 
@@ -215,30 +238,6 @@ struct Fragment {
     std::vector<Located<std::string>> FullyQualifiedNamespaces;
   };
   StyleBlock Style;
-
-  /// 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.
-  // FIXME: move this to Diagnostics.Tidy.
-  struct ClangTidyBlock {
-    std::vector<Located<std::string>> Add;
-    /// List of checks to disable.
-    /// Takes precedence over Add. To enable all llvm checks except include
-    /// order:
-    ///   Add: llvm-*
-    ///   Remove: llvm-include-onder
-    std::vector<Located<std::string>> Remove;
-
-    /// A Key-Value pair list of options to pass to clang-tidy checks
-    /// These take precedence over options specified in clang-tidy configuration
-    /// files. Example:
-    ///   CheckOptions:
-    ///     readability-braces-around-statements.ShortStatementLines: 2
-    std::vector<std::pair<Located<std::string>, Located<std::string>>>
-        CheckOptions;
-  };
-  ClangTidyBlock ClangTidy;
 };
 
 } // namespace config

diff  --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 7aaff5565497..348ee9dd1f75 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -62,7 +62,7 @@ class Parser {
     Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); });
     Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
     Dict.handle("Style", [&](Node &N) { parse(F.Style, N); });
-    Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); });
+    Dict.handle("Diagnostics", [&](Node &N) { parse(F.Diagnostics, N); });
     Dict.parse(N);
     return !(N.failed() || HadError);
   }
@@ -110,7 +110,17 @@ class Parser {
     Dict.parse(N);
   }
 
-  void parse(Fragment::ClangTidyBlock &F, Node &N) {
+  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("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))

diff  --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp
index 0a9f12221287..c26c59fd347d 100644
--- a/clang-tools-extra/clangd/TidyProvider.cpp
+++ b/clang-tools-extra/clangd/TidyProvider.cpp
@@ -255,7 +255,7 @@ TidyProvider disableUnusableChecks(llvm::ArrayRef<std::string> ExtraBadChecks) {
 
 TidyProviderRef provideClangdConfig() {
   return [](tidy::ClangTidyOptions &Opts, llvm::StringRef) {
-    const auto &CurTidyConfig = Config::current().ClangTidy;
+    const auto &CurTidyConfig = Config::current().Diagnostics.ClangTidy;
     if (!CurTidyConfig.Checks.empty())
       mergeCheckList(Opts.Checks, CurTidyConfig.Checks);
 

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index ef24b5d8417f..4b1da2035727 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -259,32 +259,36 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) {
 }
 
 TEST_F(ConfigCompileTests, Tidy) {
-  Frag.ClangTidy.Add.emplace_back("bugprone-use-after-move");
-  Frag.ClangTidy.Add.emplace_back("llvm-*");
-  Frag.ClangTidy.Remove.emplace_back("llvm-include-order");
-  Frag.ClangTidy.Remove.emplace_back("readability-*");
-  Frag.ClangTidy.CheckOptions.emplace_back(
+  auto &Tidy = Frag.Diagnostics.ClangTidy;
+  Tidy.Add.emplace_back("bugprone-use-after-move");
+  Tidy.Add.emplace_back("llvm-*");
+  Tidy.Remove.emplace_back("llvm-include-order");
+  Tidy.Remove.emplace_back("readability-*");
+  Tidy.CheckOptions.emplace_back(
       std::make_pair(std::string("StrictMode"), std::string("true")));
-  Frag.ClangTidy.CheckOptions.emplace_back(std::make_pair(
+  Tidy.CheckOptions.emplace_back(std::make_pair(
       std::string("example-check.ExampleOption"), std::string("0")));
   EXPECT_TRUE(compileAndApply());
   EXPECT_EQ(
-      Conf.ClangTidy.Checks,
+      Conf.Diagnostics.ClangTidy.Checks,
       "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*");
-  EXPECT_EQ(Conf.ClangTidy.CheckOptions.size(), 2U);
-  EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("StrictMode"), "true");
-  EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("example-check.ExampleOption"),
+  EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.size(), 2U);
+  EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.lookup("StrictMode"),
+            "true");
+  EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.lookup(
+                "example-check.ExampleOption"),
             "0");
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
 }
 
 TEST_F(ConfigCompileTests, TidyBadChecks) {
-  Frag.ClangTidy.Add.emplace_back("unknown-check");
-  Frag.ClangTidy.Remove.emplace_back("*");
-  Frag.ClangTidy.Remove.emplace_back("llvm-includeorder");
+  auto &Tidy = Frag.Diagnostics.ClangTidy;
+  Tidy.Add.emplace_back("unknown-check");
+  Tidy.Remove.emplace_back("*");
+  Tidy.Remove.emplace_back("llvm-includeorder");
   EXPECT_TRUE(compileAndApply());
   // Ensure bad checks are stripped from the glob.
-  EXPECT_EQ(Conf.ClangTidy.Checks, "-*");
+  EXPECT_EQ(Conf.Diagnostics.ClangTidy.Checks, "-*");
   EXPECT_THAT(
       Diags.Diagnostics,
       ElementsAre(

diff  --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
index 25d468ba604a..e1c81344de20 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -60,10 +60,11 @@ CompileFlags: { Add: [foo, bar] }
 Index:
   Background: Skip
 ---
-ClangTidy: 
-  CheckOptions: 
-    IgnoreMacros: true
-    example-check.ExampleOption: 0
+Diagnostics:
+  ClangTidy:
+    CheckOptions:
+      IgnoreMacros: true
+      example-check.ExampleOption: 0
   )yaml";
   auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
@@ -77,7 +78,7 @@ CompileFlags: { Add: [foo, bar] }
 
   ASSERT_TRUE(Results[2].Index.Background);
   EXPECT_EQ("Skip", *Results[2].Index.Background.getValue());
-  EXPECT_THAT(Results[3].ClangTidy.CheckOptions,
+  EXPECT_THAT(Results[3].Diagnostics.ClangTidy.CheckOptions,
               ElementsAre(PairVal("IgnoreMacros", "true"),
                           PairVal("example-check.ExampleOption", "0")));
 }


        


More information about the cfe-commits mailing list