-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy] Fix support for typedefs in readability-identifier-naming #66835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-tidy] Fix support for typedefs in readability-identifier-naming #66835
Conversation
Typedef rename were not properly handled when typedef were used behind pointer, or as a part of function type. Additionally because entire function were passed as an source-range, when function started with macro, such change were not marked for a fix. Removed workaround and used proper TypedefTypeLoc instead.
@llvm/pr-subscribers-clang-tidy ChangesTypedef rename were not properly handled when typedef were used behind pointer, or as a part of function type. Additionally because entire function were passed as an source-range, when function started with macro, such change were not marked for a fix. Removed workaround and used proper TypedefTypeLoc instead. Fixes #55156 Full diff: https://github.com/llvm/llvm-project/pull/66835.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index d24b7a65b1c4334..da1433aa2d05d47 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -256,26 +256,6 @@ class RenamerClangTidyVisitor
return true;
}
- // Fix type aliases in value declarations.
- if (const auto *Value = dyn_cast<ValueDecl>(Decl)) {
- if (const Type *TypePtr = Value->getType().getTypePtrOrNull()) {
- if (const auto *Typedef = TypePtr->getAs<TypedefType>())
- Check->addUsage(Typedef->getDecl(), Value->getSourceRange(), SM);
- }
- }
-
- // Fix type aliases in function declarations.
- if (const auto *Value = dyn_cast<FunctionDecl>(Decl)) {
- if (const auto *Typedef =
- Value->getReturnType().getTypePtr()->getAs<TypedefType>())
- Check->addUsage(Typedef->getDecl(), Value->getSourceRange(), SM);
- for (const ParmVarDecl *Param : Value->parameters()) {
- if (const TypedefType *Typedef =
- Param->getType().getTypePtr()->getAs<TypedefType>())
- Check->addUsage(Typedef->getDecl(), Value->getSourceRange(), SM);
- }
- }
-
// Fix overridden methods
if (const auto *Method = dyn_cast<CXXMethodDecl>(Decl)) {
if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
@@ -340,6 +320,11 @@ class RenamerClangTidyVisitor
return true;
}
+ bool VisitTypedefTypeLoc(const TypedefTypeLoc &Loc) {
+ Check->addUsage(Loc.getTypedefNameDecl(), Loc.getSourceRange(), SM);
+ return true;
+ }
+
bool VisitTagTypeLoc(const TagTypeLoc &Loc) {
Check->addUsage(Loc.getDecl(), Loc.getSourceRange(), SM);
return true;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d6f51998a01e57..b8977a39239ffce 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -291,7 +291,9 @@ Changes in existing checks
warnings when a type's forward declaration precedes its definition.
Additionally, it now provides appropriate warnings for ``struct`` and
``union`` in C, while also incorporating support for the
- ``Leading_upper_snake_case`` naming convention.
+ ``Leading_upper_snake_case`` naming convention. The handling of ``typedef``
+ has been enhanced, particularly within complex types like function pointers
+ and cases where style checks were omitted when functions started with macros.
- Improved :doc:`readability-implicit-bool-conversion
<clang-tidy/checks/readability/implicit-bool-conversion>` check to take
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 3445a21bfdbc4aa..0f5fc1145f8791e 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
@@ -729,3 +729,21 @@ struct forward_declared_as_struct;
class forward_declared_as_struct {
};
+namespace pr55156 {
+
+typedef enum {
+ VALUE0,
+ VALUE1,
+} ValueType;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for typedef 'ValueType' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}} value_type_t;
+
+#define STATIC_MACRO static
+STATIC_MACRO void someFunc(ValueType a_v1, const ValueType& a_v2) {}
+// CHECK-FIXES: {{^}}STATIC_MACRO void someFunc(value_type_t a_v1, const value_type_t& a_v2) {}
+STATIC_MACRO void someFunc(const ValueType** p_a_v1, ValueType (*p_a_v2)()) {}
+// CHECK-FIXES: {{^}}STATIC_MACRO void someFunc(const value_type_t** p_a_v1, value_type_t (*p_a_v2)()) {}
+STATIC_MACRO ValueType someFunc() {}
+// CHECK-FIXES: {{^}}STATIC_MACRO value_type_t someFunc() {}
+#undef STATIC_MACRO
+}
|
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fix and the great cleanup!
Typedef rename were not properly handled when typedef were used behind pointer, or as a part of function type. Additionally because entire function were passed as an source-range, when function started with macro, such change were not marked for a fix.
Removed workaround and used proper TypedefTypeLoc instead.
Fixes #55156, #54699