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

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore requested changes to this revision.
stephanemoore marked an inline comment as done.
stephanemoore added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:112
+          Result.Nodes.getNodeAs<ObjCMessageExpr>("new_call")) {
+    // Don't warn if the call expression originates from a macro expansion.
+    if (isMessageExpressionInsideMacro(CallExpr))
----------------
mwyman wrote:
> stephanemoore wrote:
> > If the message expression is within a macro expansion, maybe we should emit the diagnostic without the fixit?
> I'm leery of emitting a warning in a case where the code may not originate from the code in question. Maybe if the macro is defined in the same source file, but I think perhaps that could be a follow-up.
I think that it's reasonable to surface the diagnostic. If the user determines that the diagnostic is not relevant in this situation then they can suppress it. That is, I believe it's preferable to surface issues and have the user determine relevance rather than suppress diagnostics without the knowledge of the user.

(with that said, I do not consider this a hard blocker and am comfortable deferring this for future consideration)


================
Comment at: clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m:53
+
+ at interface ProxyFoo : NSProxy
+ at end
----------------
Shouldn't you declare `+[ProxyFoo new]` so that you can call it on line 67?


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

https://reviews.llvm.org/D61350