[PATCH] D33645: [analyzer] Add missing documentation for static analyzer checkers

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

Thanks for the patch. It is really great to see these documented!

Who is the target of this documentation? Is it developers of the analyzer or is it end users of the analyzer? If it is end users, it is unfortunate that we've been just grabbing examples from the regression tests. These tests are usually not idiomatic and, since they're designed for testing, they typically won't explain the check well to an end user.

I've suggested some more idiomatic examples inline for for some of the Objective-C targeted checkers -- but it is probably worth thinking about the remaining examples from the perspective of an end user.



================
Comment at: www/analyzer/alpha_checks.html:180
+<div class="example"><pre>
+ at protocol NSCopying
+ at end
----------------
I'd suggest replacing all of this with the following, which is more idiomatic:

```
id date = [NSDate date];

// Warning: Object has a dynamic type 'NSDate *' which is incompatible with static type 'NSNumber *'"
NSNumber *number = date;
[number doubleValue];
```


================
Comment at: www/analyzer/alpha_checks.html:563
+<div class="example"><pre>
+ at interface NSObject
++ (id)alloc;
----------------
Let's replace all of this with:



```
NSString *reminderText = NSLocalizedString(@"None", @"Indicates no reminders");
if (reminderCount == 1) {
  // Warning: Plural cases are not supported accross all languages. Use a .stringsdict file instead
  reminderText = NSLocalizedString(@"1 Reminder", @"Indicates single reminder");
} else if (reminderCount >= 2) {
  // Warning: Plural cases are not supported accross all languages. Use a .stringsdict file instead
  reminderText = [NSString stringWithFormat:
                      NSLocalizedString(@"%@ Reminders", @"Indicates multiple reminders"),
                       reminderCount];
}
```



================
Comment at: www/analyzer/available_checks.html:475
+<div class="example"><pre>
+typedef struct Dummy { int val; } Dummy;
+void takesNonnull(Dummy *_Nonnull);
----------------
How about:


```
if (name != nil)
  return;
// Warning: nil passed to a callee that requires a non-null 1st parameter
NSString *greeting = [@"Hello " stringByAppendingString:name];
```


================
Comment at: www/analyzer/available_checks.html:492
+<div class="example"><pre>
+typedef struct Dummy { int val; } Dummy;
+
----------------
How about:


```
- (nonnull id)firstChild {
  id result = nil;
  if ([_children count] > 0)
    result = _children[0];

  // Warning: nil returned from a method that is expected to return a non-null value
  return result;
}
```


================
Comment at: www/analyzer/available_checks.html:507
+<div class="example"><pre>
+typedef struct Dummy { int val; } Dummy;
+Dummy *_Nullable returnsNullable();
----------------
How about:

```
struct LinkedList {
  int data;
  struct LinkedList *next;
};

struct LinkedList * _Nullable getNext(struct LinkedList *l);

void updateNextData(struct LinkedList *list, int newData) {
  struct LinkedList *next = getNext(list);
  // Warning: Nullable pointer is dereferenced
  next->data = 7;
}
```


================
Comment at: www/analyzer/available_checks.html:597
+<div class="example"><pre>
+- (void)test {
+  UILabel *testLabel = [[UILabel alloc] init];
----------------
How about:


```
NSString *alarmText = NSLocalizedString(@"Enabled", @"Indicates alarm is turned on");
if (!isEnabled) {
  alarmText = @"Disabled";
}
UILabel *alarmStateLabel = [[UILabel alloc] init];

// Warning: User-facing text should use localized string macro
[alarmStateLabel setText:alarmText];
```


================
Comment at: www/analyzer/available_checks.html:639
+<div class="example"><pre>
+typedef const struct __CFNumber *CFNumberRef;
+void takes_int(int);
----------------
Here is an idiomatic example:


```
NSNumber *photoCount = [albumDescriptor objectForKey:@"PhotoCount"];
// Warning: Comparing a pointer value of type 'NSNumber *' to a scalar integer value
if (photoCount > 0) {
  [self displayPhotos];
}
```


================
Comment at: www/analyzer/available_checks.html:951
+<div class="example"><pre>
+ at protocol NSCopying
+ at end
----------------
The example for this checker should have Objective-C generics in it.

How about:


```
NSMutableArray<NSString *> *names = [NSMutableArray array];
NSMutableArray *birthDates = names;

// Warning: Conversion from value of type 'NSDate *' to incompatible type 'NSString *'
[birthDates addObject: [NSDate date]];
```


================
Comment at: www/analyzer/available_checks.html:1038
+ at implementation SuperDeallocThenReleaseIvarClass
+- (instancetype)initWithIvar:(NSObject *)ivar {
+  self = [super init];
----------------
I don't think it is necessary to include the -initWithIvar: method in this  example; just -dealloc is enough.


https://reviews.llvm.org/D33645