-
Notifications
You must be signed in to change notification settings - Fork 288
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
add Throwable.getCause to nullableReturns #717
add Throwable.getCause to nullableReturns #717
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the contribution! This will cause NullAway to report new errors in the next release but I think it's worth it. @lazaroclapp PTAL in case I missed something. I'll hold off on landing until then. @XN137 may take a couple of days due to other deadlines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Thanks for the contribution! 🚀
Agree with @msridhar this will likely cause new errors for existing NullAway enrolled codebases, but a) they very well could be true positives, b) we can always (say, internally, at Uber) override these models for a bit if we need to.
reasoning:
getLocalizedMessage()
callsgetMessage()
, so should be treated in the same waygetCause()
will often returnnull
, as indicated by its javadoc