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

Preconditions.checkState(x != null) #363

Closed
elharo opened this issue Oct 17, 2019 · 3 comments
Closed

Preconditions.checkState(x != null) #363

elharo opened this issue Oct 17, 2019 · 3 comments

Comments

@elharo
Copy link

elharo commented Oct 17, 2019

In 0.7.8 it's not quite smart enough to follow this

    public AppEngineWebXmlProjectStageConfiguration build() {
      Preconditions.checkState(sourceDirectory != null, "No source directory supplied");
      Preconditions.checkState(stagingDirectory != null, "No staging directory supplied");

      return new AppEngineWebXmlProjectStageConfiguration(
          this.sourceDirectory,
          this.stagingDirectory,
          this.dockerfile,
          this.enableQuickstart,
          this.disableUpdateCheck,
          this.enableJarSplitting,
          this.jarSplittingExcludes,
          this.compileEncoding,
          this.deleteJsps,
          this.enableJarClasses,
          this.disableJarJsps,
          this.runtime);
    }

That is, in a Builder inner class I'm using Preconditions.checkState (from Guava) instead of Preconditions.checkNotNull because an IllegalStateException is slightly more appropriate here than a NullPointerException. Whatever the exception type, though, the return line can't be reached when sourceDirectory or stagingDirectory are null. However Nullaway thinks these can be reached in that case.

  • [X ] If you think you found a bug, please include a code sample that reproduces the problem. A test case that reproduces the issue is preferred. A stack trace alone is ok but may not contain enough context for us to address the issue.

  • [ X] Please include the library version number, including the minor and patch version, in the issue text.

@msridhar
Copy link
Collaborator

Hi @elharo, thanks for the report! At first glance, I don't think it will easy to catch this one without a special-case hack. We have ways to specify functions that check the nullness of an argument, but nothing yet to inspect boolean arguments more deeply when a function checks their truth. I think adding this kind of support would be low priority as we haven't seen it much before.

You can work around this issue by writing your own version of checkNotNull that throws an IllegalStateException instead, and annotating that method with @Contract("null -> fail") (see docs). I'll leave this issue open in case we do decide to add more specialized support in the future.

@msridhar
Copy link
Collaborator

msridhar commented Jun 24, 2022

@lazaroclapp we can close this one yes? I think it is fixed in 0.9.8

@lazaroclapp
Copy link
Collaborator

lazaroclapp commented Jun 24, 2022

I believe we can, this should be a specific case of the support for checkArgument/checkState in #608 🎉 (fixed in NullAway 0.9.8)

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