Declaring a non-null return value?

Using VS Code and Java, I’m trying to set up a class that receives potentially null values, and verifies they are not null as part of instance construction. Here’s a simplified version (the real code uses a builder pattern).

public class Job {
   private @NonNull String name;

   public Job(@Nullable String jobName) {
      this.name = checkNotNull(jobName, "Name cannot be null");
   }
}

My checkNotNull() is…

public static <T> @NonNull T checkNotNull(@Nullable T value, @NonNull String message) {
   if (value == null)
      throw new NullPointerException(message);
   return (value);
}

On the this.name = ... assignment, SonarLint is reporting "name" is marked "org.eclipse.jdt.annotation.NonNullByDefault" but is set to null.sonarlint(java:S2637). How do I construct or annotate this so SonarLint realizes the return of checkNotNull() is @NonNull? Or am I misinterpreting the warning?

Thanks!

Hi Jason,

I can not reproduce, can you provide the imports to be sure about which @NonNull and @Nullable we are talking about? (ex: org.springframework.lang.NonNull and org.springframework.lang.Nullable)
And can you confirm that “checkNotNull” is not in the Job.java file?

Sorry for the delay getting back to you. Here is my best attempt at creating a reproduction case: GitHub - starkos/sonarlint-s2637: Replication case for SonarLint problem report

I’m not seeing the exact same behavior from this simplified version though. In my full codebase, SL behaves as described above. In this minimal example, I would expect SL to raise an issue on Job.java:14, where I’m assigning a @Nullable to a @NonNull, but I’m not getting one.

If I were getting one, I would expect the commented out checkNotNull() bit to solve it. That’s what I’m doing in the full codebase, and where I’m seeing the reported issue.

Does this help at all or just confuse the issue more?

Would it be possible to send the larger work-in-progress to you? It isn’t terribly large, and there is nothing particularly proprietary about it, especially in its unfinished state, but I’d still prefer not posting it to the internet at large. In the larger project it is reliably reproduced in both VS Code for Java and Eclipse. Disappointing that I get a different issue in my smaller reproduction case.

Hello? Any ideas on this? Thanks!

Ah excellent…on the latest version of the SonarLint extension for VS Code, my smaller example now throws the same warning (java:S2637) that I’m seeing in the larger build. With the checkNotNull() call commented out, SonarLint should warn about assigning a @Nullable to a @NonNull on Job.java:14, but does not. If you uncomment the checkNotNull() call, the code should now be error free, but SonarLint raises java:S2637 on that same line—it doesn’t seem to understand that checkNotNull() is returning a @NonNull?

Any help appreciated…I’m gathering quite the collection of @SuppressWarnings("java:S2637") in my codebase with this issue…

Hello @starkos

Sorry for the delay, this issue looks simple but hides a great complexity, it’s not easy to give a clear answer.

The good news is that I managed to reproduce the issue, thanks to your great reproducer. Indeed, the engine does not support correctly such kind of user-defined preconditions in another file. I therefore created a ticket to track it: SONARJAVA-4026.

The “bad” one is that it is unclear how to solve this issue, improving the symbolic execution engine is something we have on the table, but I don’t expect it to be fixed in the near future…

At this point, the best I can suggest is to use well-known preconditions, like java.util.Objects.requireNonNull, performing the same. This one is correctly supported by the engine.

Hope it clarifies the situtation.
Quentin

Thanks @Quentin that helps. I had hoped we could keep our custom null error message, but I can live with this.

Actually…the downside of this is Checker Framework, which insists that the signature is requireNonNull(@NonNull String), unlike SL which treats it like requireNonNull(@Nullable String). We’re trying to use this with a builder pattern. We want to make sure that the caller has set all of the required parameters, and we’re using null as a “value not set” marker. So we end up passing a @Nullable String, which Checker flags as an issue because it wants @NonNull String. Bummer.

We could switch to Optional, but that ends up being far more verbose to get what we want:

Optional<String> someValue;
someValue.orElseThrow(new NullPointerException("someValue"));

…because it is important for us to be able to see which value was not set in the logs. Don’t know how everyone seems to be living without this so feel like I must be missing something.

We could work around that be providing our own wrapper around it, but that gets us right back to SL not being able to detect it correctly. So…maybe no easy solution at this time?

Update: I worked around it by writing a stub file for Checker that overrides the default annotations. I still can’t provide my own format for the exception message—I’d need SL to recognized my own methods for that—but otherwise this is acceptable for now. Thanks!

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.