[clang-tidy] Fix false positive in readability-identifier-naming check involving override attribute
Overriding methods should not get a readability-identifier-naming
warning because the issue can only be fixed in the base class; but the
current check for whether a method is overriding does not take the
override attribute into account.
Differential Revision: https://reviews.llvm.org/D113830
GitOrigin-RevId: 6259016361345e09f0607ef4e037e00bcbe4bd40
diff --git a/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tidy/readability/IdentifierNamingCheck.cpp
index d275b47..cfbe79c 100644
--- a/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
if (const auto *Decl = dyn_cast<CXXMethodDecl>(D)) {
if (Decl->isMain() || !Decl->isUserProvided() ||
- Decl->size_overridden_methods() > 0)
+ Decl->size_overridden_methods() > 0 || Decl->hasAttr<OverrideAttr>())
return SK_Invalid;
// If this method has the same name as any base method, this is likely
diff --git a/test/clang-tidy/checkers/readability-identifier-naming.cpp b/test/clang-tidy/checkers/readability-identifier-naming.cpp
index 3622fe5..01bcb34 100644
--- a/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ b/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -285,6 +285,10 @@
virtual void BadBaseMethod() = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
// CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
+
+ virtual void BadBaseMethodNoAttr() = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method_No_Attr() = 0;
};
class COverriding : public AOverridden {
@@ -293,6 +297,9 @@
void BadBaseMethod() override {}
// CHECK-FIXES: {{^}} void v_Bad_Base_Method() override {}
+ void BadBaseMethodNoAttr() /* override */ {}
+ // CHECK-FIXES: {{^}} void v_Bad_Base_Method_No_Attr() /* override */ {}
+
void foo() {
BadBaseMethod();
// CHECK-FIXES: {{^}} v_Bad_Base_Method();
@@ -302,12 +309,79 @@
// CHECK-FIXES: {{^}} AOverridden::v_Bad_Base_Method();
COverriding::BadBaseMethod();
// CHECK-FIXES: {{^}} COverriding::v_Bad_Base_Method();
+
+ BadBaseMethodNoAttr();
+ // CHECK-FIXES: {{^}} v_Bad_Base_Method_No_Attr();
+ this->BadBaseMethodNoAttr();
+ // CHECK-FIXES: {{^}} this->v_Bad_Base_Method_No_Attr();
+ AOverridden::BadBaseMethodNoAttr();
+ // CHECK-FIXES: {{^}} AOverridden::v_Bad_Base_Method_No_Attr();
+ COverriding::BadBaseMethodNoAttr();
+ // CHECK-FIXES: {{^}} COverriding::v_Bad_Base_Method_No_Attr();
}
};
-void VirtualCall(AOverridden &a_vItem) {
+// Same test as above, now with a dependent base class.
+template<typename some_t>
+class ATOverridden {
+public:
+ virtual void BadBaseMethod() = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
+
+ virtual void BadBaseMethodNoAttr() = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method_No_Attr() = 0;
+};
+
+template<typename some_t>
+class CTOverriding : public ATOverridden<some_t> {
+ // Overriding a badly-named base isn't a new violation.
+ // FIXME: The fixes from the base class should be propagated to the derived class here
+ // (note that there could be specializations of the template base class, though)
+ void BadBaseMethod() override {}
+
+ // Without the "override" attribute, and due to the dependent base class, it is not
+ // known whether this method overrides anything, so we get the warning here.
+ virtual void BadBaseMethodNoAttr() {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method_No_Attr() {};
+};
+
+template<typename some_t>
+void VirtualCall(AOverridden &a_vItem, ATOverridden<some_t> &a_vTitem) {
a_vItem.BadBaseMethod();
// CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+
+ // FIXME: The fixes from ATOverridden should be propagated to the following call
+ a_vTitem.BadBaseMethod();
+}
+
+// Same test as above, now with a dependent base class that is instantiated below.
+template<typename some_t>
+class ATIOverridden {
+public:
+ virtual void BadBaseMethod() = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+ // CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
+};
+
+template<typename some_t>
+class CTIOverriding : public ATIOverridden<some_t> {
+public:
+ // Overriding a badly-named base isn't a new violation.
+ void BadBaseMethod() override {}
+ // CHECK-FIXES: {{^}} void v_Bad_Base_Method() override {}
+};
+
+template class CTIOverriding<int>;
+
+void VirtualCallI(ATIOverridden<int>& a_vItem, CTIOverriding<int>& a_vCitem) {
+ a_vItem.BadBaseMethod();
+ // CHECK-FIXES: {{^}} a_vItem.v_Bad_Base_Method();
+
+ a_vCitem.BadBaseMethod();
+ // CHECK-FIXES: {{^}} a_vCitem.v_Bad_Base_Method();
}
template <typename derived_t>
