[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D59806#1447929 <https://reviews.llvm.org/D59806#1447929>, @jordan_rose wrote:

> I don't think there's ever a reason to call `[super self]`, and doing so through a macro could easily indicate a bug.


Agreed.

The only relatively common usage of -[NSObject self] that I am aware of is through KVC, e.g., to construct a set of distinct objects using array operators:

  NSArray *uniqueObjects = [array valueForKeyPath:@"@distinctUnionOfObjects.self"]

I don't recall observing -[NSObject self] used in other situations.

> Diagnostic nitpick: the Objective-C term is "init method", not "initializer".

Good catch. Glancing through the project, I have the impression that there is some inconsistency in the terminology used by different diagnostics and documentation in the project. For example, the ARC documentation on method families <http://clang.llvm.org/docs/AutomaticReferenceCounting.html#method-families> and various tests use the term "init methods". In contrast, -Wobjc-designated-initializers <https://clang.llvm.org/docs/DiagnosticsReference.html#wobjc-designated-initializers> uses the term "initializer". The general pattern //seems// to be that documentation/diagnostics related to ARC use the term "init method" and diagnostics/documentation related to designated initializers use the term "initializer". I had assumed that "initializer" would be more intuitive to readers as this is the term typically used in Apple developer documentation, e.g., Concepts in Objective-C Programming > Object Initialization <https://developer.apple.com/library/archive/documentation/General/Conceptual/CocoaEncyclopedia/Initialization/Initialization.html>.

I believe that "initializer" will be more intuitive because of Apple documentation but I can change to "init method" if that is preferred. Please let me know what your preference is.



================
Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112
+      << Message->getMethodDecl()
+      << FixItHint::CreateReplacement(Message->getSourceRange(),
+                                      StringRef("[super init]"));
----------------
aaron.ballman wrote:
> This could be dangerous if the `[super self]` construct is in a macro, couldn't it? e.g.,
> ```
> #define DERP self
> 
> [super DERP];
> ```
Good point. Let me add some test cases and make sure this is handled properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806