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 @@ -70,6 +70,13 @@ // ::). 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 { + bool Enable; + std::string Checks; + std::vector> 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,68 @@ } } + void checkAdjuster(std::string &Adjuster, const Located &Arg, + bool IsPositive) { + SmallString<64> DiagBuffer; + StringRef Str = *Arg; + StringRef Type(IsPositive ? "Add" : "Remove"); + // Don't support negating here, its handled if the item is in the Add or + // Remove list. + if (Str[0] == '-') { + diag(Error, + (Type + " check item can't start with '-'").toStringRef(DiagBuffer), + Arg.Range); + return; + } + if (Str.contains(',')) { + diag(Error, + ("Invalid character ',' found in " + Type + " check item") + .toStringRef(DiagBuffer), + Arg.Range); + return; + } + if (!Adjuster.empty()) + Adjuster += ','; + if (!IsPositive) + Adjuster += '-'; + Adjuster += Str; + } + + void compile(Fragment::ClangTidyBlock &&F) { + if (F.Enable) + Out.Apply.push_back([Enable = **F.Enable](const Params &, Config &C) { + C.ClangTidy.Enable = Enable; + }); + + std::string Checks; + for (auto &CheckGlob : F.Add) + checkAdjuster(Checks, CheckGlob, true); + + for (auto &CheckGlob : F.Remove) + checkAdjuster(Checks, CheckGlob, false); + + if (!Checks.empty()) + Out.Apply.push_back( + [Checks = std::move(Checks)](const Params &, Config &C) { + if (C.ClangTidy.Checks.empty()) + C.ClangTidy.Checks = Checks; + else + C.ClangTidy.Checks += ',' + Checks; + }); + 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) { + C.ClangTidy.CheckOptions.insert(C.ClangTidy.CheckOptions.end(), + CheckOptions.begin(), + CheckOptions.end()); + }); + } + } + 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,33 @@ std::vector> FullyQualifiedNamespaces; }; StyleBlock Style; + + /// This section 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 { + llvm::Optional> Enable; + /// List of checks to enable or disable, can use wildcards. + /// \ref Add checks get ran first, then \ref Remove checks. This lets you + /// enable all checks from a module apart from a few specific checks, + /// Example: + /// Add: llvm- + /// Remove: llvm-include-order + /// This will enable all checks from the llvm module apart from + /// llvm-include-order. + std::vector> Add; + 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); } @@ -82,6 +83,33 @@ Dict.parse(N); } + void parse(Fragment::ClangTidyBlock &F, Node &N) { + DictParser Dict("ClangTidy", this); + Dict.handle("Enable", [&](Node &N) { + if (auto Value = scalarBool(N, "Enable")) + F.Enable = *Value; + }); + 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) { + DynamicDictParser DynamicDict( + "CheckOptions", + [&](Located &&Key, Node &Val) { + if (auto Value = scalarValue(Val, *Key)) + F.CheckOptions.emplace_back(std::move(Key), std::move(*Value)); + }, + this); + DynamicDict.parse(N); + }); + Dict.parse(N); + } + void parse(Fragment::IndexBlock &F, Node &N) { DictParser Dict("Index", this); Dict.handle("Background", @@ -157,6 +185,75 @@ } }; + class DynamicDictParser { + public: + using KeyValueHandler = + std::function &&, Node &)>; + + private: + llvm::StringRef Description; + KeyValueHandler Handler; + Parser *Outer; + + public: + DynamicDictParser(llvm::StringRef Description, KeyValueHandler Handler, + Parser *Parent) + : Description(Description), Handler(Handler), Outer(Parent) {} + + void parse(Node &N) { + if (N.getType() != Node::NK_Mapping) { + Outer->error(Description + " should be a dictionary", N); + return; + } + llvm::SmallString<64> MsgDesc({Description, " key"}); + llvm::SmallSet Seen; + + for (auto &KV : llvm::cast(N)) { + auto *K = KV.getKey(); + if (!K) // YAMLParser emitted an error. + continue; + auto Key = Outer->scalarValue(*K, MsgDesc); + if (!Key) + continue; + if (!Seen.insert(**Key).second) { + Outer->warning("Duplicate " + Description + " key '" + **Key + + "' is ignored", + *K); + if (auto *V = KV.getValue()) + V->skip(); + continue; + } + auto *V = KV.getValue(); + if (!V) // YAMLParser emitted an error. + continue; + Handler(std::move(*Key), *V); + } + } + }; + + // Try to parse a single boolean value from the node, warn on failure. + llvm::Optional> scalarBool(Node &N, llvm::StringRef Desc) { + llvm::SmallString<256> Buf; + llvm::StringRef Str; + if (auto *S = llvm::dyn_cast(&N)) + Str = S->getValue(Buf).trim(); + else if (auto *BS = llvm::dyn_cast(&N)) + Str = BS->getValue().trim(); + else { + warning(Desc + " should be a boolean", N); + return llvm::None; + } + if (Str.equals_lower("true")) + return Located(true, N.getSourceRange()); + if (Str.equals_lower("false")) + return Located(false, N.getSourceRange()); + bool Result; + if (!Str.getAsInteger(10, Result)) + return Located(Result, N.getSourceRange()); + warning("Couldn't parse '" + Str + "' as a boolean for " + Desc, N); + return llvm::None; + } + // Try to parse a single scalar value from the node, warn on failure. llvm::Optional> scalarValue(Node &N, llvm::StringRef Desc) { 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,25 @@ 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_THAT(Conf.ClangTidy.CheckOptions, + ElementsAre(std::make_pair("StrictMode", "true"), + std::make_pair("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,18 @@ --- Index: Background: Skip +--- +ClangTidy: { Enable: true } +--- +ClangTidy: + Enable: 0 + 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(), 5u); 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 +78,13 @@ ASSERT_TRUE(Results[2].Index.Background); EXPECT_EQ("Skip", *Results[2].Index.Background.getValue()); + ASSERT_TRUE(Results[3].ClangTidy.Enable); + EXPECT_TRUE(**Results[3].ClangTidy.Enable); + ASSERT_TRUE(Results[4].ClangTidy.Enable); + EXPECT_FALSE(**Results[4].ClangTidy.Enable); + EXPECT_THAT(Results[4].ClangTidy.CheckOptions, + ElementsAre(PairVal("IgnoreMacros", "true"), + PairVal("example-check.ExampleOption", "0"))); } TEST(ParseYAML, Locations) { @@ -84,26 +107,36 @@ CapturedDiags Diags; Annotations YAML(R"yaml( If: - [[UnknownCondition]]: "foo" + $unknown[[UnknownCondition]]: "foo" CompileFlags: Add: 'first' --- -CompileFlags: {^ +ClangTidy: + Enable: $notBool[[NotABool]] +--- +CompileFlags: {$unexpected^ )yaml"); auto Results = Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback()); ASSERT_THAT( Diags.Diagnostics, - ElementsAre(AllOf(DiagMessage("Unknown If key UnknownCondition"), - DiagKind(llvm::SourceMgr::DK_Warning), - DiagPos(YAML.range().start), DiagRange(YAML.range())), - AllOf(DiagMessage("Unexpected token. Expected Key, Flow " - "Entry, or Flow Mapping End."), - DiagKind(llvm::SourceMgr::DK_Error), - DiagPos(YAML.point()), DiagRange(llvm::None)))); + ElementsAre( + AllOf(DiagMessage("Unknown If key UnknownCondition"), + DiagKind(llvm::SourceMgr::DK_Warning), + DiagPos(YAML.range("unknown").start), + DiagRange(YAML.range("unknown"))), + AllOf( + DiagMessage("Couldn't parse 'NotABool' as a boolean for Enable"), + DiagKind(llvm::SourceMgr::DK_Warning), + DiagPos(YAML.range("notBool").start), + DiagRange(YAML.range("notBool"))), + AllOf(DiagMessage("Unexpected token. Expected Key, Flow " + "Entry, or Flow Mapping End."), + DiagKind(llvm::SourceMgr::DK_Error), + DiagPos(YAML.point("unexpected")), DiagRange(llvm::None)))); - ASSERT_EQ(Results.size(), 1u); // invalid fragment discarded. + ASSERT_EQ(Results.size(), 2u); // invalid fragment discarded. EXPECT_THAT(Results.front().CompileFlags.Add, ElementsAre(Val("first"))); EXPECT_TRUE(Results.front().If.HasUnrecognizedCondition); }