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

Stephane Moore via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 16 19:08:50 PDT 2019


stephanemoore requested changes to this revision.
stephanemoore added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:23
+  Finder->addMatcher(objcMethodDecl(hasDeclContext(
+      objcCategoryDecl())).bind(CustomCategoryMethodIdentifier), this);
+}
----------------
Technically category method prefixing is only strictly enforced on classes that are shared:
"If a class is not shared with other projects, categories extending it may omit name prefixes and method name prefixes."
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#category-naming

With that in mind, perhaps we should match on categories on classes that extend classes that are declared in system headers? I think you can accomplish that by adding a custom inner matcher in `objcCategoryDecl` which pulls out the Objective-C interface using [clang::ObjCCategoryDecl::getClassInterface](https://clang.llvm.org/doxygen/classclang_1_1ObjCCategoryDecl.html#acdb14eeca277cfa745a4e8e842312008) and then add `isExpansionInSystemHeader` as an inner matcher on the custom inner matcher. WDYT? Let me know if you need help putting together.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:41
+
+.. option:: ExpectedPrefixes
+
----------------
This option seems to describe a list of class prefixes that are whitelisted. If that is the case, perhaps `WhitelistedPrefixes` or `WhitelistedClassPrefixes` would be a better name for the option? WDYT?


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

https://reviews.llvm.org/D65917





More information about the cfe-commits mailing list