Skip to content

Commit

Permalink
Strip null on the scrutinee
Browse files Browse the repository at this point in the history
Without that, we end up with either a flexible type or a `| Null`.
  • Loading branch information
dwijnand committed Jul 5, 2024
1 parent 2915c25 commit 620d977
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
30 changes: 14 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ object SpaceEngine {
doShow(s)
}

private def exhaustivityCheckable(sel: Tree)(using Context): Boolean = trace(i"exhaustivityCheckable($sel ${sel.className})") {
private def exhaustivityCheckable(selTyp: Type)(using Context): Boolean = trace(i"exhaustivityCheckable($selTyp ${selTyp.className})") {
val seen = collection.mutable.Set.empty[Symbol]

// Possible to check everything, but be compatible with scalac by default
Expand All @@ -812,14 +812,14 @@ object SpaceEngine {
tpw.isRef(defn.BooleanClass) ||
classSym.isAllOf(JavaEnum) ||
classSym.is(Case) && {
if seen.add(classSym) then productSelectorTypes(tpw, sel.srcPos).exists(isCheckable(_))
if seen.add(classSym) then productSelectorTypes(tpw, NoSourcePosition).exists(isCheckable(_))
else true // recursive case class: return true and other members can still fail the check
}

!sel.tpe.hasAnnotation(defn.UncheckedAnnot)
!selTyp.hasAnnotation(defn.UncheckedAnnot)
&& {
ctx.settings.YcheckAllPatmat.value
|| isCheckable(sel.tpe)
|| isCheckable(selTyp)
}
}

Expand All @@ -835,7 +835,7 @@ object SpaceEngine {
/** Return the underlying type of non-module, non-constant, non-enum case singleton types.
* Also widen ExprType to its result type, and rewrap any annotation wrappers.
* For example, with `val opt = None`, widen `opt.type` to `None.type`. */
def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp)")(tp match {
def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp ${tp.className})")(tp match {
case _: ConstantType => tp
case tp: TermRef if tp.symbol.is(Module) => tp
case tp: TermRef if tp.symbol.isAllOf(EnumCase) => tp
Expand All @@ -845,8 +845,7 @@ object SpaceEngine {
case _ => tp
})

def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)") {
val selTyp = toUnderlying(m.selector.tpe).dealias
def checkExhaustivity(m: Match, selTyp: Type)(using Context): Unit = trace(i"checkExhaustivity($m)") {
val targetSpace = trace(i"targetSpace($selTyp)")(project(selTyp))

val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) =>
Expand All @@ -866,21 +865,19 @@ object SpaceEngine {
report.warning(PatternMatchExhaustivity(deduped.map(display), m), m.selector)
}

private def reachabilityCheckable(sel: Tree)(using Context): Boolean =
private def reachabilityCheckable(selTyp: Type)(using Context): Boolean =
// Ignore Expr[T] and Type[T] for unreachability as a special case.
// Quote patterns produce repeated calls to the same unapply method, but with different implicit parameters.
// Since we assume that repeated calls to the same unapply method overlap
// and implicit parameters cannot normally differ between two patterns in one `match`,
// the easiest solution is just to ignore Expr[T] and Type[T].
!sel.tpe.hasAnnotation(defn.UncheckedAnnot)
&& !sel.tpe.widen.isRef(defn.QuotedExprClass)
&& !sel.tpe.widen.isRef(defn.QuotedTypeClass)
!selTyp.hasAnnotation(defn.UncheckedAnnot)
&& !selTyp.widen.isRef(defn.QuotedExprClass)
&& !selTyp.widen.isRef(defn.QuotedTypeClass)

def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)") {
def checkReachability(m: Match, selTyp: Type)(using Context): Unit = trace(i"checkReachability($m)") {
val cases = m.cases.toIndexedSeq

val selTyp = toUnderlying(m.selector.tpe).dealias

val isNullable = selTyp.classSymbol.isNullableClass
val targetSpace = trace(i"targetSpace($selTyp)")(if isNullable
then project(OrType(selTyp, ConstantType(Constant(null)), soft = false))
Expand Down Expand Up @@ -924,6 +921,7 @@ object SpaceEngine {
}

def checkMatch(m: Match)(using Context): Unit =
if exhaustivityCheckable(m.selector) then checkExhaustivity(m)
if reachabilityCheckable(m.selector) then checkReachability(m)
val selType = toUnderlying(m.selector.tpe.stripNull()).dealias
if exhaustivityCheckable(selType) then checkExhaustivity(m, selType)
if reachabilityCheckable(selType) then checkReachability(m, selType)
}
13 changes: 13 additions & 0 deletions tests/warn/i20132.future-Left.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//> using options -Yexplicit-nulls -Yno-flexible-types

import scala.language.unsafeNulls

import java.util.concurrent.CompletableFuture
import scala.jdk.CollectionConverters._

class Test1:
def m1 =
val fut: CompletableFuture[Either[String, List[String]]] = ???
fut.thenApply:
case Right(edits: List[String]) => edits.asJava
case Left(error: String) => throw new Exception(error)

0 comments on commit 620d977

Please sign in to comment.