[clang-tools-extra] fbf611e - [clang-tidy] Extend spelling for CheckOptions
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 23 11:59:37 PDT 2022
Author: Nathan James
Date: 2022-06-23T19:59:31+01:00
New Revision: fbf611ed2a768999202e2c5e1e1a6c3c6bb94725
URL: https://github.com/llvm/llvm-project/commit/fbf611ed2a768999202e2c5e1e1a6c3c6bb94725
DIFF: https://github.com/llvm/llvm-project/commit/fbf611ed2a768999202e2c5e1e1a6c3c6bb94725.diff
LOG: [clang-tidy] Extend spelling for CheckOptions
The current way to specify CheckOptions is pretty verbose and unintuitive.
Given that the options are a dictionary it makes much more sense to treat them as such in the config files.
Example:
```
CheckOptions: {SomeCheck.Option: true, SomeCheck.OtherOption: 'ignore'}
# Or
CheckOptions:
SomeCheck.Option: true
SomeCheck.OtherOption: 'ignore'
```
This change will still handle the old syntax with no issue, ensuring we don't screw up users current config files.
The only observable differences are support for the new syntax and `-dump=config` will emit using the new syntax.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D128337
Added:
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
Modified:
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/Contributing.rst
clang-tools-extra/docs/clang-tidy/index.rst
clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index a12a4d6692c77..f07a8f9e893d8 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -81,10 +81,44 @@ struct NOptionMap {
std::vector<ClangTidyOptions::StringPair> Options;
};
+template <>
+void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
+ EmptyContext &Ctx) {
+ if (IO.outputting()) {
+ IO.beginMapping();
+ // Only output as a map
+ for (auto &Key : Options) {
+ bool UseDefault;
+ void *SaveInfo;
+ IO.preflightKey(Key.getKey().data(), true, false, UseDefault, SaveInfo);
+ StringRef S = Key.getValue().Value;
+ IO.scalarString(S, needsQuotes(S));
+ IO.postflightKey(SaveInfo);
+ }
+ IO.endMapping();
+ } else {
+ // We need custom logic here to support the old method of specifying check
+ // options using a list of maps containing key and value keys.
+ Input &I = reinterpret_cast<Input &>(IO);
+ if (isa<SequenceNode>(I.getCurrentNode())) {
+ MappingNormalization<NOptionMap, ClangTidyOptions::OptionMap> NOpts(
+ IO, Options);
+ EmptyContext Ctx;
+ yamlize(IO, NOpts->Options, true, Ctx);
+ } else if (isa<MappingNode>(I.getCurrentNode())) {
+ IO.beginMapping();
+ for (StringRef Key : IO.keys()) {
+ IO.mapRequired(Key.data(), Options[Key].Value);
+ }
+ IO.endMapping();
+ } else {
+ IO.setError("expected a sequence or map");
+ }
+ }
+}
+
template <> struct MappingTraits<ClangTidyOptions> {
static void mapping(IO &IO, ClangTidyOptions &Options) {
- MappingNormalization<NOptionMap, ClangTidyOptions::OptionMap> NOpts(
- IO, Options.CheckOptions);
bool Ignored = false;
IO.mapOptional("Checks", Options.Checks);
IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
@@ -92,7 +126,7 @@ template <> struct MappingTraits<ClangTidyOptions> {
IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // legacy compatibility
IO.mapOptional("FormatStyle", Options.FormatStyle);
IO.mapOptional("User", Options.User);
- IO.mapOptional("CheckOptions", NOpts->Options);
+ IO.mapOptional("CheckOptions", Options.CheckOptions);
IO.mapOptional("ExtraArgs", Options.ExtraArgs);
IO.mapOptional("ExtraArgsBefore", Options.ExtraArgsBefore);
IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 564f47d864eb5..b5e5191876cad 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -52,8 +52,7 @@ Configuration files:
InheritParentConfig: true
User: user
CheckOptions:
- - key: some-check.SomeOption
- value: 'some value'
+ some-check.SomeOption: 'some value'
...
)");
@@ -171,8 +170,7 @@ line or a specific configuration file.
static cl::opt<std::string> Config("config", cl::desc(R"(
Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: [{key: x,
- value: y}]}"
+ CheckOptions: {x: y}}"
When the value is empty, clang-tidy will
attempt to find a file named .clang-tidy for
each source file in its parent directories.
diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index 821b941d4c383..e3da6fb9b0967 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -236,8 +236,7 @@ def main():
config_group.add_argument('-config', default=None,
help='Specifies a configuration in YAML/JSON format: '
' -config="{Checks: \'*\', '
- ' CheckOptions: [{key: x, '
- ' value: y}]}" '
+ ' CheckOptions: {x: y}}" '
'When the value is empty, clang-tidy will '
'attempt to find a file named .clang-tidy for '
'each source file in its parent directories.')
diff --git a/clang-tools-extra/clangd/unittests/TidyProviderTests.cpp b/clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
index a16c87456a1a0..df3dcac0aa51a 100644
--- a/clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TidyProviderTests.cpp
@@ -20,20 +20,17 @@ TEST(TidyProvider, NestedDirectories) {
FS.Files[testPath(".clang-tidy")] = R"yaml(
Checks: 'llvm-*'
CheckOptions:
- - key: TestKey
- value: 1
+ TestKey: 1
)yaml";
FS.Files[testPath("sub1/.clang-tidy")] = R"yaml(
Checks: 'misc-*'
CheckOptions:
- - key: TestKey
- value: 2
+ TestKey: 2
)yaml";
FS.Files[testPath("sub1/sub2/.clang-tidy")] = R"yaml(
Checks: 'bugprone-*'
CheckOptions:
- - key: TestKey
- value: 3
+ TestKey: 3
InheritParentConfig: true
)yaml";
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7ffa6bcc83065..0d3f973c3bdeb 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,8 @@ Improvements to clang-tidy
- Added an option -verify-config which will check the config file to ensure each
`Checks` and `CheckOptions` entries are recognised.
+- .clang-tidy files can now use the more natural dictionary syntax for specifying `CheckOptions`.
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index e67de656f90da..0014bb23aee4f 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -519,17 +519,15 @@ be set in a ``.clang-tidy`` file in the following way:
.. code-block:: yaml
CheckOptions:
- - key: my-check.SomeOption1
- value: 123
- - key: my-check.SomeOption2
- value: 'some other value'
+ my-check.SomeOption1: 123
+ my-check.SomeOption2: 'some other value'
If you need to specify check options on a command line, you can use the inline
YAML format:
.. code-block:: console
- $ clang-tidy -config="{CheckOptions: [{key: a, value: b}, {key: x, value: y}]}" ...
+ $ clang-tidy -config="{CheckOptions: {a: b, x: y}}" ...
Testing Checks
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index 23d1769097a68..05e369818f069 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -135,8 +135,7 @@ An overview of all the command-line options:
--config=<string> -
Specifies a configuration in YAML/JSON format:
-config="{Checks: '*',
- CheckOptions: [{key: x,
- value: y}]}"
+ CheckOptions: {x, y}}"
When the value is empty, clang-tidy will
attempt to find a file named .clang-tidy for
each source file in its parent directories.
@@ -295,8 +294,7 @@ An overview of all the command-line options:
InheritParentConfig: true
User: user
CheckOptions:
- - key: some-check.SomeOption
- value: 'some value'
+ some-check.SomeOption: 'some value'
...
.. _clang-tidy-nolint:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/module.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
index 2c82237e4186d..d987e71ebafd2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/module.cpp
@@ -1,6 +1,6 @@
// RUN: clang-tidy -checks='-*,google*' -config='{}' -dump-config - -- | FileCheck %s
// CHECK: CheckOptions:
-// CHECK-DAG: {{- key: *google-readability-braces-around-statements.ShortStatementLines *[[:space:]] *value: *'1'}}
-// CHECK-DAG: {{- key: *google-readability-function-size.StatementThreshold *[[:space:]] *value: *'800'}}
-// CHECK-DAG: {{- key: *google-readability-namespace-comments.ShortNamespaceLines *[[:space:]] *value: *'10'}}
-// CHECK-DAG: {{- key: *google-readability-namespace-comments.SpacesBeforeComments *[[:space:]] *value: *'2'}}
+// CHECK-DAG: {{google-readability-braces-around-statements.ShortStatementLines: '1'}}
+// CHECK-DAG: {{google-readability-function-size.StatementThreshold: '800'}}
+// CHECK-DAG: {{google-readability-namespace-comments.ShortNamespaceLines: '10'}}
+// CHECK-DAG: {{google-readability-namespace-comments.SpacesBeforeComments: '2'}}
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
new file mode 100644
index 0000000000000..3abef4360d8d6
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
@@ -0,0 +1,7 @@
+InheritParentConfig: true
+Checks: 'llvm-qualified-auto'
+CheckOptions:
+ modernize-loop-convert.MaxCopySize: '20'
+ llvm-qualified-auto.AddConstToQualified: 'true'
+ IgnoreMacros: 'false'
+
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
index d708ec8777c9a..6c42bd7f495f7 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -15,12 +15,16 @@
// CHECK-COMMAND-LINE: HeaderFilterRegex: from command line
// For this test we have to use names of the real checks because otherwise values are ignored.
+// Running with the old key: <Key>, value: <value> CheckOptions
// RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
+// Running with the new <key>: <value> syntax
+// RUN: clang-tidy -dump-config %S/Inputs/config-files/4/key-dict/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
+
// CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto
-// CHECK-CHILD4-DAG: - key: llvm-qualified-auto.AddConstToQualified{{ *[[:space:]] *}}value: 'true'
-// CHECK-CHILD4-DAG: - key: modernize-loop-convert.MaxCopySize{{ *[[:space:]] *}}value: '20'
-// CHECK-CHILD4-DAG: - key: modernize-loop-convert.MinConfidence{{ *[[:space:]] *}}value: reasonable
-// CHECK-CHILD4-DAG: - key: modernize-use-using.IgnoreMacros{{ *[[:space:]] *}}value: 'false'
+// CHECK-CHILD4-DAG: llvm-qualified-auto.AddConstToQualified: 'true'
+// CHECK-CHILD4-DAG: modernize-loop-convert.MaxCopySize: '20'
+// CHECK-CHILD4-DAG: modernize-loop-convert.MinConfidence: reasonable
+// CHECK-CHILD4-DAG: modernize-use-using.IgnoreMacros: 'false'
// RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
// CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}{{[/\\]}}Inputs{{[/\\]}}config-files{{[/\\]}}4{{[/\\]}}44{{[/\\]}}.clang-tidy.
@@ -32,14 +36,21 @@
// RUN: Checks: -llvm-qualified-auto, \
// RUN: CheckOptions: [{key: modernize-loop-convert.MaxCopySize, value: 21}]}' \
// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD5
+// Also test with the {Key: Value} Syntax specified on command line
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{InheritParentConfig: true, \
+// RUN: Checks: -llvm-qualified-auto, \
+// RUN: CheckOptions: {modernize-loop-convert.MaxCopySize: 21}}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD5
+
// CHECK-CHILD5: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto,-llvm-qualified-auto
-// CHECK-CHILD5-DAG: - key: modernize-loop-convert.MaxCopySize{{ *[[:space:]] *}}value: '21'
-// CHECK-CHILD5-DAG: - key: modernize-loop-convert.MinConfidence{{ *[[:space:]] *}}value: reasonable
-// CHECK-CHILD5-DAG: - key: modernize-use-using.IgnoreMacros{{ *[[:space:]] *}}value: 'false'
+// CHECK-CHILD5-DAG: modernize-loop-convert.MaxCopySize: '21'
+// CHECK-CHILD5-DAG: modernize-loop-convert.MinConfidence: reasonable
+// CHECK-CHILD5-DAG: modernize-use-using.IgnoreMacros: 'false'
// RUN: clang-tidy -dump-config \
// RUN: --config='{InheritParentConfig: false, \
// RUN: Checks: -llvm-qualified-auto}' \
// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
// CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
-// CHECK-CHILD6-NOT: - key: modernize-use-using.IgnoreMacros
+// CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
More information about the cfe-commits
mailing list