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 @@ -26,6 +26,7 @@ #include "support/Context.h" #include "llvm/ADT/FunctionExtras.h" +#include "llvm/ADT/StringMap.h" #include #include @@ -70,6 +71,14 @@ // ::). All nested namespaces are affected as well. std::vector 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 CheckOptions; + } ClangTidy; }; } // namespace clangd 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 @@ -157,6 +157,7 @@ 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 @@ } } + void appendTidyCheckSpec(std::string &CurSpec, + const Located &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> 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; 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 @@ -174,6 +174,29 @@ std::vector> 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> 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> 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, Located>> + CheckOptions; + }; + ClangTidyBlock ClangTidy; }; } // namespace config 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 @@ -40,6 +40,7 @@ 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); } @@ -47,8 +48,10 @@ private: void parse(Fragment::IfBlock &F, Node &N) { DictParser Dict("If", this); - Dict.unrecognized( - [&](llvm::StringRef) { F.HasUnrecognizedCondition = true; }); + Dict.unrecognized([&](Located, 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 @@ 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 &&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 DictParser { llvm::StringRef Description; std::vector>> Keys; - std::function Unknown; + std::function, Node &)> UnknownHandler; Parser *Outer; public: @@ -112,10 +137,12 @@ 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 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, Node &)> Handler) { + UnknownHandler = std::move(Handler); } // Process a mapping node and call handlers for each key/value pair. @@ -135,6 +162,8 @@ 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 @@ } } if (!Matched) { - Outer->warning("Unknown " + Description + " key " + **Key, *K); - if (Unknown) - Unknown(**Key); + bool Warn = !UnknownHandler; + if (UnknownHandler) + Warn = UnknownHandler( + Located(**Key, K->getSourceRange()), *Value); + if (Warn) + Outer->warning("Unknown " + Description + " key " + **Key, *K); } } } 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 @@ -176,6 +176,26 @@ 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 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 @@ -35,6 +35,14 @@ 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 @@ --- 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 @@ 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 @@ 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 @@ 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")));