diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -49,6 +49,7 @@ Compiler.cpp Config.cpp ConfigCompile.cpp + ConfigTidyProvider.cpp ConfigProvider.cpp ConfigYAML.cpp Diagnostics.cpp 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 @@ -132,6 +132,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) { @@ -227,6 +228,63 @@ } } + 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](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)](Config &C) { + C.ClangTidy.Checks = std::move(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)](Config &C) { + C.ClangTidy.CheckOptions.insert( + C.ClangTidy.CheckOptions.end(), + std::make_move_iterator(CheckOptions.begin()), + std::make_move_iterator(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 @@ -171,6 +171,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/ConfigTidyProvider.h b/clang-tools-extra/clangd/ConfigTidyProvider.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/ConfigTidyProvider.h @@ -0,0 +1,43 @@ +//===--- ConfigTidyProvider.h ------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Creates a ClangTidyOptionsProvider that merges a users clangd config with +// .clang-tidy files +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H + +#include "../clang-tidy/ClangTidyOptions.h" + +namespace clang { +namespace clangd { +namespace config { +class Provider; +} // namespace config + +class ConfigTidyOptionsProvider : public tidy::FileOptionsBaseProvider { +public: + ConfigTidyOptionsProvider( + const tidy::ClangTidyGlobalOptions &GlobalOptions, + const tidy::ClangTidyOptions &DefaultOptions, + config::Provider &ConfigProvider, + const tidy::ClangTidyOptions &OverrideOptions, + llvm::IntrusiveRefCntPtr FS = nullptr); + + std::vector getRawOptions(llvm::StringRef FileName) override; + +private: + config::Provider &ConfigProvider; +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H diff --git a/clang-tools-extra/clangd/ConfigTidyProvider.cpp b/clang-tools-extra/clangd/ConfigTidyProvider.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/ConfigTidyProvider.cpp @@ -0,0 +1,57 @@ +//===--- ConfigTidyProvider.cpp ----------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ConfigTidyProvider.h" +#include "../clang-tidy/ClangTidyOptions.h" +#include "Config.h" +#include "ConfigProvider.h" + +namespace clang { +namespace clangd { +ConfigTidyOptionsProvider::ConfigTidyOptionsProvider( + const tidy::ClangTidyGlobalOptions &GlobalOptions, + const tidy::ClangTidyOptions &DefaultOptions, + config::Provider &ConfigProvider, + const tidy::ClangTidyOptions &OverrideOptions, + llvm::IntrusiveRefCntPtr FS) + : tidy::FileOptionsBaseProvider(GlobalOptions, DefaultOptions, + OverrideOptions, FS), + ConfigProvider(ConfigProvider) {} + +std::vector +ConfigTidyOptionsProvider::getRawOptions(llvm::StringRef FileName) { + std::vector RawOptions = + DefaultOptionsProvider::getRawOptions(FileName); + assert(FS && "FS must be set."); + + llvm::SmallString<128> AbsoluteFilePath(FileName); + + if (!FS->makeAbsolute(AbsoluteFilePath)) { + addRawFileOptions(AbsoluteFilePath, RawOptions); + } + + config::Params P; + P.Path = AbsoluteFilePath; + auto IgnoreDiag = [](const llvm::SMDiagnostic &) {}; + Config C = ConfigProvider.getConfig(P, IgnoreDiag); + + tidy::ClangTidyOptions ConfigOptions; + + if (!C.ClangTidy.Checks.empty()) + ConfigOptions.Checks = C.ClangTidy.Checks; + for (auto &Item : C.ClangTidy.CheckOptions) + ConfigOptions.CheckOptions.try_emplace(Item.first, Item.second); + + RawOptions.emplace_back(ConfigOptions, ".clangd config"); + + RawOptions.emplace_back(OverrideOptions, + "command-line option '-clang-tidy-checks'"); + return RawOptions; +} +} // namespace clangd +} // namespace clang 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,32 @@ 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 Parser( + "CheckOptions", + [&](Located &&Key, Node &Val) { + if (auto Value = scalarValue(Val, *Key)) + F.CheckOptions.emplace_back(std::move(Key), std::move(*Value)); + }, + this); + }); + Dict.parse(N); + } + void parse(Fragment::IndexBlock &F, Node &N) { DictParser Dict("Index", this); Dict.handle("Background", @@ -157,6 +184,70 @@ } }; + class DynamicDictParser { + public: + using KeyValueHandler = + llvm::function_ref &&, 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::SmallSet Seen; + + for (auto &KV : llvm::cast(N)) { + auto *K = KV.getKey(); + if (!K) // YAMLParser emitted an error. + continue; + auto Key = Outer->scalarValue(*K, "CheckOption Key"); + if (!Key) + continue; + if (!Seen.insert(**Key).second) { + Outer->warning("Duplicate CheckOption " + **Key + " is ignored", *K); + 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/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -8,6 +8,7 @@ #include "ClangdLSPServer.h" #include "CodeComplete.h" +#include "ConfigTidyProvider.h" #include "Features.inc" #include "PathMapping.h" #include "Protocol.h" @@ -811,10 +812,18 @@ tidy::ClangTidyOptions OverrideClangTidyOptions; if (!ClangTidyChecks.empty()) OverrideClangTidyOptions.Checks = ClangTidyChecks; - ClangTidyOptProvider = std::make_unique( - tidy::ClangTidyGlobalOptions(), - /* Default */ EmptyDefaults, - /* Override */ OverrideClangTidyOptions, TFS.view(/*CWD=*/llvm::None)); + if (Opts.ConfigProvider) + ClangTidyOptProvider = std::make_unique( + tidy::ClangTidyGlobalOptions(), + /* Default */ EmptyDefaults, *Opts.ConfigProvider, + /* Override */ OverrideClangTidyOptions, + TFS.view(/*CWD=*/llvm::None)); + else + ClangTidyOptProvider = std::make_unique( + tidy::ClangTidyGlobalOptions(), + /* Default */ EmptyDefaults, + /* Override */ OverrideClangTidyOptions, + TFS.view(/*CWD=*/llvm::None)); Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &, llvm::StringRef File) { // This function must be thread-safe and tidy option providers are not. 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 @@ -116,6 +116,24 @@ "Invalid Background value 'Foo'. Valid values are Build, Skip."))); } +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); }