[clangd] Add clang-tidy options to config

First step of implementing clang-tidy configuration into clangd config.
This is just adding support for reading and verifying the clang tidy options from the config fragments.
No support is added for actually using the options within clang-tidy yet.

That will be added in a follow up as its a little more involved.

Reviewed By: sammccall

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

clang-tools-extra/clangd/Config.h

@@ -26,6 +26,7 @@
 
 #include "support/Context.h"
 #include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include <string>
 #include <vector>
 
@@ -70,6 +71,14 @@ 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

clang-tools-extra/clangd/ConfigCompile.cpp

@@ -157,6 +157,7 @@ struct FragmentCompiler {
     compile(std::move(F.If));
     compile(std::move(F.CompileFlags));
     compile(std::move(F.Index));
+    compile(std::move(F.ClangTidy));
   }
 
   void compile(Fragment::IfBlock &&F) {
@@ -264,6 +265,49 @@ struct FragmentCompiler {
     }
   }
 
+  void appendTidyCheckSpec(std::string &CurSpec,
+                           const Located<std::string> &Arg, bool IsPositive) {
+    StringRef Str = *Arg;
+    // Don't support negating here, its handled if the item is in the Add or
+    // Remove list.
+    if (Str.startswith("-") || Str.contains(',')) {
+      diag(Error, "Invalid clang-tidy check name", Arg.Range);
+      return;
+    }
+    CurSpec += ',';
+    if (!IsPositive)
+      CurSpec += '-';
+    CurSpec += Str;
+  }
+
+  void compile(Fragment::ClangTidyBlock &&F) {
+    std::string Checks;
+    for (auto &CheckGlob : F.Add)
+      appendTidyCheckSpec(Checks, CheckGlob, true);
+
+    for (auto &CheckGlob : F.Remove)
+      appendTidyCheckSpec(Checks, CheckGlob, false);
+
+    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);
+          });
+    if (!F.CheckOptions.empty()) {
+      std::vector<std::pair<std::string, std::string>> CheckOptions;
+      for (auto &Opt : F.CheckOptions)
+        CheckOptions.emplace_back(std::move(*Opt.first),
+                                  std::move(*Opt.second));
+      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);
+          });
+    }
+  }
+
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
   constexpr static llvm::SourceMgr::DiagKind Warning =
       llvm::SourceMgr::DK_Warning;

clang-tools-extra/clangd/ConfigFragment.h

@@ -174,6 +174,29 @@ 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.
+  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

clang-tools-extra/clangd/ConfigYAML.cpp

@@ -40,15 +40,18 @@ 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.parse(N);
     return !(N.failed() || HadError);
   }
 
 private:
   void parse(Fragment::IfBlock &F, Node &N) {
     DictParser Dict("If", this);
-    Dict.unrecognized(
-        [&](llvm::StringRef) { F.HasUnrecognizedCondition = true; });
+    Dict.unrecognized([&](Located<std::string>, Node &) {
+      F.HasUnrecognizedCondition = true;
+      return true; // Emit a warning for the unrecognized key.
+    });
     Dict.handle("PathMatch", [&](Node &N) {
       if (auto Values = scalarValues(N))
         F.PathMatch = std::move(*Values);
@@ -82,6 +85,28 @@ class Parser {
     Dict.parse(N);
   }
 
+  void parse(Fragment::ClangTidyBlock &F, Node &N) {
+    DictParser Dict("ClangTidy", this);
+    Dict.handle("Add", [&](Node &N) {
+      if (auto Values = scalarValues(N))
+        F.Add = std::move(*Values);
+    });
+    Dict.handle("Remove", [&](Node &N) {
+      if (auto Values = scalarValues(N))
+        F.Remove = std::move(*Values);
+    });
+    Dict.handle("CheckOptions", [&](Node &N) {
+      DictParser CheckOptDict("CheckOptions", this);
+      CheckOptDict.unrecognized([&](Located<std::string> &&Key, Node &Val) {
+        if (auto Value = scalarValue(Val, *Key))
+          F.CheckOptions.emplace_back(std::move(Key), std::move(*Value));
+        return false; // Don't emit a warning
+      });
+      CheckOptDict.parse(N);
+    });
+    Dict.parse(N);
+  }
+
   void parse(Fragment::IndexBlock &F, Node &N) {
     DictParser Dict("Index", this);
     Dict.handle("Background",
@@ -94,7 +119,7 @@ class Parser {
   class DictParser {
     llvm::StringRef Description;
     std::vector<std::pair<llvm::StringRef, std::function<void(Node &)>>> Keys;
-    std::function<void(llvm::StringRef)> Unknown;
+    std::function<bool(Located<std::string>, Node &)> UnknownHandler;
     Parser *Outer;
 
   public:
@@ -112,10 +137,12 @@ class Parser {
       Keys.emplace_back(Key, std::move(Parse));
     }
 
-    // Fallback is called when a Key is not matched by any handle().
-    // A warning is also automatically emitted.
-    void unrecognized(std::function<void(llvm::StringRef)> Fallback) {
-      Unknown = std::move(Fallback);
+    // Handler is called when a Key is not matched by any handle().
+    // If this is unset or the Handler returns true, a warning is emitted for
+    // the unknown key.
+    void
+    unrecognized(std::function<bool(Located<std::string>, Node &)> Handler) {
+      UnknownHandler = std::move(Handler);
     }
 
     // Process a mapping node and call handlers for each key/value pair.
@@ -135,6 +162,8 @@ class Parser {
           continue;
         if (!Seen.insert(**Key).second) {
           Outer->warning("Duplicate key " + **Key + " is ignored", *K);
+          if (auto *Value = KV.getValue())
+            Value->skip();
           continue;
         }
         auto *Value = KV.getValue();
@@ -149,9 +178,12 @@ class Parser {
           }
         }
         if (!Matched) {
-          Outer->warning("Unknown " + Description + " key " + **Key, *K);
-          if (Unknown)
-            Unknown(**Key);
+          bool Warn = !UnknownHandler;
+          if (UnknownHandler)
+            Warn = UnknownHandler(
+                Located<std::string>(**Key, K->getSourceRange()), *Value);
+          if (Warn)
+            Outer->warning("Unknown " + Description + " key " + **Key, *K);
         }
       }
     }

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

@@ -176,6 +176,26 @@ TEST_F(ConfigCompileTests, PathSpecMatch) {
     ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   }
 }
+
+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(
+      std::make_pair(std::string("StrictMode"), std::string("true")));
+  Frag.ClangTidy.CheckOptions.emplace_back(std::make_pair(
+      std::string("example-check.ExampleOption"), std::string("0")));
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(
+      Conf.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"),
+            "0");
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd

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

@@ -35,6 +35,14 @@ MATCHER_P(Val, Value, "") {
   return false;
 }
 
+MATCHER_P2(PairVal, Value1, Value2, "") {
+  if (*arg.first == Value1 && *arg.second == Value2)
+    return true;
+  *result_listener << "values are [" << *arg.first << ", " << *arg.second
+                   << "]";
+  return false;
+}
+
 TEST(ParseYAML, SyntacticForms) {
   CapturedDiags Diags;
   const char *YAML = R"yaml(
@@ -50,10 +58,15 @@ CompileFlags: { Add: [foo, bar] }
 ---
 Index:
   Background: Skip
+---
+ClangTidy: 
+  CheckOptions: 
+    IgnoreMacros: true
+    example-check.ExampleOption: 0
   )yaml";
   auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_EQ(Results.size(), 3u);
+  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")));
@@ -62,6 +75,9 @@ CompileFlags: { Add: [foo, bar] }
 
   ASSERT_TRUE(Results[2].Index.Background);
   EXPECT_EQ("Skip", *Results[2].Index.Background.getValue());
+  EXPECT_THAT(Results[3].ClangTidy.CheckOptions,
+              ElementsAre(PairVal("IgnoreMacros", "true"),
+                          PairVal("example-check.ExampleOption", "0")));
 }
 
 TEST(ParseYAML, Locations) {
@@ -84,11 +100,11 @@ TEST(ParseYAML, Diagnostics) {
   CapturedDiags Diags;
   Annotations YAML(R"yaml(
 If:
-  [[UnknownCondition]]: "foo"
+  $unknown[[UnknownCondition]]: "foo"
 CompileFlags:
   Add: 'first'
 ---
-CompileFlags: {^
+CompileFlags: {$unexpected^
 )yaml");
   auto Results =
       Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
@@ -97,11 +113,13 @@ CompileFlags: {^
       Diags.Diagnostics,
       ElementsAre(AllOf(DiagMessage("Unknown If key UnknownCondition"),
                         DiagKind(llvm::SourceMgr::DK_Warning),
-                        DiagPos(YAML.range().start), DiagRange(YAML.range())),
+                        DiagPos(YAML.range("unknown").start),
+                        DiagRange(YAML.range("unknown"))),
                   AllOf(DiagMessage("Unexpected token. Expected Key, Flow "
                                     "Entry, or Flow Mapping End."),
                         DiagKind(llvm::SourceMgr::DK_Error),
-                        DiagPos(YAML.point()), DiagRange(llvm::None))));
+                        DiagPos(YAML.point("unexpected")),
+                        DiagRange(llvm::None))));
 
   ASSERT_EQ(Results.size(), 1u); // invalid fragment discarded.
   EXPECT_THAT(Results.front().CompileFlags.Add, ElementsAre(Val("first")));