[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>,
----------------
stephanemoore wrote:
> 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.
Sent out https://reviews.llvm.org/D60543.


================
Comment at: clang-tools-extra/test/clang-tidy/objc-super-self.m:41
+  INITIALIZER_IMPL();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+}
----------------
aaron.ballman wrote:
> stephanemoore wrote:
> > aaron.ballman wrote:
> > > Are you missing a `CHECK-FIXES` here?
> > > 
> > > Personally, I don't think we should try to generate a fixit for this case, so I would expect a CHECK-FIXES that ensures this doesn't get modified.
> > No fix is currently generated for this case which is why there is no `CHECK-FIXES`. I also agree that no fix should be generated for this case.
> > 
> > I must confess that I have yet to fully understand why the fix for this case is discarded (though I am grateful for the behavior). Let me dig around to try to better understand why no fixit is generated for this case and assess adding a condition for emitting the fixit.
> Our typical way to check that a fix is not applied is to use `// CHECK-FIXES: <original code>` to test that the fix was not applied.
Gotcha. Added `CHECK-FIXES` to verify original code is preserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806