linter icon indicating copy to clipboard operation
linter copied to clipboard

Flag the more obvious `null` bugs

Open filiph opened this issue 9 years ago • 3 comments

I know we can't ever catch them all but there is a class of null bugs that are common and relatively easy to spot.

bool commitMapChange(FWorldContext context) {
  ....
  levelStreamingObject = context.world.streamingLevels[j];
  if (levelStreamingObject != null) {
    ....
  } else {
    check(levelStreamingObject);
    LOG(logStreaming, log,
        "Unable to handle streaming object ${levelStreamingObject.name}");
  }
  ....
}

Basically, if we do check for null then it's quite clear the value can be null, and that means we should hint if we're trying to access a member of that value in a place that's not checked for null.

filiph avatar Feb 16 '16 19:02 filiph

Another low hanging fruit would be something like this:

void init() {
  ....
  if (gStreamingPauseBackground == null && gUseStreamingPause) {
    ...
    gStreamingPauseBackground.initRHI();
  }
}

This can easily survive when gUseStreaminPause is always false in testing.

filiph avatar Feb 16 '16 19:02 filiph

And another one:

if (someValue.isEmpty) {
  ...
}
...
if (someValue == null) {
  ...
}

Basically, whenever there is any check for null (even a null-aware operator like ?.) anywhere in scope then we should warn whenever we don't check for null.

filiph avatar Feb 16 '16 19:02 filiph

Also applies inside one if statement:

if (someObject.name.startsWith("Mr. ") ||
    someObject == null ||
    someObject.age >= 18) {
  ...
}

Sorry for the comment spam. These bugs seem to be extremely prevalent.

filiph avatar Feb 16 '16 19:02 filiph

I’m going to close this issue as null safety and the improved type flow analysis that came along with that should solve these concerns. The analyzer has a bunch of extra warnings related to null checks now as well.

If anyone would like to see more null-related lints, please open a new issue. Thanks =D

parlough avatar Dec 12 '23 20:12 parlough