[clang-tidy] Improve alternate snake case warnings #71385

jcmoyer · 2023-11-06T12:24:24Z

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:

// 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:

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

llvmbot · 2023-11-06T12:24:52Z

@llvm/pr-subscribers-clang-tidy

Author: J.C. Moyer (jcmoyer)

Changes

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:

// 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:

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

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

PiotrZSL

LGTM

PiotrZSL · 2023-11-06T15:43:38Z

Also update release notes, add one sentence there about this.

jcmoyer · 2023-11-06T16:11:15Z

Also update release notes, add one sentence there about this.

Done.

jcmoyer · 2023-11-11T11:34:39Z

Rebased to resolve merge conflict.

PiotrZSL · 2023-11-17T21:40:23Z

@jcmoyer Do you want this be merged as jcmoyer@users.noreply.github.com ? If not then please disable privacy settings in GitHub or change author of commit.

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.

jcmoyer · 2023-11-18T12:07:45Z

@PiotrZSL I don't mind but I changed it just in case it's an issue.

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.

llvmbot added the clang-tidy label Nov 6, 2023

EugeneZelenko requested a review from PiotrZSL November 6, 2023 15:19

PiotrZSL reviewed Nov 6, 2023

View reviewed changes

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp Show resolved Hide resolved

PiotrZSL approved these changes Nov 6, 2023

View reviewed changes

PiotrZSL merged commit f9974f7 into llvm:main Nov 18, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang-tidy] Improve alternate snake case warnings #71385

[clang-tidy] Improve alternate snake case warnings #71385

jcmoyer commented Nov 6, 2023

llvmbot commented Nov 6, 2023

PiotrZSL left a comment

PiotrZSL commented Nov 6, 2023

jcmoyer commented Nov 6, 2023

jcmoyer commented Nov 11, 2023

PiotrZSL commented Nov 17, 2023

jcmoyer commented Nov 18, 2023

[clang-tidy] Improve alternate snake case warnings #71385

[clang-tidy] Improve alternate snake case warnings #71385

Conversation

jcmoyer commented Nov 6, 2023

llvmbot commented Nov 6, 2023

PiotrZSL left a comment

Choose a reason for hiding this comment

PiotrZSL commented Nov 6, 2023

jcmoyer commented Nov 6, 2023

jcmoyer commented Nov 11, 2023

PiotrZSL commented Nov 17, 2023

jcmoyer commented Nov 18, 2023