llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: J.C. Moyer (jcmoyer) <details> <summary>Changes</summary> Improves the accuracy of `readability-identifier-naming` for cases `Camel_Snake_Case` and `camel_Snake_Back`. Prior to this commit, these cases matched identifiers with **only** a leading upper case letter or leading lower case letter respectively. Now, uppercase letters can only appear at the start of an identifier or directly following an underscore. --- Currently, the regex for `Camel_Snake_Case` matches any identifier that starts with a capital letter: ``` ^[A-Z]([a-z0-9]*(_[A-Z])?)* ^^^^^^^^^-- underscore + capital letter after the first capital is optional ``` This means that `Camel_Snake_Case` matches other cases - in particular `CamelCase` and `Leading_upper_snake_case` - which causes clang-tidy to sometimes not flag incorrect casing. It also matches `UPPER_CASE`, but I think it's reasonable to consider this a subset of `Camel_Snake_Case` since some users may prefer e.g. `XML_Parser` to `Xml_Parser`. It's really easy to accidentally type an identifier that clang-tidy doesn't catch; all you have to do is omit an underscore or forget to capitalize a letter. The same problem also applies to `camel_Snake_Back` except that any identifier starting with a lower case letter matches, so I went ahead and adjusted its regex too. Fixing it also uncovered a minor error in an existing test. Here are some examples of the current behavior when using `Camel_Snake_Case`: ```cpp // accepted by clang-tidy String_Reader StringReader // bad, CamelCase String_reader // bad, Leading_upper_snake_case Xml_Parser_2 XML_Parser_2 XML_PaRser_2 // bad? mixed case in second word T Foo A_B_C A_b_c // bad, Leading_upper_snake_case // rejected by clang-tidy t foo a_b_c ``` After this patch: ```cpp // accepted by clang-tidy String_Reader Xml_Parser_2 XML_Parser_2 T Foo A_B_C // rejected by clang-tidy StringReader String_reader XML_PaRser_2 t foo A_b_c a_b_c ``` --- Full diff: https://github.com/llvm/llvm-project/pull/71385.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp (+2-2) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-match.cpp (+60) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp (+2-1) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 7539b3899682e13..793b86cadd72501 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -871,8 +871,8 @@ bool IdentifierNamingCheck::matchesStyle( llvm::Regex("^[a-z][a-zA-Z0-9]*$"), llvm::Regex("^[A-Z][A-Z0-9_]*$"), llvm::Regex("^[A-Z][a-zA-Z0-9]*$"), - llvm::Regex("^[A-Z]([a-z0-9]*(_[A-Z])?)*"), - llvm::Regex("^[a-z]([a-z0-9]*(_[A-Z])?)*"), + llvm::Regex("^[A-Z]+([a-z0-9]*_[A-Z0-9]+)*[a-z0-9]*$"), + llvm::Regex("^[a-z]+([a-z0-9]*_[A-Z0-9]+)*[a-z0-9]*$"), llvm::Regex("^[A-Z]([a-z0-9_]*[a-z])*$"), }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-match.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-match.cpp new file mode 100644 index 000000000000000..f692b01923455e8 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-match.cpp @@ -0,0 +1,60 @@ +// RUN: %check_clang_tidy -std=c++20 %s readability-identifier-naming %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-identifier-naming.ClassCase: Camel_Snake_Case, \ +// RUN: readability-identifier-naming.StructCase: camel_Snake_Back, \ +// RUN: }}' + +// clang-format off + +//===----------------------------------------------------------------------===// +// Camel_Snake_Case tests +//===----------------------------------------------------------------------===// +class XML_Parser {}; +class Xml_Parser {}; +class XML_Parser_2 {}; +// NO warnings or fixes expected as these identifiers are Camel_Snake_Case + +class XmlParser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'XmlParser' +// CHECK-FIXES: {{^}}class Xml_Parser {};{{$}} + +class Xml_parser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'Xml_parser' +// CHECK-FIXES: {{^}}class Xml_Parser {};{{$}} + +class xml_parser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'xml_parser' +// CHECK-FIXES: {{^}}class Xml_Parser {};{{$}} + +class xml_Parser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'xml_Parser' +// CHECK-FIXES: {{^}}class Xml_Parser {};{{$}} + +class xml_Parser_2 {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'xml_Parser_2' +// CHECK-FIXES: {{^}}class Xml_Parser_2 {};{{$}} + +class t {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 't' +// CHECK-FIXES: {{^}}class T {};{{$}} + +//===----------------------------------------------------------------------===// +// camel_Snake_Back tests +//===----------------------------------------------------------------------===// +struct json_Parser {}; +struct json_Parser_2 {}; +struct u {}; +// NO warnings or fixes expected as these identifiers are camel_Snake_Back + +struct JsonParser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'JsonParser' +// CHECK-FIXES: {{^}}struct json_Parser {};{{$}} + +struct Json_parser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'Json_parser' +// CHECK-FIXES: {{^}}struct json_Parser {};{{$}} + +struct json_parser {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'json_parser' +// CHECK-FIXES: {{^}}struct json_Parser {};{{$}} + diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp index 84bf7764583e801..5e13d141ca0140c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp @@ -413,7 +413,8 @@ class my_other_templated_class : my_templated_class< my_class>, private my_deri template<typename t_t> using mysuper_tpl_t = my_other_templated_class <:: FOO_NS ::my_class>; -// CHECK-FIXES: {{^}}using mysuper_tpl_t = CMyOtherTemplatedClass <:: foo_ns ::CMyClass>;{{$}} +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'mysuper_tpl_t' +// CHECK-FIXES: {{^}}using mysuper_Tpl_t = CMyOtherTemplatedClass <:: foo_ns ::CMyClass>;{{$}} const int global_Constant = 6; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global constant 'global_Constant' `````````` </details> https://github.com/llvm/llvm-project/pull/71385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits