[PATCH] D27535: [analyzer] Add ObjCPropertyChecker - check for autosynthesized copy-properties of mutable types.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

This looks great to me other than the idiom I mentioned inline (which is common, as I have found to my chagrin).



================
Comment at: lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp:12
+//  Currently finds only one kind of issue:
+//  - Find autosynthesized properties with copy attribute of
+//    mutable NS collection types. Calling -copy on such collections
----------------
Super pedantic nit: 'autosynthesized' properties are properties for which the programmer didn't even write @synthesize but the compiler decided to synthesize storage and accessors for them anyway (it didn't always do this; it was a new feature a while back).  If I understand correctly, I think you warn for both synthesized and autosynthesized properties.


================
Comment at: lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp:68
+    return;
+
+  BR.EmitBasicReport(
----------------
You'll also want to make sure to not warn on the following idiom, in which programmers use @synthesize to generate the storage but still provide their own accessors:

```
@interface Foo
@property(copy) NSMutableString *foo;
@end

@implementation Foo
@synthesize foo;
-(NSMutableString *)foo {
  return foo;
}
- (void)setFoo:(NSMutableString *)newValue {
  foo = [newValue mutableCopy];
}
@end
```
I *think* a call to `ObjCContainerDecl::HasUserDeclaredSetterMethod()` should be sufficient to detect and early exit in this case.


https://reviews.llvm.org/D27535