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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
