[llvm-branch-commits] [clang-tools-extra] 20b69af - [clangd] Add clang-tidy options to config
Nathan James via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Nov 22 02:08:55 PST 2020
Author: Nathan James
Date: 2020-11-22T10:04:01Z
New Revision: 20b69af7c9c8bd9a621b05203f944bf94a3a4c26
URL: https://github.com/llvm/llvm-project/commit/20b69af7c9c8bd9a621b05203f944bf94a3a4c26
DIFF: https://github.com/llvm/llvm-project/commit/20b69af7c9c8bd9a621b05203f944bf94a3a4c26.diff
LOG: [clangd] Add clang-tidy options to config
First step of implementing clang-tidy configuration into clangd config.
This is just adding support for reading and verifying the clang tidy options from the config fragments.
No support is added for actually using the options within clang-tidy yet.
That will be added in a follow up as its a little more involved.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D90531
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 087dc4fb805d..ff285d34633b 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/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 @@ struct Config {
// ::). 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
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index e2823894dd11..ff031238418d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -157,6 +157,7 @@ struct FragmentCompiler {
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 @@ struct FragmentCompiler {
}
}
+ 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;
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 65772715095f..766463e11e25 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -174,6 +174,29 @@ struct Fragment {
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
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index bc7d4fdfe85b..742abb42670f 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -40,6 +40,7 @@ class Parser {
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 @@ class Parser {
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 @@ class Parser {
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 Parser {
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 @@ class Parser {
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 @@ class Parser {
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 @@ class Parser {
}
}
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);
}
}
}
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index f791820f7e45..6e535f3bea97 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -176,6 +176,26 @@ TEST_F(ConfigCompileTests, PathSpecMatch) {
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(), 2U);
+ EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("StrictMode"), "true");
+ EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("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
index 27b1c0cfc56d..9cdfdf9657d3 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -35,6 +35,14 @@ MATCHER_P(Val, Value, "") {
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,15 @@ CompileFlags: { Add: [foo, bar] }
---
Index:
Background: Skip
+---
+ClangTidy:
+ 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 +75,9 @@ CompileFlags: { Add: [foo, bar] }
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 +100,11 @@ TEST(ParseYAML, Diagnostics) {
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 +113,13 @@ CompileFlags: {^
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")));
More information about the llvm-branch-commits
mailing list