njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh.
Herald added subscribers: carlosgalvezp, arphaman, xazax.hun.
Herald added a project: All.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang-tools-extra.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D128337
Files:
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/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/Inputs/config-files/4/key-dict/.clang-tidy
clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ 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
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
===================================================================
--- /dev/null
+++ 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'
+
Index: clang-tools-extra/test/clang-tidy/checkers/google-module.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/google-module.cpp
+++ 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'}}
Index: clang-tools-extra/docs/clang-tidy/index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -135,8 +135,7 @@
--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.
@@ -292,8 +291,7 @@
InheritParentConfig: true
User: user
CheckOptions:
- - key: some-check.SomeOption
- value: 'some value'
+ some-check.SomeOption: 'some value'
...
.. _clang-tidy-nolint:
Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -519,17 +519,15 @@
.. 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
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -110,6 +110,8 @@
from suppressing diagnostics associated with macro arguments. This fixes
`Issue 55134 <https://github.com/llvm/llvm-project/issues/55134>`_.
+- .clang-tidy files can now use the more natural dictionary syntax for specifying `CheckOptions`
+
New checks
^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===================================================================
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -236,8 +236,7 @@
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.')
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -51,8 +51,7 @@
InheritParentConfig: true
User: user
CheckOptions:
- - key: some-check.SomeOption
- value: 'some value'
+ some-check.SomeOption: 'some value'
...
)");
@@ -170,8 +169,7 @@
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.
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -81,10 +81,44 @@
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 @@
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
