readability-identifier-naming not ignored when overriding member function of class template

[llvmbot]

Bugzilla Link	45815
Resolution	FIXED
Resolved on	Nov 19, 2021 12:54
Version	unspecified
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@fabianwolff,@salman-javed-nz

Extended Description

The following code should not trigger readability-identifier-naming for Derived::LOL():

template class Base { public: virtual void LOL() = 0; };

template class Derived : public Base { void LOL() override{}; };

using e.g. this config file.

---

Checks: 'readability-identifier-naming' CheckOptions:

• key: readability-identifier-naming.FunctionCase value: 'lower_case' ...

[FabianWolff]

Fixed in https://reviews.llvm.org/rG6259016361345e09f0607ef4e037e00bcbe4bd40.

[llvmbot]

Hmmm, on looking there are actually more issues with this than mentioned here. First off what bug you are mentioning is because the check doesn't look for the override attribute, instead it looks for overridden methods. However as it can't see the definition of the LOL from a template dependent base class it (incorrectly) assumes its not an override.

However the bigger issue is even without the template it doesn't propagate the fixes down to derived classes that override the method

class Base { virtual void LOL() = 0; };

class Derived : public Base { void LOL() override {} };

---

MainSourceFile: '/home/ce/' Diagnostics:

• DiagnosticName: readability-identifier-naming DiagnosticMessage: Message: 'invalid case style for function ''Func''' FilePath: '/home/ce/example.cpp' FileOffset: 53 Replacements:
  • FilePath: '/home/ce/example.cpp' Offset: 53 Length: 4 ReplacementText: func ...

[llvmbot]

Well, not according to the description of readability-identifier-naming (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. This also applies for pseudo-override patterns like CRTP."

Remove the template stuff and things work as expected.

[llvmbot]

It most definitely should trigger for the Derived<T>::LOL() as its a function call that has the incorrect case.