njames93 updated this revision to Diff 302818.
njames93 added a comment.
Rebase and make apply take reference to Params.
Fix fragments checks being applied on top of each other instead of overwriting
the current config checks.
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/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.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].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);
}
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ 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
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.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<std::string> &&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<void(Located<std::string> &&, 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<std::string, 8> Seen;
+
+ for (auto &KV : llvm::cast<MappingNode>(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<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/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -174,6 +174,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 ClangTidyBlock {
+ 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;
+ };
+ ClangTidyBlock ClangTidy;
};
} // namespace config
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ 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<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::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<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) {
+ 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;
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
