Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
strelec opened this issue Jul 13, 2022 · 10 comments · Fixed by #19432
Closed

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

strelec opened this issue Jul 13, 2022 · 10 comments · Fixed by #19432

Comments

@strelec
Copy link

strelec commented Jul 13, 2022

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 strelec added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jul 13, 2022
@som-snytt
Copy link
Contributor

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>

@dwijnand dwijnand added area:gadt stat:needs triage Every issue needs to have an "area" and "itype" label area:pattern-matching and removed stat:needs triage Every issue needs to have an "area" and "itype" label area:gadt labels Jul 13, 2022
@dwijnand
Copy link
Member

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

@strelec
Copy link
Author

strelec commented Jul 13, 2022

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) =>

@dwijnand
Copy link
Member

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.

@som-snytt
Copy link
Contributor

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 ??

@strelec
Copy link
Author

strelec commented Sep 9, 2022

Still a problem with Scala 3.2.0

@bplommer
Copy link
Contributor

bplommer commented Jan 7, 2024

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

@agaro1121
Copy link

Is there a workaround for this in the meantime?

nicolasstucki added a commit that referenced this issue Jan 15, 2024
Closes #15661

This is broken in 3.2.0-RC1, but fixed in main as well as 3.3.0-RC1.
@strelec
Copy link
Author

strelec commented Jan 16, 2024

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

@dwijnand
Copy link
Member

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

@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants