[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood marked an inline comment as done. dgatwood added inline comments. Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:23 + Finder->addMatcher(objcMethodDecl(hasDeclContext( + objcCategoryDecl())).bind(CustomCategoryMethodIdentifier), this); +} stephanemoore wrote: > dgatwood wrote: > > stephanemoore wrote: > > > 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. > > 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. :-) > > 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](https://reviews.llvm.org/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](https://github.com/google/styleguide/blob/gh-pages/objcguide.md#class-names)). > > Generated Objective-C Protocol
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 221142.
dgatwood added a comment.
Updated the configuration key in the test file.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.ExemptClassPrefixes, 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)unprefixedMethodInClassWithExpectedPrefix;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+
+
+Warns when Objective-C category method names are not properly prefixed (e.g.
+``gmo_methodName``) unless the category is extending a class with an
+expected prefix (configurable).
+
+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 allows you to provide a list of expected three-letter
+prefixes specific to your project, and ignores non-prefixed methods in
+categories on classes whose names start with any of 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 specify ``QED`` as an expected three-letter prefix, the following code
+sample is also allowed:
+
+.. code-block:: objc
+
+ @interface QEDMyClass (MyCategory)
+ - (BOOL)myCustomMethod;
+ @end
+
+Options
+---
+
+.. option:: ExemptClassPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 221128.
dgatwood marked 5 inline comments as done.
dgatwood added a comment.
Renamed to ExemptClassPrefixes.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.ExpectedPrefixes, 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)unprefixedMethodInClassWithExpectedPrefix;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+
+
+Warns when Objective-C category method names are not properly prefixed (e.g.
+``gmo_methodName``) unless the category is extending a class with an
+expected prefix (configurable).
+
+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 allows you to provide a list of expected three-letter
+prefixes specific to your project, and ignores non-prefixed methods in
+categories on classes whose names start with any of 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 specify ``QED`` as an expected three-letter prefix, the following code
+sample is also allowed:
+
+.. code-block:: objc
+
+ @interface QEDMyClass (MyCategory)
+ - (BOOL)myCustomMethod;
+ @end
+
+Options
+---
+
+.. option:: ExemptClassPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
---
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood added inline comments. Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:23 + Finder->addMatcher(objcMethodDecl(hasDeclContext( + objcCategoryDecl())).bind(CustomCategoryMethodIdentifier), this); +} stephanemoore wrote: > 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. 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. :-) 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
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 220123.
dgatwood added a comment.
Switched to objcMethodDecl(hasDeclContext(objcCategoryDecl())).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.ExpectedPrefixes, 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)unprefixedMethodInClassWithExpectedPrefix;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+
+
+Warns when Objective-C category method names are not properly prefixed (e.g.
+``gmo_methodName``) unless the category is extending a class with an
+expected prefix (configurable).
+
+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 allows you to provide a list of expected three-letter
+prefixes specific to your project, and ignores non-prefixed methods in
+categories on classes whose names start with any of 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 specify ``QED`` as an expected three-letter prefix, the following code
+sample is also allowed:
+
+.. code-block:: objc
+
+ @interface QEDMyClass (MyCategory)
+ - (BOOL)myCustomMethod;
+ @end
+
+Options
+---
+
+.. option:: ExpectedPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood added inline comments. Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57 + } + std::string method_name = method_declaration->getNameAsString(); + auto owning_objc_class_interface = method_declaration->getClassInterface(); aaron.ballman wrote: > dgatwood wrote: > > aaron.ballman wrote: > > > dgatwood wrote: > > > > aaron.ballman wrote: > > > > > dgatwood wrote: > > > > > > aaron.ballman wrote: > > > > > > > This should use `getName()` to get a `StringRef` to avoid the > > > > > > > copy. > > > > > > 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". > > > > > 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. > > > > I just tried it, and getIdentifier() returns NULL consistently for > > > > every category method, so I changed it back to getNameAsString(), which > > > > works. > > > 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. > > 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. :-/ > > 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() > ``` Yeah, any Objective-C method inside a class is inherently named by a selector. That code works. Thanks. :-) Comment at: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:28 + +If you whitelist the ``QED`` three-letter prefix, the following code sample +is also allowed: aaron.ballman wrote: > whitelist the QED three-letter prefix -> expect the three-letter prefix QED Applied approximately. :-) 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
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 219151.
dgatwood marked 9 inline comments as done.
dgatwood added a comment.
Tweaked documentation, and replaced the one remaining instance of
getNameAsString().
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.ExpectedPrefixes, 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)unprefixedMethodInClassWithExpectedPrefix;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+
+
+Warns when Objective-C category method names are not properly prefixed (e.g.
+``gmo_methodName``) unless the category is extending a class with an
+expected prefix (configurable).
+
+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 allows you to provide a list of expected three-letter
+prefixes specific to your project, and ignores non-prefixed methods in
+categories on classes whose names start with any of 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 specify ``QED`` as an expected three-letter prefix, the following code
+sample is also allowed:
+
+.. code-block:: objc
+
+ @interface QEDMyClass (MyCategory)
+ - (BOOL)myCustomMethod;
+ @end
+
+Options
+---
+
+.. option:: ExpectedPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood added a comment.
Ah. I expected my comments to be submitted as part of uploading a new
revision. Still learning this UI. :-/
Comment at:
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57
+ }
+ std::string method_name = method_declaration->getNameAsString();
+ auto owning_objc_class_interface = method_declaration->getClassInterface();
aaron.ballman wrote:
> dgatwood wrote:
> > aaron.ballman wrote:
> > > This should use `getName()` to get a `StringRef` to avoid the copy.
> > 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".
> 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.
I just tried it, and getIdentifier() returns NULL consistently for every
category method, so I changed it back to getNameAsString(), which works.
Comment at:
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57
+ }
+ std::string method_name = method_declaration->getNameAsString();
+ auto owning_objc_class_interface = method_declaration->getClassInterface();
aaron.ballman wrote:
> dgatwood wrote:
> > aaron.ballman wrote:
> > > dgatwood wrote:
> > > > aaron.ballman wrote:
> > > > > This should use `getName()` to get a `StringRef` to avoid the copy.
> > > > 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".
> > > 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.
> > I just tried it, and getIdentifier() returns NULL consistently for every
> > category method, so I changed it back to getNameAsString(), which works.
> 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.
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. :-/
Comment at:
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:30
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "WhitelistedPrefixes", WhitelistedPrefixes);
+}
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.).
Comment at:
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:30
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "WhitelistedPrefixes", WhitelistedPrefixes);
+}
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
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 218953.
dgatwood marked 7 inline comments as done.
dgatwood added a comment.
Fixed a couple of variable names in the .h file, renamed
kCustomCategoryMethodIdentifier to CustomCategoryMethodIdentifier, and switched
to llvm::any_of.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.ExpectedPrefixes, 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)unprefixedMethodInClassWithExpectedPrefix;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+
+
+Warns when Objective-C category method names are not properly prefixed (e.g.
+``gmo_methodName``) unless the category is extending a class with an
+expected prefix (configurable).
+
+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.
+
+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
+
+Options
+---
+
+.. option:: ExpectedPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
---
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 218812.
dgatwood marked 22 inline comments as done.
dgatwood added a comment.
Incorporated additional feedback.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.ExpectedPrefixes, 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)unprefixedMethodInClassWithExpectedPrefix;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+
+
+Warns when Objective-C category method names are not properly prefixed (e.g.
+``gmo_methodName``) unless the category is extending a class with an
+expected prefix (configurable).
+
+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.
+
+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
+
+Options
+---
+
+.. option:: ExpectedPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,11 +73,12 @@
Finds instances where variables with static
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood marked 17 inline comments as done. dgatwood added inline comments. Comment at: clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57 + } + std::string method_name = method_declaration->getNameAsString(); + auto owning_objc_class_interface = method_declaration->getClassInterface(); aaron.ballman wrote: > This should use `getName()` to get a `StringRef` to avoid the copy. 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". Comment at: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:19 + +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 Eugene.Zelenko wrote: > See other checks documentation for proper option section style. > > clang -> :program:`clang-tidy` and enclose WhitelistedPrefixes in single > back-ticks. Removed the text in question, because it duplicates the Options info. 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
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood updated this revision to Diff 218478.
dgatwood added a comment.
Incorporated feedback.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65917/new/
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// 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
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -230,6 +230,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+===
+
+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.
+
+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.
+
+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
+
+Options
+---
+
+.. option:: WhitelistedPrefixes
+
+ A semicolon-delimited list of class name prefixes. Methods in categories
+ that extend classes whose names begin with any of these prefixes are exempt
+ from the method name prefixing requirement.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,13 @@
Improvements to clang-tidy
--
+- New :doc:`google-objc-require-category-method-prefixes
+ `
[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.
dgatwood added a comment. 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. :-) Repository: rG LLVM Github Monorepo 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
[PATCH] D65917: Added clang-tidy module for the Google style guide's category method naming rule.
dgatwood created this revision.
dgatwood added a reviewer: stephanemoore.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65917
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// 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
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -229,6 +229,7 @@
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-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,39 @@
+.. 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
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,13 @@
Improvements to clang-tidy
--
+- New :doc:`google-objc-require-category-method-prefixes
+ ` check.
+
+ Warns when Objective-C category method names are not
