[clang-tidy] readability-identifier-naming shouldn't complain about CRTP pseudo-overrides

Reviewers: ilya-biryukov

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

llvm-svn: 370193

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

@@ -10,6 +10,7 @@
 
 #include "../utils/ASTUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/AST/CXXInheritance.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
@@ -579,6 +580,15 @@ static StyleKind findStyleKind(
         Decl->size_overridden_methods() > 0)
       return SK_Invalid;
 
+    // If this method has the same name as any base method, this is likely
+    // necessary even if it's not an override. e.g. CRTP.
+    auto FindHidden = [&](const CXXBaseSpecifier *S, clang::CXXBasePath &P) {
+      return CXXRecordDecl::FindOrdinaryMember(S, P, Decl->getDeclName());
+    };
+    CXXBasePaths UnusedPaths;
+    if (Decl->getParent()->lookupInBases(FindHidden, UnusedPaths))
+      return SK_Invalid;
+
     if (Decl->isConstexpr() && NamingStyles[SK_ConstexprMethod])
       return SK_ConstexprMethod;
 

clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

@@ -26,6 +26,10 @@ Many configuration options are available, in order to be able to create
 different rules for different kinds of identifiers. In general, the rules are
 falling back to a more generic rule if the specific case is not configured.
 
+The naming of virtual methods is reported where they occur in the base class,
+but not where they are overridden, as it can't be fixed locally there.
+This also applies for pseudo-override patterns like CRTP.
+
 Options
 -------
 

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

@@ -262,6 +262,32 @@ void InstantiateClassMethods() {
   CMyWellNamedClass2<int> x5(42, nullptr);
 }
 
+class AOverridden {
+public:
+  virtual ~AOverridden() = default;
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+};
+
+class COverriding : public AOverridden {
+public:
+  // Overriding a badly-named base isn't a new violation.
+  void BadBaseMethod() override {}
+};
+
+template <typename derived_t>
+class CRTPBase {
+public:
+  void BadBaseMethod(int) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'BadBaseMethod'
+};
+
+class CRTPDerived : CRTPBase<CRTPDerived> {
+public:
+  // Hiding a badly-named base isn't a new violation.
+  double BadBaseMethod(double) { return 0; }
+};
+
 template<typename T>
 // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for type template parameter 'T'
 // CHECK-FIXES: {{^}}template<typename t_t>{{$}}