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

NullAway complaining about returning null for a return type of CompletableFuture<Void> #801

Open
thugsatbay opened this issue Aug 11, 2023 · 5 comments
Labels
jspecify Related to support for jspecify standard (see jspecify.dev)

Comments

@thugsatbay
Copy link

thugsatbay commented Aug 11, 2023

// The purpose of this method is to run some heavy code of publish data on a separate thread pool 
// as that is blocking code. Whereas once published switch back to application thread pool. 
// The response needs to be a CompletableFuture<Void>  that informs if publish has successfully
// happened.
  @SuppressWarnings("NullAway")
  public CompletableFuture<Void> produceAsync(
      @Nonnull Parameter1, @Nonnull Parameter2) {

    return CompletableFuture.supplyAsync(
            () -> {
              // Some method that will publish data, also might throw an error as RuntimeException
              return null; 
            },
            someExecutorService)
        // If an exception occurs or not we move the flow to application thread pool.
        .handleAsync(
            (ignored, throwable) -> {
              if (throwable != null) {
                if (throwable instanceof RuntimeException) {
                  throw (RuntimeException) throwable;
                }

                throw new RuntimeException(throwable);
              }
              // In case there was no exception return null.
              // NullAway is complaining we are returning null. Hence using suppress warning annotation.
              // The null is converted to CompletableFuture<Object> and then to CompletbaleFuture<Void>.
              return null; 
            },
            applicationExecutorService)
        // Convert to Void completable future. whenCompleteAsync returns Object completable future.
        .thenRun(() -> {});
  }
}

Linter complaining about null when contract of method is @nonnull. However, the null is converted from CompletableFuture -> CompletableFuture

We might need to fix the edge case for the linter. In general I think, NullAway for the special case of T is unable to handle it.

@msridhar
Copy link
Collaborator

Sorry, I can't understand what error you are seeing. Can you point out the exact location where the error is reported and what the error message is? And also can you fix the formatting of your example code? Thanks!

@Wolf2323
Copy link

I also run into this issue, it seems that the Void return objective used with Generics is not detected correctly in every case. I wrote a simple example code showing the issue:

package org.betonquest.betonquest;

public class Test {

    public TestInterface<Void> getTest() {
        return () -> {
            return null;
        };
    }

    @FunctionalInterface
    interface TestInterface<T> {
        T test();
    }
}

NullAway reports:

[7,13] [NullAway] returning @Nullable expression from method with @NonNull return type

TheosRee added a commit to Wolf2323/BetonQuest that referenced this issue Jun 27, 2024
TheosRee added a commit to Wolf2323/BetonQuest that referenced this issue Jun 27, 2024
TheosRee added a commit to Wolf2323/BetonQuest that referenced this issue Jun 27, 2024
TheosRee added a commit to Wolf2323/BetonQuest that referenced this issue Jun 27, 2024
@msridhar msridhar added the jspecify Related to support for jspecify standard (see jspecify.dev) label Jun 29, 2024
@msridhar
Copy link
Collaborator

Thanks for the repro case, @Wolf2323, I see this behavior as well. Unfortunately fixing this issue will require better support for generics / JSpecify; I don't we can easily fix it otherwise. I added a test case in #990 that shows how this code can be type checked in JSpecify mode. This mode is still a work in progress and not ready for production I think, but you are free to try it on your code base by adding the option -XepOpt:NullAway:JSpecifyMode=true to see how it does.

Note that in JSpecify mode you need to write @Nullable Void to allow for null values; see jspecify/jspecify#51 for discussion.

I'll leave this issue open so I can comment back once we think JSpecify mode is ready for broader testing, at which point we can close it.

@Wolf2323
Copy link

sounds good in general. We use jetbrains annotations, when i understand correctly @Nullable Void is from jspecify and i will currently not add this to the project, so i can not test this out

@msridhar
Copy link
Collaborator

msridhar commented Jul 6, 2024

Yeah, the JSpecify support will not work with the Jetbrains annotations right now. Once it's in better shape I can comment back and we can see if it's feasible for you all to switch over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jspecify Related to support for jspecify standard (see jspecify.dev)
Projects
None yet
Development

No branches or pull requests

3 participants