[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

Author: njames93

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

@@ -81,18 +81,52 @@ 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);
     IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
     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);

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.

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.')

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";
 

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
 ^^^^^^^^^^
 

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

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:

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'}}

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'
+

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