[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz accepted this revision.
salman-javed-nz added a comment.
This revision is now accepted and ready to land.

LGTM. Nothing more to suggest from my side. Can we allow a few days for the other reviewers to put in their 2c.

As for the Bugzilla ticket <https://bugs.llvm.org/show_bug.cgi?id=45815>, I'd say you've done enough to satisfy the primary grievance expressed in the PR: warnings should be raised in the base class only.
This is in line with the check's state purpose in the documentation <https://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html>.

> 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.

The comments below the PR talk about the need to propagate naming fixes to the derived classes and call sites. The last comment shows an example where fixes are not propagated at all, not even for the simple (non-template) case. But your tests for 
`COverriding` and `CTIOverriding` show that the replacements are indeed working. In any case, applying fixes outside the base class is going above and beyond the check's scope, and as you say, could lead to incorrect suggestions if the code is even slightly less trivial. More work could be done on this class, but doesn't have to be in this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113830/new/

https://reviews.llvm.org/D113830