Skip to content

Conversation

@copybara-service
Copy link
Contributor

Add RedundantNullCheck bug pattern.

Closes #5107.

A null check is redundant if it is performed on a variable or method call that is known to be non-null within a @NullMarked scope.

Within a @NullMarked scope, types are non-null by default unless explicitly annotated with @Nullable.
Therefore, checking a variable or method return value (that isn't @Nullable) for nullness is unnecessary, as it should never be null.

Example:

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

@NullMarked
class MyClass {
  void process(String definitelyNonNull) {
    // BUG: Diagnostic contains: RedundantNullCheck
    if (definitelyNonNull == null) {
      System.out.println("This will never happen");
    }
    // ...
  }

  String getString() {
    return "hello";
  }

  @Nullable String getNullableString() {
    return null;
  }

  void anotherMethod() {
    String s = getString();
    // BUG: Diagnostic contains: RedundantNullCheck
    if (s == null) {
      // s is known to be non-null because getString() is not @Nullable
      // and we are in a @NullMarked scope.
      System.out.println("Redundant check");
    }

    String nullableStr = getNullableString();
    if (nullableStr == null) { // This check is NOT redundant
      System.out.println("Nullable string might be null");
    }
  }
}

This check helps to clean up code and reduce clutter by removing unnecessary null checks, making the code easier to read and maintain.
It also reinforces the contract provided by @NullMarked and @Nullable annotations.

Fixes #5121

FUTURE_COPYBARA_INTEGRATE_REVIEW=#5121 from bdragan:5107-redundant_null_check f935970

Closes #5107.

A null check is redundant if it is performed on a variable or method call that is known to be non-null within a `@NullMarked` scope.

Within a `@NullMarked` scope, types are non-null by default unless explicitly annotated with `@Nullable`.
Therefore, checking a variable or method return value (that isn't `@Nullable`) for nullness is unnecessary, as it should never be null.

Example:

```java
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

@NullMarked
class MyClass {
  void process(String definitelyNonNull) {
    // BUG: Diagnostic contains: RedundantNullCheck
    if (definitelyNonNull == null) {
      System.out.println("This will never happen");
    }
    // ...
  }

  String getString() {
    return "hello";
  }

  @nullable String getNullableString() {
    return null;
  }

  void anotherMethod() {
    String s = getString();
    // BUG: Diagnostic contains: RedundantNullCheck
    if (s == null) {
      // s is known to be non-null because getString() is not @nullable
      // and we are in a @NullMarked scope.
      System.out.println("Redundant check");
    }

    String nullableStr = getNullableString();
    if (nullableStr == null) { // This check is NOT redundant
      System.out.println("Nullable string might be null");
    }
  }
}
```

This check helps to clean up code and reduce clutter by removing unnecessary null checks, making the code easier to read and maintain.
It also reinforces the contract provided by `@NullMarked` and `@Nullable` annotations.

Fixes #5121

COPYBARA_INTEGRATE_REVIEW=#5121 from bdragan:5107-redundant_null_check f935970
PiperOrigin-RevId: 811924542
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnnecessaryCheckNotNull doesn't work with @NullMarked

1 participant