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

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57
+/// \endcode
+AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf,
+              ast_matchers::internal::Matcher<ObjCInterfaceDecl>,
----------------
aaron.ballman wrote:
> This matcher seems like it would be generally useful -- would you mind adding it to the AST matcher interface rather than local to this check? It doesn't have to be done as part of this patch (we can leave the matcher here for the moment).
Definitely agreed. I will send out a followup for a new AST matcher.


================
Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110
+                               "invoke a superclass initializer?")
+      << Message->getMethodDecl()
+      << FixItHint::CreateReplacement(Message->getSourceRange(),
----------------
aaron.ballman wrote:
> Is there a %0 missing from the diagnostic for this method declaration you're passing in?
Good catch! That's egg on my face 😅🥚

I think I was internally conflicted over specifically mentioning -[NSObject self] versus using the method declaration of the message expression and somehow got stuck halfway in-between 😓 I think it's better to mention the method declaration of the message. Let me know if you think that I should instead mention -[NSObject self].


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:110
+  Finds invocations of -self on super instances in initializers of subclasses
+  of NSObject and recommends calling a superclass initializer instead.
+
----------------
Eugene.Zelenko wrote:
> Please enclose NSObject in ``. Probably same for -self if this is language construct.
Good catch. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806