[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Tue Apr 10 08:40:24 PDT 2018

alexfh added a comment.

In https://reviews.llvm.org/D45392#1061960, @Wizard wrote:

> In https://reviews.llvm.org/D45392#1061433, @alexfh wrote:
>
> > I wonder whether the readability-identifier-naming check could be extended to support this use case instead of adding a new check specifically for underscores in ivar names?
>
>
> Hmm readability-identifier-naming is a C++ check but this one is only for ObjC. I prefer putting them in separate places unless they work for both languages.

I see no reasons why this check can't work for ObjC, if the handling of ObjC-specific AST nodes is added to it.

> Moreover, readability-identifier-naming always runs all matchers and apply the same checks on all identifiers.

It has some sort of a hierarchical structure of rules that allow it to only touch a certain subset of identifiers. E.g. configure different naming styles for local variables and local constants. What it's lacking is a good documentation for all of these options =\

> We have to change at least part of the structure to make it applicable for this check.

Yes, the existing check may need some modifications to do what this check needs to do, but I'd expect these modifications to be quite small, and we would potentially get a much more useful and generic tool instead of "one check per type of named entity per naming style" situation.

> And readability-identifier-naming is not in google default clang-tidy check list yet. We will even need more work to import it.

IIUC, it can be configured to only verify certain kinds of named entities. Thus there's no requirement to make it work with every aspect of our naming rules.

> So I think creating a new simple ObjC-specific check is a better way here.

I'll respectfully disagree here. I would prefer to have a generic solution, if it's feasible. And so far, it looks like it is.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45392