njames93 updated this revision to Diff 303596.
njames93 marked an inline comment as done.
njames93 added a comment.
Removed Enable.
Removed the scalarBool parser as it was only needed by Enable.
Removed DynamicDictParser in favour of reworking DictParser to get better
control of how to handle unknown keys.
Use a StringMap to store the CheckOptions in the Config.
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,16 @@
---
Index:
Background: Skip
+---
+ClangTidy:
+ Enable: off
+ 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 +76,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 +101,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 +114,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")));
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_EQ(Conf.ClangTidy.CheckOptions.size(), 2);
+ EXPECT_STREQ(Conf.ClangTidy.CheckOptions.lookup("StrictMode"), "true"),
+ EXPECT_STREQ(Conf.ClangTidy.CheckOptions.lookup("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);
}
@@ -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<std::string>, 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<std::string> &&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<std::pair<llvm::StringRef, std::function<void(Node &)>>> Keys;
- std::function<void(llvm::StringRef)> Unknown;
+ std::function<bool(Located<std::string>, 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<void(llvm::StringRef)> 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<bool(Located<std::string>, 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<std::string>(**Key, K->getSourceRange()), *Value);
+ if (Warn)
+ Outer->warning("Unknown " + Description + " key " + **Key, *K);
}
}
}
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -174,6 +174,29 @@
std::vector<Located<std::string>> 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<Located<std::string>> 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<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,49 @@
}
}
+ void appendTidyCheckSpec(std::string &CurSpec,
+ const Located<std::string> &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<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) {
+ 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;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ 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 <string>
#include <vector>
@@ -70,6 +71,14 @@
// ::). 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 {
+ // A comma-seperated list of globs to specify which clang-tidy checks to
+ // run.
+ std::string Checks;
+ llvm::StringMap<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
