[clang-tidy] Fixed false-negative in readability-identifier-naming

[PiotrZSL]

Summary

Issue were cased by checking if canonical declaration of record is a
definition. Unfortunately when forward declarations were used, it were
not a definition.

Fixes: #64463

[PiotrZSL]

Properly handle situation when class is forward-declared as struct.

[carlosgalvezp]

LGTM, minor question that can be fixed post-review unless you want to discuss further!

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp

716

Nit: shouldn't we use also struct here?

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp

716

No, thats on prupose. I wanted to test also in same place that style is taken from definition, not forward declaration, can add some comment, and "public:"

[PiotrZSL]

Rebase + make tests more simple

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

if (const auto *EnumConst = dyn_cast<EnumConstantDecl>(D)) {
 
     return SK_Invalid;
   }
 
   if (const auto *Decl = dyn_cast<CXXRecordDecl>(D)) {
     if (Decl->isAnonymousStructOrUnion())
       return SK_Invalid;
 
-    if (!Decl->getCanonicalDecl()->isThisDeclarationADefinition())
-      return SK_Invalid;
-
-    if (Decl->hasDefinition() && Decl->isAbstract() &&
-        NamingStyles[SK_AbstractClass])
+    if (const auto *Definition = Decl->getDefinition()) {
+      if (Definition->isAbstract() && NamingStyles[SK_AbstractClass])
         return SK_AbstractClass;
 
-    if (Decl->isStruct() && NamingStyles[SK_Struct])
+      if (Definition->isStruct() && NamingStyles[SK_Struct])
         return SK_Struct;
 
-    if (Decl->isStruct() && NamingStyles[SK_Class])
+      if (Definition->isStruct() && NamingStyles[SK_Class])
         return SK_Class;
 
-    if (Decl->isClass() && NamingStyles[SK_Class])
+      if (Definition->isClass() && NamingStyles[SK_Class])
         return SK_Class;
 
-    if (Decl->isClass() && NamingStyles[SK_Struct])
+      if (Definition->isClass() && NamingStyles[SK_Struct])
         return SK_Struct;
 
-    if (Decl->isUnion() && NamingStyles[SK_Union])
+      if (Definition->isUnion() && NamingStyles[SK_Union])
         return SK_Union;
 
-    if (Decl->isEnum() && NamingStyles[SK_Enum])
+      if (Definition->isEnum() && NamingStyles[SK_Enum])
         return SK_Enum;
+    }
 
     return SK_Invalid;
   }
 
   if (const auto *Decl = dyn_cast<FieldDecl>(D)) {
     QualType Type = Decl->getType();
 
     if (!Type.isNull() && Type.isConstQualified()) {

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
   to ignore ``static`` variables declared within the scope of
   ``class``/``struct``.
 
 - Improved :doc:`llvm-namespace-comment
   <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
 
+- Improved the :doc:`readability-identifier-naming
+  <clang-tidy/checks/readability/identifier-naming>` check to emit proper
+  warnings when a type forward declaration precedes its definition.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp

 struct async_obj {
 public:
   never_suspend operator co_await() const noexcept;
 };
 
 task ImplicitDeclTest(async_obj &a_object) {
   co_await a_object;  // CHECK-MESSAGES-NOT: warning: invalid case style for local variable
 }
+
+// Test scenario when canonical declaration will be a forward declaration
+struct ForwardDeclStruct;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'ForwardDeclStruct' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}struct forward_decl_struct;
+// CHECK-FIXES: {{^}}struct forward_decl_struct {
+struct ForwardDeclStruct {
+};
+
+struct forward_declared_as_struct;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for class 'forward_declared_as_struct' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}struct CForwardDeclaredAsStruct;
+// CHECK-FIXES: {{^}}class CForwardDeclaredAsStruct {
+class forward_declared_as_struct {
+};
+