scala3 icon indicating copy to clipboard operation
scala3 copied to clipboard

Misleading warning "Unreachable case except for null" (and one more issue)

Open strelec opened this issue 3 years ago • 6 comments
trafficstars

Compiler version

Tested both 3.1.3 and 3.2.0-RC1

Minimized code

case class Composite[T](l: List[T], v: T)

def m(composite: Composite[_]): Unit =
  composite match {
    case Composite(l: List[Int], v: Int) => println(v)
    case _ => println("This is not Int")
  }

Output

Two bad warnings. First one is major, second one minor. Reporting both together, because I think they have the same root cause.

  1. Unreachable case except for null (if this is intentional, consider writing case null => instead).

Strangely, If I follow the advice and specify case null => I get a proper match may not be exhaustive. warning.

  1. the type test for List[Int] cannot be checked at runtime

Explanation: List[Int] does not need to be checked at runtime, because matching second parameter as Int fixes the possible type of the list.

Expectation

No warnings at all.

strelec avatar Jul 13 '22 00:07 strelec

I can't tell which is major or minor, but

scala> m(Composite("" :: 42 :: Nil, 27))
27

is an int and a list of whatever it infers these days.

scala> "" :: 42 :: Nil
val res0: List[Matchable] = List("", 42)

Here is a difference:

scala> case class Composite[T](l: List[T], v: T)
// defined case class Composite

scala> def m(composite: Composite[?]): Unit =
     |   composite match {
     |     case Composite(l: List[Int], v: Int) => println(v)
     |     case _ => println("This is not Int")
     |   }
     |
2 warnings found
def m(composite: Composite[?]): Unit
-- Unchecked Warning: -------------------------------------------------------------------------------------------------------
3 |    case Composite(l: List[Int], v: Int) => println(v)
  |                   ^
  |                   the type test for List[Int] cannot be checked at runtime
-- [E121] Pattern Match Warning: --------------------------------------------------------------------------------------------
4 |    case _ => println("This is not Int")
  |         ^
  |         Unreachable case except for null (if this is intentional, consider writing case null => instead).

scala> def m[A](composite: Composite[A]): Unit =
     |   composite match {
     |     case Composite(l: List[Int], v: Int) => println(v)
     |     case _ => println("This is not Int")
     |   }
     |
1 warning found
def m[A](composite: Composite[A]): Unit
-- Unchecked Warning: -------------------------------------------------------------------------------------------------------
3 |    case Composite(l: List[Int], v: Int) => println(v)
  |                   ^
  |                   the type test for List[Int] cannot be checked at runtime

scala>

som-snytt avatar Jul 13 '22 01:07 som-snytt

Yeah, only the null reachability lint is wrong. Even if T is invariant, you can't tie the type test of v to l's type:

scala> case class Composite[T](l: List[T], v: T)
Composite// defined case class Composite

scala> Composite[Any](List("foo", false), 1)
val res0: Composite[Any] = Composite(List(foo, false),1)

scala> res0.l.asInstanceOf[List[Int]].head + 1
java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap')
  at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99)
  ... 35 elided

dwijnand avatar Jul 13 '22 08:07 dwijnand

I see, then I just have to be explicit in the type parameter, to prevent Composite[Any], like so:

case Composite[Int](l, v) =>

Meaning this should not emit warning, right?

case Composite[Int](l: List[Int], v) =>

strelec avatar Jul 13 '22 09:07 strelec

No, there's nothing you can do. List is covariant, so anywhere it is, it could exist with a type argument that is a supertype of the precise type it was constructed with. Such as when it's a component of a Composite case class.

dwijnand avatar Jul 13 '22 13:07 dwijnand

The difference between ? and the type param makes me not want to use ?.

I wonder if anyone has a meme handy of The Riddler from the old Batman tv show, whose signature was a giant ??

som-snytt avatar Jul 13 '22 16:07 som-snytt

Still a problem with Scala 3.2.0

strelec avatar Sep 09 '22 00:09 strelec

I'm experiencing this in Scala 3.3.1 for a case where it wasn't occurring in 3.2.2.

bplommer avatar Jan 07 '24 16:01 bplommer

Is there a workaround for this in the meantime?

agaro1121 avatar Jan 10 '24 14:01 agaro1121

Interesting, so was I correct saying that Composite(l: List[Int], v: Int) does not need to check List[Int] at runtime?

strelec avatar Jan 16 '24 17:01 strelec

Interesting, so was I correct saying that Composite(l: List[Int], v: Int) does not need to check List[Int] at runtime?

No:

scala> case class Composite[T](l: List[T], v: T)
Composite// defined case class Composite

scala> Composite[Any](List("foo", false), 1)
val res0: Composite[Any] = Composite(List(foo, false),1)

scala> res0.l.asInstanceOf[List[Int]].head + 1
java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap')
  at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99)
  ... 35 elided

dwijnand avatar Jan 16 '24 18:01 dwijnand