From 4f775fab0f16a27d6be276dde0834cd286d8b506 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Thu, 17 Oct 2024 15:00:04 -0400 Subject: [PATCH] Implement change and add test cases --- .../tools/dotc/transform/patmat/Space.scala | 82 ++++++++----------- .../pos/interop-constructor.scala | 2 +- tests/explicit-nulls/warn/i21577.check | 34 ++++++-- tests/explicit-nulls/warn/i21577.scala | 48 +++++++---- tests/explicit-nulls/warn/interop.check | 8 ++ tests/explicit-nulls/warn/interop/J.java | 6 ++ tests/explicit-nulls/warn/interop/S.scala | 10 +++ 7 files changed, 114 insertions(+), 76 deletions(-) create mode 100644 tests/explicit-nulls/warn/interop.check create mode 100644 tests/explicit-nulls/warn/interop/J.java create mode 100644 tests/explicit-nulls/warn/interop/S.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index deaf285c79cc..c7b304e6caf6 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -14,6 +14,7 @@ import typer.*, Applications.*, Inferencing.*, ProtoTypes.* import util.* import scala.annotation.internal.sharable +import scala.annotation.tailrec import scala.collection.mutable import SpaceEngine.* @@ -696,7 +697,7 @@ object SpaceEngine { else NoType }.filter(_.exists) parts - + case tp: FlexibleType => List(tp.underlying, ConstantType(Constant(null))) case _ => ListOfNoType end rec @@ -876,6 +877,7 @@ object SpaceEngine { case tp: SingletonType => toUnderlying(tp.underlying) case tp: ExprType => toUnderlying(tp.resultType) case AnnotatedType(tp, annot) => AnnotatedType(toUnderlying(tp), annot) + case tp: FlexibleType => tp.derivedFlexibleType(toUnderlying(tp.underlying)) case _ => tp }) @@ -910,58 +912,40 @@ object SpaceEngine { && !sel.tpe.widen.isRef(defn.QuotedExprClass) && !sel.tpe.widen.isRef(defn.QuotedTypeClass) - def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)") { - val cases = m.cases.toIndexedSeq - + def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)"): val selTyp = toUnderlying(m.selector.tpe).dealias - - val isNullable = selTyp.classSymbol.isNullableClass - val targetSpace = trace(i"targetSpace($selTyp)")(if isNullable + val isNullable = selTyp.isInstanceOf[FlexibleType] || selTyp.classSymbol.isNullableClass + val targetSpace = trace(i"targetSpace($selTyp)"): + if isNullable && !ctx.mode.is(Mode.SafeNulls) then project(OrType(selTyp, ConstantType(Constant(null)), soft = false)) else project(selTyp) - ) - - var i = 0 - val len = cases.length - var prevs = List.empty[Space] - var deferred = List.empty[Tree] - - while (i < len) { - val CaseDef(pat, guard, _) = cases(i) - val curr = trace(i"project($pat)")(project(pat)) - - val covered = trace("covered")(simplify(intersect(curr, targetSpace))) - - val prev = trace("prev")(simplify(Or(prevs))) - - if prev == Empty && covered == Empty then // defer until a case is reachable - deferred ::= pat - else { - for (pat <- deferred.reverseIterator) - report.warning(MatchCaseUnreachable(), pat.srcPos) - if pat != EmptyTree // rethrow case of catch uses EmptyTree - && !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase - && isSubspace(covered, Or(List(prev, Typ(defn.NullType)))) // for when Null is not subtype of AnyRef under explicit nulls - then { - val nullOnly = - (isNullable || (defn.NullType <:< selTyp)) - && i == len - 1 - && isWildcardArg(pat) - if nullOnly then { - report.warning(MatchCaseOnlyNullWarning(), pat.srcPos) - } else if isSubspace(covered, prev) then { - report.warning(MatchCaseUnreachable(), pat.srcPos) - } - } - deferred = Nil - } - - // in redundancy check, take guard as false in order to soundly approximate - prevs ::= (if guard.isEmpty then covered else Empty) - i += 1 - } - } + @tailrec def recur(cases: List[CaseDef], prevs: List[Space], deferred: List[Tree]): Unit = + cases match + case Nil => + case CaseDef(pat, guard, _) :: rest => + val curr = trace(i"project($pat)")(project(pat)) + val covered = trace("covered")(simplify(intersect(curr, targetSpace))) + val prev = trace("prev")(simplify(Or(prevs))) + if prev == Empty && covered == Empty then // defer until a case is reachable + recur(rest, prevs, pat :: deferred) + else + for pat <- deferred.reverseIterator + do report.warning(MatchCaseUnreachable(), pat.srcPos) + + if pat != EmptyTree // rethrow case of catch uses EmptyTree + && !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase + && isSubspace(covered, prev) + then + val nullOnly = isNullable && rest.isEmpty && isWildcardArg(pat) + val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable() + report.warning(msg, pat.srcPos) + + val newPrev = if guard.isEmpty then covered :: prevs else prevs + recur(rest, newPrev, Nil) + + recur(m.cases, Nil, Nil) + end checkReachability def checkMatch(m: Match)(using Context): Unit = if exhaustivityCheckable(m.selector) then checkExhaustivity(m) diff --git a/tests/explicit-nulls/pos/interop-constructor.scala b/tests/explicit-nulls/pos/interop-constructor.scala index f222d24b0919..4ebfaa752b3a 100644 --- a/tests/explicit-nulls/pos/interop-constructor.scala +++ b/tests/explicit-nulls/pos/interop-constructor.scala @@ -1,4 +1,4 @@ -// Test that constructors have a non-nullab.e return type. +// Test that constructors have a non-nullable return type. class Foo { val x: java.lang.String = new java.lang.String() diff --git a/tests/explicit-nulls/warn/i21577.check b/tests/explicit-nulls/warn/i21577.check index 944b2ccab3c9..acedd7a9c713 100644 --- a/tests/explicit-nulls/warn/i21577.check +++ b/tests/explicit-nulls/warn/i21577.check @@ -1,12 +1,28 @@ -- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:5:9 -------------------------------------------- -5 | case _ => println(2) // warn +5 | case _ => // warn | ^ | Unreachable case except for null (if this is intentional, consider writing case null => instead). --- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:11 ------------------------------------------ -12 | case _ => println(2) // warn - | ^ - | Unreachable case except for null (if this is intentional, consider writing case null => instead). --- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:18:11 ------------------------------------------ -18 | case _ => println(2) // warn - | ^ - | Unreachable case except for null (if this is intentional, consider writing case null => instead). +-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:9 ------------------------------------------- +12 | case _ => // warn + | ^ + | Unreachable case except for null (if this is intentional, consider writing case null => instead). +-- [E030] Match case Unreachable Warning: tests/explicit-nulls/warn/i21577.scala:20:7 ---------------------------------- +20 | case _ => // warn + | ^ + | Unreachable case +-- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:29:27 ----------------------------- +29 |def f7(s: String | Null) = s match // warn + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: _: Null + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:36:33 ----------------------------- +36 |def f9(s: String | Int | Null) = s match // warn + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: _: Int + | + | longer explanation available when compiling with `-explain` diff --git a/tests/explicit-nulls/warn/i21577.scala b/tests/explicit-nulls/warn/i21577.scala index d5da1403deea..67da6068f22c 100644 --- a/tests/explicit-nulls/warn/i21577.scala +++ b/tests/explicit-nulls/warn/i21577.scala @@ -1,24 +1,38 @@ def f(s: String) = val s2 = s.trim() s2 match - case s3: String => println(1) - case _ => println(2) // warn + case s3: String => + case _ => // warn def f2(s: String | Null) = val s2 = s.nn.trim() - s2 match - case s3: String => println(1) - case _ => println(2) // warn - -def f3(s: String | Null) = - val s2 = s - s2 match - case s3: String => println(1) - case _ => println(2) // warn - -def f4(s: String | Int) = - val s2 = s - s2 match - case s3: String => println(1) - case _ => println(2) \ No newline at end of file + s2 match + case s3: String => + case _ => // warn + +def f3(s: String | Null) = s match + case s2: String => + case _ => + +def f5(s: String) = s match + case _: String => + case _ => // warn + +def f6(s: String) = s.trim() match + case _: String => + case null => + +def f61(s: String) = s.trim() match + case _: String => + +def f7(s: String | Null) = s match // warn + case _: String => + +def f8(s: String | Null) = s match + case _: String => + case null => + +def f9(s: String | Int | Null) = s match // warn + case _: String => + case null => \ No newline at end of file diff --git a/tests/explicit-nulls/warn/interop.check b/tests/explicit-nulls/warn/interop.check new file mode 100644 index 000000000000..0afc1dc0a3cb --- /dev/null +++ b/tests/explicit-nulls/warn/interop.check @@ -0,0 +1,8 @@ +-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/interop/S.scala:8:11 ---------------------------------------- +8 | case _ => // warn + | ^ + | Unreachable case except for null (if this is intentional, consider writing case null => instead). +-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/interop/S.scala:9:9 ----------------------------------------- +9 | case _ => println(2) // warn + | ^ + | Unreachable case except for null (if this is intentional, consider writing case null => instead). diff --git a/tests/explicit-nulls/warn/interop/J.java b/tests/explicit-nulls/warn/interop/J.java new file mode 100644 index 000000000000..f81cf685b9a9 --- /dev/null +++ b/tests/explicit-nulls/warn/interop/J.java @@ -0,0 +1,6 @@ +import java.util.ArrayList; + +class J { + ArrayList> foo(String x) { return null; } + static String fooStatic(String x) { return null; } +} diff --git a/tests/explicit-nulls/warn/interop/S.scala b/tests/explicit-nulls/warn/interop/S.scala new file mode 100644 index 000000000000..57beebe4eb76 --- /dev/null +++ b/tests/explicit-nulls/warn/interop/S.scala @@ -0,0 +1,10 @@ +import java.util.ArrayList +def f() = + val j = new J() + val s2 = j.foo(null) + s2 match + case s3: ArrayList[ArrayList[String]] => s3.get(0) match + case _: ArrayList[_] => + case _ => // warn + case _ => println(2) // warn +