[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore requested changes to this revision.
stephanemoore marked an inline comment as done.
stephanemoore added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:30
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WhitelistedPrefixes", WhitelistedPrefixes);
+}
----------------
aaron.ballman wrote:
> dgatwood wrote:
> > aaron.ballman wrote:
> > > dgatwood wrote:
> > > > aaron.ballman wrote:
> > > > > `ExpectedPrefixes` here as well.
> > > > > 
> > > > > Should there be a default list of these?
> > > > Done.  And no, there should be no default, unless somehow Xcode's project prefix makes it down as far as LLVM, in which case //maybe// that could be the default.
> > > > 
> > > > The idea is that you can whitelist your own Xcode project's prefix, along with the prefixes of your own in-house libraries, so that each individual team/workgroup can add categories on their own classes, but will get warned when they try to add unprefixed category methods on classes that they don't own (e.g. classes in system frameworks, third-party frameworks, etc.).
> > > Still wondering whether we should have a default list of expected prefixes or not.
> > This is weird.  I don't know why this comment system didn't submit my comment before.
> > 
> > No, there should be no default, unless somehow Xcode's project prefix makes it down as far as LLVM, in which case maybe that could be the default.
> > 
> > The idea is that you can whitelist your own Xcode project's prefix, along with the prefixes of your own in-house libraries, so that each individual team/workgroup can add categories on their own classes, but will get warned when they try to add unprefixed category methods on classes that they don't own (e.g. classes in system frameworks, third-party frameworks, etc.).
> Ah, thank you for the explanation!
Agreed that there should be no default.


================
Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:22-23
+
+  Finder->addMatcher(objcMethodDecl().bind(CustomCategoryMethodIdentifier),
+      this);
+}
----------------
What do you think of `objcMethodDecl(hasDeclContext(objcCategoryDecl()))`? I think that more narrowly targets the method declarations that we are interested in. There are other method declarations that we like to prefix but I consider those outside the domain of category method name prefixing practices.


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

https://reviews.llvm.org/D65917