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

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-08-06T09:22:31Z
New Revision: 2111577d4dfc1d48b4e2221f84597fa4f54b8cfd

URL: https://github.com/llvm/llvm-project/commit/2111577d4dfc1d48b4e2221f84597fa4f54b8cfd
DIFF: https://github.com/llvm/llvm-project/commit/2111577d4dfc1d48b4e2221f84597fa4f54b8cfd.diff

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

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

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D157190

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 0e16edc10a9530..d240d8d244ef91 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1140,30 +1140,28 @@ StyleKind IdentifierNamingCheck::findStyleKind(
     if (Decl->isAnonymousStructOrUnion())
       return SK_Invalid;
 
-    if (!Decl->getCanonicalDecl()->isThisDeclarationADefinition())
-      return SK_Invalid;
-
-    if (Decl->hasDefinition() && Decl->isAbstract() &&
-        NamingStyles[SK_AbstractClass])
-      return SK_AbstractClass;
+    if (const auto *Definition = Decl->getDefinition()) {
+      if (Definition->isAbstract() && NamingStyles[SK_AbstractClass])
+        return SK_AbstractClass;
 
-    if (Decl->isStruct() && NamingStyles[SK_Struct])
-      return SK_Struct;
+      if (Definition->isStruct() && NamingStyles[SK_Struct])
+        return SK_Struct;
 
-    if (Decl->isStruct() && NamingStyles[SK_Class])
-      return SK_Class;
+      if (Definition->isStruct() && NamingStyles[SK_Class])
+        return SK_Class;
 
-    if (Decl->isClass() && NamingStyles[SK_Class])
-      return SK_Class;
+      if (Definition->isClass() && NamingStyles[SK_Class])
+        return SK_Class;
 
-    if (Decl->isClass() && NamingStyles[SK_Struct])
-      return SK_Struct;
+      if (Definition->isClass() && NamingStyles[SK_Struct])
+        return SK_Struct;
 
-    if (Decl->isUnion() && NamingStyles[SK_Union])
-      return SK_Union;
+      if (Definition->isUnion() && NamingStyles[SK_Union])
+        return SK_Union;
 
-    if (Decl->isEnum() && NamingStyles[SK_Enum])
-      return SK_Enum;
+      if (Definition->isEnum() && NamingStyles[SK_Enum])
+        return SK_Enum;
+    }
 
     return SK_Invalid;
   }

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1bebc55eec7d08..1d006ae3fa16f5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -180,6 +180,10 @@ Changes in existing checks
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``.
 
+- 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
 ^^^^^^^^^^^^^^
 

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 b506b4bbe01034..a017fc51d4cbf4 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
@@ -707,3 +707,19 @@ struct async_obj {
 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 {
+};
+