[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

Stephane Moore via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 14 15:08:26 PDT 2019


stephanemoore accepted this revision.
stephanemoore marked an inline comment as done.
stephanemoore added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp:54
+    CheckFactories.registerCheck<objc::AvoidNSObjectNewCheck>(
+        "google-objc-avoid-nsobject-new");
     CheckFactories.registerCheck<objc::AvoidThrowingObjCExceptionCheck>(
----------------
It is a little odd that "google-objc-avoid-nsobject-new" warns on `+new` even for classes that are not derived from `NSObject`. In a sense, "nsobject-new" is being used as an identifier for any class method named "new". Interestingly, renaming to the more direct  "google-objc-avoid-new-class-method" is even more misleading. I have yet to think of a name that I think would actually be better.

While I think the current name is potentially misleading, I think I am okay with this name for the following reasons:
(1) I believe this will only be misleading in marginal scenarios. I believe that it will be exceedingly rare for a class method named `new` to exist in a class hierarchy where the root class is not `NSObject`.
(2) We can always amend the check to restrict it to `NSObject`. In the unexpected circumstance where someone does provide a legitimate use case for a class method named `new` in a class hierarchy where the root class is not `NSObject`, we can simply amend the check to restrict its enforcement to class hierarchies where the root class is `NSObject`.


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

https://reviews.llvm.org/D61350





More information about the cfe-commits mailing list