-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Enable NullAway's JSpecify mode and fix resulting issues #4618
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
Conversation
4d67c0e
to
dc41153
Compare
@Nullable | ||
T proceed() throws Throwable; |
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.
Does it really make sense to force all Invocation
implementations to return @Nullable
value?
Would it be better to keep an older Invocation<T extends @Nullable Object>
approach so every implementation declares if it really wants to produce nullable value or not?
* @see #forJavaUtilLogging(Level) | ||
*/ | ||
public static LoggingListener forBiConsumer(BiConsumer<Throwable, Supplier<String>> logger) { | ||
public static LoggingListener forBiConsumer(BiConsumer<@Nullable Throwable, Supplier<String>> logger) { |
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.
In most of the cases, the proper signature for Function
-receiving methods is as follows:
public static LoggingListener forBiConsumer(BiConsumer<@Nullable Throwable, Supplier<String>> logger) { | |
public static LoggingListener forBiConsumer(BiConsumer<? super @Nullable Throwable, ? super Supplier<String>> logger) { |
That would allow users to pass even something that implements BiConsumer<@Nullable Object, @Nullable Object>
.
Unfortunately, Java uses use-site variance, so effectively it requires users to add super
and extends
for almost every method that uses generic parameters.
abstract void invokeMethod(); | ||
|
||
abstract <T> T invokeConstructor(Constructor<T> constructor, @Nullable Object outerInstance); | ||
abstract <T> @Nullable T invokeConstructor(Constructor<T> constructor, @Nullable Object outerInstance); |
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.
It does not sound right for invokeConstructor
to return null. Does it mean the null signals the failure?
Superseded by #4628 |
Overview
In order to check nullability annotations on generics in the build.
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@API
annotations