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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
