njames93 updated this revision to Diff 302141.
njames93 added a comment.
Added unittests for reading and compiling config and fixed the bugs causing
said unittests to fail.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90531/new/
https://reviews.llvm.org/D90531
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigTidyProvider.cpp
clang-tools-extra/clangd/ConfigTidyProvider.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ 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].Tidy.Enable);
+ EXPECT_TRUE(**Results[3].Tidy.Enable);
+ ASSERT_TRUE(Results[4].Tidy.Enable);
+ EXPECT_FALSE(**Results[4].Tidy.Enable);
+ EXPECT_THAT(Results[4].Tidy.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);
}
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ 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.Tidy.Add.emplace_back("bugprone-use-after-move");
+ Frag.Tidy.Add.emplace_back("llvm-*");
+ Frag.Tidy.Remove.emplace_back("llvm-include-order");
+ Frag.Tidy.Remove.emplace_back("readability-*");
+ Frag.Tidy.CheckOptions.emplace_back(
+ std::make_pair(std::string("StrictMode"), std::string("true")));
+ Frag.Tidy.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
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ 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::FileOptionsProvider>(
- tidy::ClangTidyGlobalOptions(),
- /* Default */ EmptyDefaults,
- /* Override */ OverrideClangTidyOptions, TFS.view(/*CWD=*/llvm::None));
+ if (Opts.ConfigProvider)
+ ClangTidyOptProvider = std::make_unique<ConfigTidyOptionsProvider>(
+ tidy::ClangTidyGlobalOptions(),
+ /* Default */ EmptyDefaults, *Opts.ConfigProvider,
+ /* Override */ OverrideClangTidyOptions,
+ TFS.view(/*CWD=*/llvm::None));
+ else
+ ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
+ 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.
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ 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.Tidy, N); });
Dict.parse(N);
return !(N.failed() || HadError);
}
@@ -82,6 +83,50 @@
Dict.parse(N);
}
+ void parse(Fragment::TidyBlock &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) {
+ if (N.getType() != Node::NK_Mapping) {
+ this->error("CheckOptions should be a dictionary", N);
+ return;
+ }
+ llvm::SmallSet<std::string, 8> Seen;
+
+ for (auto &KV : llvm::cast<MappingNode>(N)) {
+ auto *K = KV.getKey();
+ if (!K) // YAMLParser emitted an error.
+ continue;
+ auto Key = this->scalarValue(*K, "CheckOption Key");
+ if (!Key)
+ continue;
+ if (!Seen.insert(**Key).second) {
+ this->warning("Duplicate CheckOption " + **Key + " is ignored", *K);
+ continue;
+ }
+ auto *V = KV.getValue();
+ if (!V) // YAMLParser emitted an error.
+ continue;
+ auto Value = this->scalarValue(*V, **Key);
+ if (!Value) // Couldn't parse V as a String
+ continue;
+ F.CheckOptions.emplace_back(std::move(*Key), std::move(*Value));
+ }
+ });
+ Dict.parse(N);
+ }
+
void parse(Fragment::IndexBlock &F, Node &N) {
DictParser Dict("Index", this);
Dict.handle("Background",
@@ -157,6 +202,29 @@
}
};
+ // Try to parse a single boolean value from the node, warn on failure.
+ llvm::Optional<Located<bool>> scalarBool(Node &N, llvm::StringRef Desc) {
+ llvm::SmallString<256> Buf;
+ llvm::StringRef Str;
+ if (auto *S = llvm::dyn_cast<ScalarNode>(&N))
+ Str = S->getValue(Buf).trim();
+ else if (auto *BS = llvm::dyn_cast<BlockScalarNode>(&N))
+ Str = BS->getValue().trim();
+ else {
+ warning(Desc + " should be a boolean", N);
+ return llvm::None;
+ }
+ if (Str.equals_lower("true"))
+ return Located<bool>(true, N.getSourceRange());
+ if (Str.equals_lower("false"))
+ return Located<bool>(false, N.getSourceRange());
+ bool Result;
+ if (!Str.getAsInteger(10, Result))
+ return Located<bool>(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<Located<std::string>> scalarValue(Node &N,
llvm::StringRef Desc) {
Index: clang-tools-extra/clangd/ConfigTidyProvider.h
===================================================================
--- /dev/null
+++ 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<llvm::vfs::FileSystem> FS = nullptr);
+
+ std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+
+private:
+ config::Provider &ConfigProvider;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H
\ No newline at end of file
Index: clang-tools-extra/clangd/ConfigTidyProvider.cpp
===================================================================
--- /dev/null
+++ 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<llvm::vfs::FileSystem> FS)
+ : tidy::FileOptionsBaseProvider(GlobalOptions, DefaultOptions,
+ OverrideOptions, FS),
+ ConfigProvider(ConfigProvider) {}
+
+std::vector<tidy::ClangTidyOptionsProvider::OptionsSource>
+ConfigTidyOptionsProvider::getRawOptions(llvm::StringRef FileName) {
+ std::vector<OptionsSource> 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
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -171,6 +171,33 @@
std::vector<Located<std::string>> 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 TidyBlock {
+ llvm::Optional<Located<bool>> 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<Located<std::string>> Add;
+ 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;
+ };
+ TidyBlock Tidy;
};
} // namespace config
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ 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.Tidy));
}
void compile(Fragment::IfBlock &&F) {
@@ -227,6 +228,63 @@
}
}
+ void checkAdjuster(std::string &Adjuster, const Located<std::string> &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::TidyBlock &&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<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)](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;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -70,6 +70,13 @@
// ::). 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 {
+ bool Enable;
+ std::string Checks;
+ std::vector<std::pair<std::string, std::string>> CheckOptions;
+ } ClangTidy;
};
} // namespace clangd
Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -49,6 +49,7 @@
Compiler.cpp
Config.cpp
ConfigCompile.cpp
+ ConfigTidyProvider.cpp
ConfigProvider.cpp
ConfigYAML.cpp
Diagnostics.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
