warn about unnecessary uses of .nn#23327
Conversation
|
I can't figure out the bootstrapped test failures. It seems to be a Heisenbug. Locally, if I do |
| def admitsNull(using Context): Boolean = { | ||
| self.isNullType || self.isAny || (self match | ||
| case OrType(l, r) => r.admitsNull || l.admitsNull | ||
| case AndType(l, r) => r.admitsNull && l.admitsNull | ||
| case TypeBounds(lo, hi) => lo.admitsNull | ||
| case FlexibleType(lo, hi) => true | ||
| case tp: TypeProxy => tp.underlying.admitsNull | ||
| case _ => false | ||
| ) | ||
| } |
There was a problem hiding this comment.
I'd like to delete the case tp: TypeProxy, since it will essentially widen a type.
Let's also move it next to isNotNull because of the similarity.
def admitsNull(using Context): Boolean =
self.isNullType || self.isAny || self.dealias match
case OrType(l, r) => r.admitsNull || l.admitsNull
case AndType(l, r) => r.admitsNull && l.admitsNull
case tp: TypeBounds => tp.lo.admitsNull
case _: FlexibleType => true
case _ => falseThere was a problem hiding this comment.
Without the TypeProxy case, the following test fails (no warning):
def f6[T >: String|Null](s: String|Null): T = s.nn // warnSince the test is approximately NullType <:< tp, what's the downside of widening tp if the original narrow tp fails the test. (I can see that widening eagerly before the match would cause us to miss some cases.)
There was a problem hiding this comment.
Will it be enough if we add a case for TypeRef and recur on its info?
There was a problem hiding this comment.
That does cover this f6 test case, but would miss other cases such as AnnotatedType.
Do you have a particular example in mind where TypeProxy would do the wrong thing?
There was a problem hiding this comment.
I'm ok with the current rule and can be merged, given a singleton type wouldn't be inferred at type argument places of nn.
608a506 to
0f76b83
Compare
Issue warnings when
.nnis called on a receiver that is already not null or when the expected type of the result of.nnadmits null.cc @noti0na1