njames93 created this revision.
njames93 added reviewers: carlosgalvezp, LegalizeAdulthood, aaron.ballman.
Herald added subscribers: arphaman, mgrang, 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.
Remove a lot of duplication in configuration files by supporting nested
dictionaries in the `CheckOptions` entry
For Example to set `ParameterCase`, `VariableCase` and `MemberCase` in
`readability-identifier-naming` this new syntax is supported
CheckOptions:
readability-identifier-naming:
ParameterCase: CamelCase
VariableCase: camelBack
MemberCase: UPPER_CASE
This maintains backwards compatability with the old method of input:
CheckOptions:
readability-identifier-naming.ParameterCase: CamelCase
And:
CheckOptions:
- key: readability-identifier-naming.ParameterCase
value: CamelCase
This new syntax is also able to be interleaved with the current dictionary type
input.
CheckOptions:
readability-identifier-naming.ParameterCase: CamelCase
readability-identifier-naming:
VariableCase: camelBack
MemberCase: UPPER_CASE
Typically the current dictionary syntax is meant for checks with only one
configured option
The `-dump-config` option has been updated to use the grouped syntax for checks
with more than one option(while also having a deterministic ordering)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147955
Files:
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/docs/ReleaseNotes.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/44/.clang-tidy
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
@@ -21,10 +21,14 @@
// 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: 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'
+// CHECK-CHILD4: llvm-qualified-auto.AddConstToQualified: 'true'
+// CHECK-CHILD4: modernize-loop-convert:
+// CHECK-CHILD4-NEXT: IncludeStyle: llvm
+// CHECK-CHILD4-NEXT: MakeReverseRangeFunction: Reverse
+// CHECK-CHILD4-NEXT: MakeReverseRangeHeader: ''
+// CHECK-CHILD4-NEXT: MaxCopySize: '20'
+// CHECK-CHILD4-NEXT: MinConfidence: reasonable
+// CHECK-CHILD4: 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.
@@ -44,9 +48,10 @@
// 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: modernize-loop-convert.MaxCopySize: '21'
-// CHECK-CHILD5-DAG: modernize-loop-convert.MinConfidence: reasonable
-// CHECK-CHILD5-DAG: modernize-use-using.IgnoreMacros: 'false'
+// CHECK-CHILD5: modernize-loop-convert:
+// CHECK-CHILD5: MaxCopySize: '21'
+// CHECK-CHILD5: MinConfidence: reasonable
+// CHECK-CHILD5: modernize-use-using.IgnoreMacros: 'false'
// RUN: clang-tidy -dump-config \
// RUN: --config='{InheritParentConfig: false, \
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/key-dict/.clang-tidy
@@ -4,4 +4,7 @@
modernize-loop-convert.MaxCopySize: '20'
llvm-qualified-auto.AddConstToQualified: 'true'
IgnoreMacros: 'false'
+ modernize-loop-convert:
+ MakeReverseRangeFunction: Reverse
+ UseCxx20ReverseRanges: false
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy
@@ -7,3 +7,7 @@
value: 'true'
- key: IgnoreMacros
value: 'false'
+ - key: modernize-loop-convert.MakeReverseRangeFunction
+ value: Reverse
+ - key: modernize-loop-convert.UseCxx20ReverseRanges
+ value: 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,8 @@
// RUN: clang-tidy -checks='-*,google*' -config='{}' -dump-config - -- | FileCheck %s
// CHECK: CheckOptions:
-// 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'}}
+// CHECK: {{ google-readability-braces-around-statements.ShortStatementLines: '1'}}
+// CHECK: {{ google-readability-function-size:}}
+// CHECK: {{ StatementThreshold: '800'}}
+// CHECK: {{ google-readability-namespace-comments:}}
+// CHECK-NEXT: {{ ShortNamespaceLines: '10'}}
+// CHECK-NEXT: {{ 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
@@ -259,6 +259,9 @@
options. Example:
CheckOptions:
some-check.SomeOption: 'some value'
+ multi-option-check:
+ Option1: 'option_1_value'
+ Option2: 'option_2_value'
Checks - Same as '--checks'.
ExtraArgs - Same as '--extra-args'.
ExtraArgsBefore - Same as '--extra-args-before'.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,9 @@
`ImplementationFileExtensions`, replacing the check-local options of the
same name.
+- Ability to group `CheckOptions` for a specific check in the `.clang-tidy`
+ configuration file.
+
New checks
^^^^^^^^^^
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
@@ -52,6 +52,9 @@
options. Example:
CheckOptions:
some-check.SomeOption: 'some value'
+ multi-option-check:
+ Option1: 'option_1_value'
+ Option2: 'option_2_value'
Checks - Same as '--checks'.
ExtraArgs - Same as '--extra-args'.
ExtraArgsBefore - Same as '--extra-args-before'.
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -84,17 +84,72 @@
template <>
void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
EmptyContext &Ctx) {
+
+ bool UseDefault;
+ bool UseDefault2;
+ void *SaveInfo;
+ void *SaveInfo2;
if (IO.outputting()) {
+ // Nested option names need to be converted to null terminated strings for
+ // yaml serialization
+ llvm::SmallString<64> Buffer;
+ auto ToCString = [&Buffer](StringRef Str) {
+ Buffer.assign(Str);
+ return Buffer.c_str();
+ };
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;
+ // Using a map here means we have a deterministic order
+ std::map<StringRef, std::vector<std::pair<StringRef, StringRef>>>
+ LocalItems;
+ SmallVector<std::pair<StringRef, StringRef>, 8> GlobalItems;
+ for (auto &Entry : Options) {
+ auto Key = Entry.getKey();
+ // Look for a '.' seperator in the key name
+ auto Idx = Key.find('.');
+ if (Idx == StringRef::npos) {
+ GlobalItems.emplace_back(Key, Entry.getValue().Value);
+ } else {
+ LocalItems[Key.take_front(Idx)].emplace_back(Key.drop_front(Idx + 1),
+ Entry.getValue().Value);
+ }
+ }
+ llvm::sort(GlobalItems,
+ [](auto &Lhs, auto &Rhs) { return Lhs.first < Rhs.first; });
+ for (auto &Item : GlobalItems) {
+ IO.preflightKey(Item.first.data(), true, false, UseDefault, SaveInfo);
+ StringRef S = Item.second;
IO.scalarString(S, needsQuotes(S));
IO.postflightKey(SaveInfo);
}
+ for (auto &Item : LocalItems) {
+ // If a check only has 1 option, Emit it directly as a scalar.
+ if (Item.second.size() == 1) {
+ // This is a bit of a hack as Item.first only contains the check name,
+ // but the null terminated buffer it references will contain the whole
+ // option name.
+ IO.preflightKey(Item.first.data(), true, false, UseDefault, SaveInfo);
+ StringRef S = Item.second.front().second;
+ IO.scalarString(S, needsQuotes(S));
+ IO.postflightKey(SaveInfo);
+ continue;
+ }
+ // Sort all of the checks options for nicer output
+ llvm::sort(Item.second,
+ [](auto &Lhs, auto &Rhs) { return Lhs.first < Rhs.first; });
+ // The stringref in Item.first is not null terminated, so we need to copy
+ // to a null terminated buffer
+ IO.preflightKey(ToCString(Item.first), true, false, UseDefault, SaveInfo);
+ IO.beginMapping();
+ for (auto &SubItem : Item.second) {
+ IO.preflightKey(SubItem.first.data(), true, false, UseDefault2,
+ SaveInfo2);
+ StringRef S = SubItem.second;
+ IO.scalarString(S, needsQuotes(S));
+ IO.postflightKey(SaveInfo2);
+ }
+ IO.endMapping();
+ IO.postflightKey(SaveInfo);
+ }
IO.endMapping();
} else {
// We need custom logic here to support the old method of specifying check
@@ -107,8 +162,27 @@
yamlize(IO, NOpts->Options, true, Ctx);
} else if (isa<MappingNode>(I.getCurrentNode())) {
IO.beginMapping();
+ llvm::SmallString<128> Buffer;
for (StringRef Key : IO.keys()) {
- IO.mapRequired(Key.data(), Options[Key].Value);
+ IO.preflightKey(Key.data(), true, false, UseDefault, SaveInfo);
+ if (isa<ScalarNode, BlockScalarNode>(I.getCurrentNode())) {
+ yamlize(IO, Options[Key].Value, true, Ctx);
+ } else if (isa<MappingNode>(I.getCurrentNode())) {
+ IO.beginMapping();
+ Buffer.assign({Key, "."});
+ auto Size = Buffer.size();
+ for (StringRef Key : IO.keys()) {
+ IO.preflightKey(Key.data(), true, false, UseDefault2, SaveInfo2);
+ Buffer.truncate(Size);
+ Buffer.append(Key);
+ yamlize(I, Options[Buffer].Value, true, Ctx);
+ IO.postflightKey(SaveInfo2);
+ }
+ IO.endMapping();
+ } else {
+ IO.setError("Expected a scalar or a map");
+ }
+ IO.postflightKey(SaveInfo);
}
IO.endMapping();
} else {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
