[clang-tools-extra] 866dc09 - [clang-tidy] Better support for Override function in RenamerClangTidy based checks

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-10-19T15:21:06+01:00
New Revision: 866dc09784495bd2c204945144cd31cd8e653f0e

URL: https://github.com/llvm/llvm-project/commit/866dc09784495bd2c204945144cd31cd8e653f0e
DIFF: https://github.com/llvm/llvm-project/commit/866dc09784495bd2c204945144cd31cd8e653f0e.diff

LOG: [clang-tidy] Better support for Override function in RenamerClangTidy based checks

Methods that override virtual methods will now get renamed if the initial virtual method has a name violation.
Addresses https://bugs.llvm.org/show_bug.cgi?id=34879

Reviewed By: alexfh

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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.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/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 2d67ca4a1618..bb8caf1d84e2 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -135,6 +135,23 @@ void RenamerClangTidyCheck::registerPPCallbacks(
                                                          this));
 }
 
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+  if (Method->size_overridden_methods() != 1)
+    return nullptr;
+  while (true) {
+    Method = *Method->begin_overridden_methods();
+    assert(Method && "Overridden method shouldn't be null");
+    unsigned NumOverrides = Method->size_overridden_methods();
+    if (NumOverrides == 0)
+      return Method;
+    if (NumOverrides > 1)
+      return nullptr;
+  }
+}
+
 void RenamerClangTidyCheck::addUsage(
     const RenamerClangTidyCheck::NamingCheckId &Decl, SourceRange Range,
     SourceManager *SourceMgr) {
@@ -172,6 +189,10 @@ void RenamerClangTidyCheck::addUsage(
 
 void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
                                      SourceManager *SourceMgr) {
+  if (const auto *Method = dyn_cast<CXXMethodDecl>(Decl)) {
+    if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+      Decl = Overridden;
+  }
   Decl = cast<NamedDecl>(Decl->getCanonicalDecl());
   return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
                                                        Decl->getNameAsString()),
@@ -412,6 +433,14 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) {
       }
     }
 
+    // Fix overridden methods
+    if (const auto *Method = Result.Nodes.getNodeAs<CXXMethodDecl>("decl")) {
+      if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
+        addUsage(Overridden, Method->getLocation());
+        return; // Don't try to add the actual decl as a Failure.
+      }
+    }
+
     // Ignore ClassTemplateSpecializationDecl which are creating duplicate
     // replacements with CXXRecordDecl.
     if (isa<ClassTemplateSpecializationDecl>(Decl))

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6e6d46df4933..595b1c465d5f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,9 @@ Changes in existing checks
   Added an option `GetConfigPerFile` to support including files which use
   
diff erent naming styles.
 
+  Now renames overridden virtual methods if the method they override has a
+  style violation.
+
 - Removed `google-runtime-references` check because the rule it checks does
   not exist in the Google Style Guide anymore.
 

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 fed362bbecde..de53dddc0f92 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
@@ -266,14 +266,32 @@ class AOverridden {
   virtual ~AOverridden() = default;
   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;
 };
 
 class COverriding : public AOverridden {
 public:
   // Overriding a badly-named base isn't a new violation.
   void BadBaseMethod() override {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  
+  void foo() {
+    BadBaseMethod();
+    // CHECK-FIXES: {{^}}    v_Bad_Base_Method();
+    this->BadBaseMethod();
+    // CHECK-FIXES: {{^}}    this->v_Bad_Base_Method();
+    AOverridden::BadBaseMethod();
+    // CHECK-FIXES: {{^}}    AOverridden::v_Bad_Base_Method();
+    COverriding::BadBaseMethod();
+    // CHECK-FIXES: {{^}}    COverriding::v_Bad_Base_Method();
+  }
 };
 
+void VirtualCall(AOverridden &a_vItem) {
+  a_vItem.BadBaseMethod();
+  // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+}
+
 template <typename derived_t>
 class CRTPBase {
 public: