diff --git a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala index 291498dbc558..47427f58e591 100644 --- a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala +++ b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala @@ -8,13 +8,14 @@ import Types.* object NullOpsDecorator: extension (self: Type) - /** Syntactically strips the nullability from this type. + /** If explicit-nulls is enabled, syntactically strips the nullability from this type. + * If explicit-nulls is not enabled and forceStrip is enabled, removes all Null in unions. * If the type is `T1 | ... | Tn`, and `Ti` references to `Null`, * then return `T1 | ... | Ti-1 | Ti+1 | ... | Tn`. * If this type isn't (syntactically) nullable, then returns the type unchanged. - * The type will not be changed if explicit-nulls is not enabled. + * The type will not be changed if explicit-nulls is not enabled and forceStrip is false. */ - def stripNull(stripFlexibleTypes: Boolean = true)(using Context): Type = { + def stripNull(stripFlexibleTypes: Boolean = true, forceStrip: Boolean = false)(using Context): Type = { def strip(tp: Type): Type = val tpWiden = tp.widenDealias val tpStripped = tpWiden match { @@ -42,12 +43,12 @@ object NullOpsDecorator: } if tpStripped ne tpWiden then tpStripped else tp - if ctx.explicitNulls then strip(self) else self + if ctx.explicitNulls || forceStrip then strip(self) else self } /** Is self (after widening and dealiasing) a type of the form `T | Null`? */ - def isNullableUnion(using Context): Boolean = { - val stripped = self.stripNull() + def isNullableUnion(stripFlexibleTypes: Boolean = true, forceStrip: Boolean = false)(using Context): Boolean = { + val stripped = self.stripNull(stripFlexibleTypes, forceStrip) stripped ne self } end extension diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 103687abdbff..040a742e0094 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -235,6 +235,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case CannotInstantiateQuotedTypeVarID // errorNumber: 219 case DefaultShadowsGivenID // errorNumber: 220 case RecurseWithDefaultID // errorNumber: 221 + case MatchCaseUnnecessaryOrNullID // errorNumber: 222 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 880c8add64cf..f283cf134689 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -962,6 +962,12 @@ extends PatternMatchMsg(MatchCaseOnlyNullWarningID) { def explain(using Context) = "" } +class MatchCaseUnnecessaryNullable(tp: Type)(using Context) +extends PatternMatchMsg(MatchCaseUnnecessaryOrNullID) { + def msg(using Context) = i"""$tp is nullable, but will not be matched by ${hl("null")}.""" + def explain(using Context) = "" +} + class MatchableWarning(tp: Type, pattern: Boolean)(using Context) extends TypeMsg(MatchableWarningID) { def msg(using Context) = diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index a8c8ec8ce1d8..4ed5cb988d69 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -16,6 +16,7 @@ import reporting.* import config.Printers.{ transforms => debug } import patmat.Typ +import dotty.tools.dotc.core.NullOpsDecorator.stripNull import dotty.tools.dotc.util.SrcPos /** This transform normalizes type tests and type casts, @@ -323,7 +324,7 @@ object TypeTestsCasts { * The transform happens before erasure of `testType`, thus cannot be merged * with `transformIsInstanceOf`, which depends on erased type of `testType`. */ - def transformTypeTest(expr: Tree, testType: Type, flagUnrelated: Boolean): Tree = testType.dealias match { + def transformTypeTest(expr: Tree, testType: Type, flagUnrelated: Boolean): Tree = testType.dealias.stripNull(stripFlexibleTypes = false, forceStrip = true) match { case tref: TermRef if tref.symbol == defn.EmptyTupleModule => ref(defn.RuntimeTuples_isInstanceOfEmptyTuple).appliedTo(expr) case _: SingletonType => diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 82e69ccd072e..541b1733e7a8 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -926,8 +926,38 @@ object SpaceEngine { cases match case Nil => case (c @ CaseDef(pat, _, _)) :: rest => + def checkForUnnecessaryNullable(p: Tree): Unit = p match { + case Bind(_, body) => checkForUnnecessaryNullable(body) + case Typed(expr, tpt) => + if tpt.tpe.isNullableUnion(stripFlexibleTypes = false, forceStrip = true) then + report.warning(MatchCaseUnnecessaryNullable(tpt.tpe), p.srcPos) + else + checkForUnnecessaryNullable(expr) + case UnApply(_, _, patterns) => + patterns.map(checkForUnnecessaryNullable) + case Alternative(patterns) => patterns.map(checkForUnnecessaryNullable) + case _ => + } + + def handlesNull(p: Tree): Boolean = p match { + case Literal(Constant(null)) => true + case Typed(expr, tpt) => tpt.tpe.isSingleton || handlesNull(expr) // assume all SingletonType's handle null (see redundant-null.scala) + case Bind(_, body) => handlesNull(body) + case Alternative(patterns) => patterns.exists(handlesNull) + case _ => false + } val curr = trace(i"project($pat)")(projectPat(pat)) - val covered = trace("covered")(simplify(intersect(curr, targetSpace))) + var covered = trace("covered")(simplify(intersect(curr, targetSpace))) + + checkForUnnecessaryNullable(pat) + + if !handlesNull(pat) && !isWildcardArg(pat) then + // Remove nullSpace from covered only if: + // 1. No pattern is the null constant (e.g., `case null =>` or `case ... | null | ... =>`) or + // has a SingletonType (e.g., `case _: n.type =>` where `val n = null`, see redundant-null.scala) in one of its pattern(s) + // 2. The pattern is a wildcard pattern. + covered = minus(covered, nullSpace) + val prev = trace("prev")(simplify(Or(prevs))) if prev == Empty && covered == Empty then // defer until a case is reachable recur(rest, prevs, pat :: deferred) diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala index 609dad894b6c..50d35ab6a1f2 100644 --- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala +++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala @@ -433,10 +433,10 @@ object Nullables: def computeAssignNullable()(using Context): tree.type = var nnInfo = tree.rhs.notNullInfo tree.lhs match - case TrackedRef(ref) if ctx.explicitNulls && ref.isNullableUnion => + case TrackedRef(ref) if ctx.explicitNulls && ref.isNullableUnion() => nnInfo = nnInfo.seq: val rhstp = tree.rhs.typeOpt - if rhstp.isNullType || rhstp.isNullableUnion then + if rhstp.isNullType || rhstp.isNullableUnion() then // If the type of rhs is nullable (`T|Null` or `Null`), then the nullability of the // lhs variable is no longer trackable. We don't need to check whether the type `T` // is correct here, as typer will check it. diff --git a/tests/explicit-nulls/warn/i23243.scala b/tests/explicit-nulls/warn/i23243.scala new file mode 100644 index 000000000000..5ff05a17c8a0 --- /dev/null +++ b/tests/explicit-nulls/warn/i23243.scala @@ -0,0 +1,18 @@ +object Extractor: + def unapply(s: String|Null): Boolean = true + +class A + +def main = + ("foo": (A|String)) match + case Extractor() => println("foo matched") // warn: String | Null is nullable + case _ => println("foo didn't match") + val s: Some[Any] = Some(5) + + s match { + case Some(s: (String | Null)) => // warn: String | Null is nullable + case Some(Some(s: (String | Null))) => // warn: String | Null is nullable + case _: s.type => + case Some(s: Int) => + case _ => + } diff --git a/tests/neg/i23243a.scala b/tests/neg/i23243a.scala new file mode 100644 index 000000000000..c6330e49c17e --- /dev/null +++ b/tests/neg/i23243a.scala @@ -0,0 +1,8 @@ +def main = { + ("foo": String) match { + case a: (String | Nothing) => // error + println("bar") + case _ => + println("foo") + } +} diff --git a/tests/neg/i4004.scala b/tests/neg/i4004.scala index bf757a0863a7..37b55891fd83 100644 --- a/tests/neg/i4004.scala +++ b/tests/neg/i4004.scala @@ -1,13 +1,8 @@ @main def Test = - "a".isInstanceOf[Null] // error - null.isInstanceOf[Null] // error - "a".isInstanceOf[Nothing] // error - "a".isInstanceOf[Singleton] // error "a" match case _: Null => () // error case _: Nothing => () // error - case _: Singleton => () // error case _ => () null match diff --git a/tests/neg/i4004a.scala b/tests/neg/i4004a.scala new file mode 100644 index 000000000000..10201409c908 --- /dev/null +++ b/tests/neg/i4004a.scala @@ -0,0 +1,10 @@ +@main def Test = + "a".isInstanceOf[Null] // error + null.isInstanceOf[Null] // error + "a".isInstanceOf[Nothing] // error + "a".isInstanceOf[Singleton] // error + + "a" match { + case _: Singleton => () // error + case _ => () + } diff --git a/tests/warn/i23243.check b/tests/warn/i23243.check new file mode 100644 index 000000000000..5cbc8538a410 --- /dev/null +++ b/tests/warn/i23243.check @@ -0,0 +1,20 @@ +-- [E222] Pattern Match Warning: tests/warn/i23243.scala:8:9 ----------------------------------------------------------- +8 | case Extractor() => println("foo matched") // warn: String | Null is nullable + | ^^^^^^^^^^^ + | String | Null is nullable, but will not be matched by null. +-- [E222] Pattern Match Warning: tests/warn/i23243.scala:13:17 --------------------------------------------------------- +13 | case Some(s: (String | Null)) => // warn: String | Null is nullable + | ^^^^^^^^^^^^^^^ + | String | Null is nullable, but will not be matched by null. +-- [E222] Pattern Match Warning: tests/warn/i23243.scala:14:22 --------------------------------------------------------- +14 | case Some(Some(s: (String | Null))) => // warn: String | Null is nullable + | ^^^^^^^^^^^^^^^ + | String | Null is nullable, but will not be matched by null. +-- [E222] Pattern Match Warning: tests/warn/i23243.scala:17:22 --------------------------------------------------------- +17 | case (null | Some(_: (Int | Null)))| Some(_: (String | Null)) => // warn: Int | Null is nullable // warn: String | Null is nullable + | ^^^^^^^^^^^^^^^ + | Int | Null is nullable, but will not be matched by null. +-- [E222] Pattern Match Warning: tests/warn/i23243.scala:17:46 --------------------------------------------------------- +17 | case (null | Some(_: (Int | Null)))| Some(_: (String | Null)) => // warn: Int | Null is nullable // warn: String | Null is nullable + | ^^^^^^^^^^^^^^^^^^ + | String | Null is nullable, but will not be matched by null. diff --git a/tests/warn/i23243.scala b/tests/warn/i23243.scala new file mode 100644 index 000000000000..21cadd057b8e --- /dev/null +++ b/tests/warn/i23243.scala @@ -0,0 +1,19 @@ +object Extractor: + def unapply(s: String|Null): Boolean = true + +class A + +def main = + ("foo": (A|String)) match + case Extractor() => println("foo matched") // warn: String | Null is nullable + case _ => println("foo didn't match") + val s: Any = 5 + + s match { + case Some(s: (String | Null)) => // warn: String | Null is nullable + case Some(Some(s: (String | Null))) => // warn: String | Null is nullable + case Some(null) => + case _: s.type => + case (null | Some(_: (Int | Null)))| Some(_: (String | Null)) => // warn: Int | Null is nullable // warn: String | Null is nullable + case _ => + }