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

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

16

Please use static. See LLVM Coding Guidelines.

58

Please don't use auto, unless type presents in same statement or in case of iterators.

clang-tools-extra/docs/ReleaseNotes.rst

74

Please enclose gmo_methodName in double back-ticks.

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

6

Please make first statement same as in Release Notes.

16

Please enclose gmo_ in double back-ticks.

19

See other checks documentation for proper option section style.

clang -> :program:clang-tidy and enclose WhitelistedPrefixes in single back-ticks.

24

Please enclose NSObject in double back-ticks.

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

31

Please enclose QED in double back-ticks.

[aaron.ballman]

clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp

44

Please keep this list in alphabetical order.

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

21

You should elide the braces here per our usual coding conventions (also applies elsewhere).

25

No need for the local here, I'd just inline it below.

37

I think this interface should accept a StringRef argument and either return a std::string or an llvm::Optional<std::string> rather than allocating a pointer.

43

No need for messy C APIs here -- you can compare a StringRef to a std::string directly.

57

This should use getName() to get a StringRef to avoid the copy.

62

Can use getName() and assign to a StringRef

100

Drop the period at the end of the diagnostic -- clang-tidy diagnostics are not meant to be grammatically correct.

101

You should pass method_declaration instead; it will get quoted and converted to the proper identifier automatically.

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h

28

You should pick names that match the LLVM coding style naming rules (here and elsewhere in the patch).

[dgatwood]

Whoops. I was expecting to get emailed about review comments and never got anything, so I thought it was just not getting reviewed. I'll work on this again. Sorry about that. :-)

[dgatwood]

Incorporated feedback.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

70

Please sort new checks list alphabetically.

[dgatwood]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

57

That's actually what I originally tried, but that method won't work here, unless I'm missing something. The getName() method crashes with a message saying that "Name is not a simple identifier".

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

19

Removed the text in question, because it duplicates the Options info.

[aaron.ballman]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

16

This comment also meant that you should drop the anonymous namespace (per our coding conventions -- we only use anonymous namespaces for types).

20

Finder

31

ExpectedPrefixes here as well.

Should there be a default list of these?

40–44

llvm::find_if(PrefixArray, [ClassName](const std::string &Str) { return ClassName.startswith(Str); });

50

MethodDeclaration

57

MethodName (and so on, I'll stop commenting on them.)

57

You can call getIdentifier() instead, and if you get a non-null object back, you can call getName() on that. If it is null, there's nothing to check.

58

Drop the clang::

65

Can use const auto * because the type is in the initialization. However, drop the clang:: as it's not needed.

76

I notice we never actually care about the string contained within MatchingPrefix, which suggests that matchingWhitelistedPrefix() could just return a bool.

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h

23–24

Finder and Result per coding style.

29–32

I think the name ExpectedPrefixes and matchesExpectedPrefix() are more clear.

32

class_name should be ClassName.

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

5

Underlining looks off here.

[dgatwood]

Incorporated additional feedback.

[aaron.ballman]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

16

Drop the k prefix (we don't use Hungarian notation).

31

Still wondering whether we should have a default list of expected prefixes or not.

37–41

Sorry for not recognizing this earlier, but since we don't care about which item was found, we can go with:

return llvm::any_of(PrefixArray, ...);

57

The comment to use getIdentifier() was marked as done but the changes were not applied; was that a mistake? I'm pushing back on getNameAsString() because the function is commented as having its use discouraged, so we should not be adding new uses of it.

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h

23–24

finder -> Finder
result -> Result

32

class_name -> ClassName

[dgatwood]

Fixed a couple of variable names in the .h file, renamed kCustomCategoryMethodIdentifier to CustomCategoryMethodIdentifier, and switched to llvm::any_of.

[dgatwood]

Ah. I expected my comments to be submitted as part of uploading a new revision. Still learning this UI. :-/

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

31

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.).

31

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.).

57

Yeah. I finally realized I should just grep for '_' instead of looking for them by hand — one of the joys of converting something originally written under Google's style guide so that it conforms to the llvm style guide instead. Sorry about that. I hope I got all of them this time.

57

I just tried it, and getIdentifier() returns NULL consistently for every category method, so I changed it back to getNameAsString(), which works.

57

I marked that as done because I tried it and it didn't work. The getIdentifier() method returned NULL for every category method.

BTW, this isn't my first attempt at writing this code in a way that doesn't require that method. I literally fought with getting the name of category methods for a day or more when I first started writing this, because I kept getting NULLs or crashes. At one point, I think I even tried looking for the owning class and querying its interface. Nothing worked until I discovered getNameAsString().

I'm assuming that this is simply a bug somewhere in the LLVM core that nobody has noticed or bothered to fix, because it really should not be difficult to get the name of a method. :-/

76

We could. Originally, I was printing it for diagnostics, but now that I ripped out the debug logging, it isn't really needed. Done.

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h

23–24

What the heck? Oh, I fixed it in the .cc file and not the .h file. Sorry. Done.

[aaron.ballman]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

31

Ah, thank you for the explanation!

57

No worries, things are looking good now!

57

I'm assuming that this is simply a bug somewhere in the LLVM core that nobody has noticed or bothered to fix, because it really should not be difficult to get the name of a method. :-/

I am not certain if it's a bug or not, but we shouldn't be using essentially deprecated APIs to work around bugs elsewhere. I'd really like to know what's going on here. I am looking through the logic of DeclarationName::print() (which is what getNameAsString() eventually calls into) and it doesn't look like it makes any special accommodations for ObjC method declarations, but it does for ObjC selectors. I'm not an ObjC expert, but I think that's what the name of an ObjC method is, isn't it? If so, I think you would do (assuming the methods can only be named using selectors):

MethodDeclaration->getDeclName().getObjCSelector().getAsString()

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h

23–24

No worries!

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

29

whitelist the QED three-letter prefix -> expect the three-letter prefix QED

[dgatwood]

Tweaked documentation, and replaced the one remaining instance of getNameAsString().

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

57

Yeah, any Objective-C method inside a class is inherently named by a selector. That code works. Thanks. :-)

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

29

Applied approximately. :-)

[aaron.ballman]

LGTM!

[stephanemoore]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

23–24

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.

31

Agreed that there should be no default.

[dgatwood]

Switched to objcMethodDecl(hasDeclContext(objcCategoryDecl())).

[stephanemoore]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

24

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 and then add isExpansionInSystemHeader as an inner matcher on the custom inner matcher. WDYT? Let me know if you need help putting together.

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

42

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?

[aaron.ballman]

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

42

No, please. We should try to avoid "white" and "black" lists in favor of more descriptive terms with less social connotations. I'd be fine with ExpectedClassPrefixes, AllowedPrefixes, KnownPrefixes, etc.

[stephanemoore]

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

42

Makes sense. Would ExemptClassPrefixes work?

[aaron.ballman]

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

42

I think that'd be a great name.

[dgatwood]

Renamed to ExemptClassPrefixes.

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

24

Requiring users to specify which classes should be covered by this checker doesn't scale well. System classes are a tiny fraction of the shared code that we bring in. Proto classes alone probably outnumber system framework classes 10:1, plus all the shared code from other internal framework teams. A list of every shared class that we bring in would be massive, and generating it programmatically would be relatively expensive. The same problem exists for a list of prefixes to protect.

Also with that approach, a mistake by a person or script that maintains such a list would result in not getting warnings. Silent failures are the worst kind of failure, because you don't even know that something is going wrong. By contrast, if you require the user to specify a list of prefixes to ignore, as this patch does, then any mistake by someone maintaining the lest results in getting extra warnings, which makes it obvious that something is wrong and needs to be fixed.

I realize that ostensibly a team could own some code that is also shared, and it could have the same prefix as their app. But there's no real reason to care about that case. After all, if someone changes the shared code and it breaks the category, it's the same team, so their tests should catch the breakage, unlike changes made by some far-flung team on the other side of the world. Also, if they break their own code, they also have permission to fix that breakage without additional approvals. So that edge case is largely academic; if anybody asks for a way to not ignore specific classes that have an exempt prefix, we can certainly add that feature later pretty easily, but I really doubt anybody would bother to use it. :-)

[dgatwood]

Updated the configuration key in the test file.

[stephanemoore]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

24

Requiring users to specify which classes should be covered by this checker doesn't scale well.

I am not convinced that listing exemptions scales well either. In D51832, we abandoned a growing list of exemptions that were being used to enforce naming conventions. I am worried that a list of exempt prefixes will also end up growing in a similar fashion.

System classes are a tiny fraction of the shared code that we bring in.

That is true.

However, system classes generally have the most breadth and are the most sensitive (for example, there was a recent bug introduced when someone declared a category on UIImageView with an unprefixed getter named animationDuration which overrode the actual [animationDuration](https://developer.apple.com/documentation/uikit/uiimageview/1621058-animationduration?language=objc) property on UIImageView).

Proto classes alone probably outnumber system framework classes 10:1, plus all the shared code from other internal framework teams.

That is true but proto classes and internal shared code also tend to have much more limited scope.

My concern with the check in its current form is that it has a strong potential to be noisy by default. That is, if you enable this check without specifying any exempt prefixes then it will warn for _any category_, including categories on classes that are not shared which do not require a prefix. Moreover, the check provides no option to exempt classes that do not have prefixes at all (prefixes are only required for classes that are shared per Google Objective-C class naming guidelines).

Generated Objective-C Protocol Buffer classes should be pretty easy to target since they have a common base class of GPBMessage. It should be pretty simple to check if the category is on a class that is derived from GPBMessage. Note, however, that there are situations where an unprefixed method declaration in a category on an Objective-C Protocol Buffer class is justified, e..g, when overriding -isEqual: or -hash as suggested by the Objective-C Protocol Buffers library.

In general, I would expect there to be less shared code than there is unshared code (compare how many classes are declared in Apple system framework headers to how many classes are collectively declared in macOS/iOS/watchOS/tvOS applications globally). For that reason, I think a better approach would be to opt shared code into enforcement rather than opt unshared code out.

I can imagine a matching structure like so (hasInterface, baseClassRequiresMethodPrefixing, and classRequiresMethodPrefixing would be custom matchers that would need to be implemented in this example):

objcMethodDecl(
  hasDeclContext(
    objcCategoryDecl(
      hasInterface(
        anyOf(
          isExpansionInSystemHeader(),
          isDerivedFrom(baseClassRequiresMethodPrefixing(BaseClassNames)),
          classRequiresMethodPrefixing(ClassNamePatterns)
        )
      )
    )
  )
)

That would allow opting in class hierarchies by base class (handling Objective-C Protocol Buffer classes) as well as opting in classes in shared libraries using the prefix of the shared library.

WDYT?

[dgatwood]

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

24

The difference is that the exemption list would be per-project, created by whoever turns on the check for the project. I was assuming that this checker would be turned on by each team, rather than globally, and that part of doing so would be specifying which class prefixes should be ignored for that particular project.

That said, if you want to turn this check on globally, we probably should support either approach, with the default being to check only categories on system frameworks and classes derived from a specific set of opted-in base classes. Then, if a project owner provides a set of prefixes to exempt, it would expand the check to cover everything not exempted. (That's the mode in which we would prefer to use the feature for our team's project.)

Either way, it is a good idea to auto-exempt any class that isn’t prefixed.

So basically, I'm thinking:

If no prefix list is provided:

Categories on:

• System framework classes.
• A specific list of protected classes and subclasses thereof.

If a prefix list is provided:

Categories on all classes EXCEPT:

• Classes whose prefixes are explicitly excepted.
• Classes that do not start with at least three capital letters (non-prefixed classes).

Does that approach seem reasonable to you?

[stephanemoore]

(sorry for the delay 😅)

The difference is that the exemption list would be per-project, created by whoever turns on the check for the project. I was assuming that this checker would be turned on by each team, rather than globally, and that part of doing so would be specifying which class prefixes should be ignored for that particular project.

The vision that I had in mind for this check was for it to be enabled more widely which would require it to be useful by default without requiring options to be specified to reduce noise.

That said, if you want to turn this check on globally, we probably should support either approach, with the default being to check only categories on system frameworks and classes derived from a specific set of opted-in base classes. Then, if a project owner provides a set of prefixes to exempt, it would expand the check to cover everything not exempted. (That's the mode in which we would prefer to use the feature for our team's project.)

Either way, it is a good idea to auto-exempt any class that isn’t prefixed.

That's a good idea but note that detecting that a class name does not have a prefix is not easy because acronyms and initialisms are capitalized (e.g., a class named URLCache probably does not have a prefix).

So basically, I'm thinking:

If no prefix list is provided:

Categories on:

System framework classes.
A specific list of protected classes and subclasses thereof.
If a prefix list is provided:

Categories on all classes EXCEPT:

Classes whose prefixes are explicitly excepted.
Classes that do not start with at least three capital letters (non-prefixed classes).
Does that approach seem reasonable to you?

I think we are moving in a good direction. I suspect that a single option may not be the best to control the relevant behaviors.

It sounds like we want the following behaviors:
(1) Require method prefixes on categories on system classes.
(2) Require method prefixes on categories on user classes except (2A) user classes whose names begin with an exempted prefix and (2B) user classes that are derived from an exempted base class.

I think it's unlikely that we would want (1) to be conditional but I think the others are likely options that developers would want to control. Based on that judgment call, I would imagine three options like so (option names have not been thought through extensively): IncludeUserClasses, ExemptUserClassPrefixes, and ExemptUserBaseClasses. That way when the check is enabled without any options the developer gets enforcement on system classes which should reduce a common source of errors and developers that want enforcement on their own classes can opt in and use the additional options to control enforcement and reduce noise.

WDYT?

clang-tools-extra/docs/clang-tidy/checks/list.rst

233

Technically this check is not specific to Google Objective-C:
"In order to avoid undefined behavior, it’s best practice to add a prefix to method names in categories on framework classes, just like you should add a prefix to the names of your own classes. You might choose to use the same three letters you use for your class prefixes, but lowercase to follow the usual convention for method names, then an underscore, before the rest of the method name."
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/CustomizingExistingClasses/CustomizingExistingClasses.html

I think we should consider moving this to the objc module (rename_check.py might be used if we decide that the move is justified).

clang-tools-extra/clang-tidy/google/CMakeLists.txt

add_clang_library(clangTidyGoogleModule
   ExplicitMakePairCheck.cpp
   FunctionNamingCheck.cpp
   GlobalNamesInHeadersCheck.cpp
   GlobalVariableDeclarationCheck.cpp
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
   NonConstReferences.cpp
   OverloadedUnaryAndCheck.cpp
+  RequireCategoryMethodPrefixesCheck.cpp
   TodoCommentCheck.cpp
   UnnamedNamespaceInHeaderCheck.cpp
   UpgradeGoogletestCaseCheck.cpp
   UsingNamespaceDirectiveCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
   )

clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp

 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
 #include "FunctionNamingCheck.h"
 #include "GlobalNamesInHeadersCheck.h"
 #include "GlobalVariableDeclarationCheck.h"
 #include "IntegerTypesCheck.h"
 #include "NonConstReferences.h"
 #include "OverloadedUnaryAndCheck.h"
+#include "RequireCategoryMethodPrefixesCheck.h"
 #include "TodoCommentCheck.h"
 #include "UnnamedNamespaceInHeaderCheck.h"
 #include "UpgradeGoogletestCaseCheck.h"
 #include "UsingNamespaceDirectiveCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace google {
 
 class GoogleModule : public ClangTidyModule {
  public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<objc::RequireCategoryMethodPrefixesCheck>(
+        "google-objc-require-category-method-prefixes");
     CheckFactories.registerCheck<build::ExplicitMakePairCheck>(
         "google-build-explicit-make-pair");
     CheckFactories.registerCheck<build::UnnamedNamespaceInHeaderCheck>(
         "google-build-namespaces");
     CheckFactories.registerCheck<build::UsingNamespaceDirectiveCheck>(
         "google-build-using-namespace");
     CheckFactories.registerCheck<DefaultArgumentsCheck>(
         "google-default-arguments");

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h

+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_REQUIRECATEGORYMETHODPREFIXES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_REQUIRECATEGORYMETHODPREFIXES_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace objc {
+
+// Warn about missing prefixes in category method names.
+class RequireCategoryMethodPrefixesCheck : public ClangTidyCheck {
+public:
+  RequireCategoryMethodPrefixesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+        whitelisted_prefixes_(Options.get("WhitelistedPrefixes", "")) {}
+
+  /// Make configuration of checker discoverable.
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+  void registerMatchers(ast_matchers::MatchFinder *finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &result) override;
+
+private:
+  /// Semicolon-separated list of whitelisted class prefixes.
+  /// Defaults to empty.
+  const std::string whitelisted_prefixes_;
+
+  std::unique_ptr<std::string>
+  matching_whitelisted_prefix(std::string class_name);
+};
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_REQUIRECATEGORYMETHODPREFIXES_H

clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp

+#include "RequireCategoryMethodPrefixesCheck.h"
+
+#include "../ClangTidyOptions.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers; // NOLINT: Too many names.
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace objc {
+
+namespace {
+const char *kCustomCategoryMethodIdentifier = "ThisIsACategoryMethod";
+} // anonymous namespace
+
+void RequireCategoryMethodPrefixesCheck::registerMatchers(MatchFinder *finder) {
+  // This check should be run only on Objective-C code.
+  if (!getLangOpts().ObjC) {
+    return;
+  }
+
+  auto method_declaration =
+      objcMethodDecl().bind(kCustomCategoryMethodIdentifier);
+  finder->addMatcher(method_declaration, this);
+}
+
+void RequireCategoryMethodPrefixesCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WhitelistedPrefixes", whitelisted_prefixes_);
+}
+
+std::unique_ptr<std::string>
+RequireCategoryMethodPrefixesCheck::matching_whitelisted_prefix(
+    std::string class_name) {
+  std::vector<std::string> prefix_array =
+      utils::options::parseStringList(whitelisted_prefixes_);
+
+  for (auto &iterator : prefix_array) {
+    const char *prefix = iterator.c_str();
+    if (!strncmp(class_name.c_str(), prefix, strlen(prefix))) {
+      return llvm::make_unique<std::string>(prefix);
+    }
+  }
+  return nullptr;
+}
+
+void RequireCategoryMethodPrefixesCheck::check(
+    const MatchFinder::MatchResult &result) {
+  const auto *method_declaration =
+      result.Nodes.getNodeAs<ObjCMethodDecl>(kCustomCategoryMethodIdentifier);
+  if (!method_declaration) {
+    return;
+  }
+  std::string method_name = method_declaration->getNameAsString();
+  auto owning_objc_class_interface = method_declaration->getClassInterface();
+  if (!owning_objc_class_interface) {
+    return;
+  }
+  std::string class_name = owning_objc_class_interface->getNameAsString();
+
+  auto owning_objc_category =
+      dyn_cast<clang::ObjCCategoryDecl>(method_declaration->getDeclContext());
+  // Bail early if the method is not in a category.
+  if (!owning_objc_category || owning_objc_category->isInvalidDecl()) {
+    return;
+  }
+
+  // Bail early if the method is in a category on a class with a whitelisted
+  // prefix.
+  std::unique_ptr<std::string> matching_prefix =
+      matching_whitelisted_prefix(class_name);
+  if (matching_prefix) {
+    return;
+  }
+
+  // Check whether the method name has a proper prefix.
+  for (const char *pos = method_name.c_str(); pos && *pos; ++pos) {
+    // The characters a-z are allowed in a prefix.  Go to the next
+    // character.
+    if (*pos >= 'a' && *pos <= 'z') {
+      continue;
+    }
+
+    // The underscore character (_) terminates the prefix.  As long as it is
+    // not the last character in the name, the declaration passes.
+    if (*pos == '_' && *(pos + 1) != '\0') {
+      return;
+    }
+
+    // The name does not start with /[a-z]+_/.  This declaration fails.
+    break;
+  }
+  // If the end of the string is reached or an illegal character appears
+  // before the terminating underscore, then the method name is not
+  // properly prefixed.  Throw an error.
+  diag(method_declaration->getBeginLoc(),
+       "the category method %0 is not properly prefixed.")
+      << method_name;
+}
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
+- New :doc:`google-objc-require-category-method-prefixes
+  <clang-tidy/checks/google-objc-require-category-method-prefixes>` check.
+
+  Warns when Objective-C category method names are not properly prefixed (e.g.
+  gmo_methodName) unless the category is extending a class with a (configurable)
+  whitelisted prefix.
+
 - New :doc:`linuxkernel-must-use-errs
   <clang-tidy/checks/linuxkernel-must-use-errs>` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
   ``linux/err.h``.
 
 - New :doc:`google-upgrade-googletest-case
   <clang-tidy/checks/google-upgrade-googletest-case>` check.

clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst

+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+===========================
+
+Finds method declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The Google Objective-C style guide requires
+`prefixes for methods http://go/objc-style#Category_Names`_ in categories on
+classes that you don't control (for example, categories on Apple or third-party
+framework classes or classes created by other teams) to prevent name collisions
+when those frameworks are updated.
+
+This checker ensures that all methods in categories have some sort of prefix
+(e.g. gmo_). It excludes categories on classes whose names have a whitelisted
+three-letter prefix.
+
+You should set the clang option WhitelistedPrefixes to a semicolon-delimited
+lits of class prefixes within your project if you want to be able to create
+unprefixed category methods on classes whose names begin with those prefixes.
+
+For example, the following code sample is a properly prefixed method on a
+non-owned class (NSObject):
+
+.. code-block:: objc
+  @interface NSObject (QEDMyCategory)
+  - (BOOL)qed_myCustomMethod;
+  @end
+
+If you whitelist the QED three-letter prefix, the following code sample
+is also allowed:
+
+.. code-block:: objc
+
+  @interface QEDMyClass (MyCategory)
+  - (BOOL)myCustomMethod;
+  @end
+

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. toctree::
    google-build-using-namespace
    google-default-arguments
    google-explicit-constructor
    google-global-names-in-headers
    google-objc-avoid-nsobject-new
    google-objc-avoid-throwing-exception
    google-objc-function-naming
    google-objc-global-variable-declaration
+   google-objc-require-category-method-prefixes
    google-readability-avoid-underscore-in-googletest-name
    google-readability-braces-around-statements (redirects to readability-braces-around-statements) <google-readability-braces-around-statements>
    google-readability-casting
    google-readability-function-size (redirects to readability-function-size) <google-readability-function-size>
    google-readability-namespace-comments (redirects to llvm-namespace-comment) <google-readability-namespace-comments>
    google-readability-todo
    google-runtime-int
    google-runtime-operator
    google-runtime-references

clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m

+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.WhitelistedPrefixes, value: GMO}]}"
+
+@class NSString;
+
+@interface NSURL
++ (nullable instancetype)URLWithString:(NSString *)URLString;
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+@interface NSURL (CustomExtension)
+
+- (void)unprefixedMethod;
+- (void)unprefixed;
+- (void)justprefixed_;
+
+@end
+
+// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method unprefixedMethod is not properly prefixed. [google-objc-require-category-method-prefixes]
+// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method unprefixed is not properly prefixed. [google-objc-require-category-method-prefixes]
+// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method justprefixed_ is not properly prefixed. [google-objc-require-category-method-prefixes]
+
+@interface NSURL (CustomExtension2)
+- (void)gmo_prefixedMethod;
+@end
+
+@interface GMOURL
++ (nullable instancetype)URLWithString:(NSString *)URLString;
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+@interface GMOURL (CustomExtension3)
+- (void)unprefixedMethodInWhitelistedClass;
+@end