-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Consider nullable annotations in explicit nulls #21953
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
base: main
Are you sure you want to change the base?
Conversation
| case tp: TypeRef if defn.isTupleClass(tp.symbol) => false | ||
| case _ => true | ||
|
|
||
| override def apply(tp: Type): Type = tp match |
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.
How about List<@Nullable String> from Java? I would expect an annotated type. Not sure if this is supporeted by the java parser or class reader?
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.
Support can be added, however, in our nullification code, we chose to not recurse on type argument for generic types coming from Java, so supporting these annotations may be in conflict with that decision:
if `C` is Java-defined, then `n(C[T]) = C[T] | Null`. That is, we don't recurse
on the type argument, and only add Null on the outside. This is because
`C` itself will be nullified, and in particular so will be usages of `C`'s type argument within C's body.
e.g. calling `get` on a `java.util.List[String]` already returns `String|Null` and not `String`, so
we don't need to write `java.util.List[String | Null]`.
Now if we have a List<@Nullable String> or List<@Nonnull String>, I believe the above argument doesn't hold anymore.
Fixes #21629
Not sure if the tests are sufficient.