Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect IntelliJ warning: Method invocation may produce 'NullPointerException' #527

Open
EotT123 opened this issue Nov 24, 2023 · 2 comments

Comments

@EotT123
Copy link
Contributor

EotT123 commented Nov 24, 2023

Describe the bug
Extension methods are a great way to deal with nullness, eg see the example here:

public static boolean isNullOrEmpty(@This CharSequence thiz) {
  return thiz == null || thiz.length() == 0;
}

String name = null;
if (name.isNullOrEmpty()) {
  println("empty");
}

This works great. However, when using a value returned by a method annotated with @Nullable, IntelliJ warns me: Method invocation 'xxx' may produce 'NullPointerException'.

Eg:

// a method that returns a @Nullable object
public @Nullable MyObject doSomething() {
  if(...){
    return new MyObject();
  }else{
    return null;
 }
}

// extension method which accepts a @Nullable object
public static boolean validate(@This @Nullable MyObject thiz, Predicate<MyObject> predicate) {
  return this != null && predicate.test(this);
}

// 
doSomething().validate(<predicate>); // --> Method invocation 'validate' may produce 'NullPointerException'

This is clearly not the case: Even if the value is null, it is still correctly handled, and no NPE is thrown.

To Reproduce
Steps to reproduce the behavior:

  1. Annotate the return value (eg MyObject) of a method with @Nullable
  2. Write an extension method for the return type (MyObject), which accepts a @Nullable MyObject
  3. Call the method of the extension on a @Nullable value.

Expected behavior
IntelliJ should not warn me about a potential NPE

Desktop (please complete the following information):

  • OS Type & Version: Windows 10 22H2
  • Java/JDK version: openjdk 21.0.1
  • IDE version (IntelliJ IDEA or Android Studio): IntelliJ IDEA 2023.2.5
  • Manifold version: 2023.1.30 (latest version)
  • Manifold IntelliJ plugin version: 2023.2.6 (latest version)
@EotT123 EotT123 changed the title Wrong warning: Method invocation may produce 'NullPointerException' Incorrect IntelliJ warning: Method invocation may produce 'NullPointerException' Nov 24, 2023
@rsmckinney
Copy link
Member

Yep, these NPEs should be suppressed, great idea!

@CC007
Copy link
Contributor

CC007 commented Mar 17, 2024

While technically true, this would make extension methods behave different from normal methods when it comes to nullability.

I think it would be better to invest into null-safe operators like ?. for null-safe method calls or ?? to provide a default value when the object is null.

In Kotlin you'd have to mark a receiver specifically to be nullable, if you want to be allowed to call an extension method on a variable that contains the null value. This also adds clarity, since now you know if it's expected if you're allowed to use null in the call or if you'd need to use the ?. operator to make it a null-safe call.

As an example where there's a difference between nullable extension methods called with ?. and .:

public static String myToString(@This @Nullable Object thiz) {
  if (thiz == null) return "null value";
  return thiz.toString();
}

public static final main() {
  String foo = "not null value";
  String bar = null;
  
  String fooString1 = foo.myToString(); // returns "not null value"
  String fooString2 = foo?.myToString(); // returns "not null value"
  String barString1 = bar.myToString(); // returns "null value"
  String barString2 = bar?.myToString(); // returns null
}

So for consistency with normal methods, I think the following would be preferred if the receiver is not specified to be nullable:

public static String myToString(@This Object thiz) { // note the absence of the nullable annotation
  return thiz.toString();
}

public static final main() {
  String foo = "not null value";
  String bar = null;
  
  String fooString1 = foo.myToString(); // returns "not null value"
  String fooString2 = foo?.myToString(); // returns "not null value"
  String barString1 = bar.myToString(); // throws NullPointerException
  String barString2 = bar?.myToString(); // returns null
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants