Unions of Singletons unreachable literals not being checked in pattern match.

[Swoorup]

The pattern match on the lhs side doesn't exhaustively check if the pattern is of valid type for singletons of union.

In this example both 240 and 4H are not a valid subtype of both TimeframeN and Timeframe respectively, but compiler appears to compile without reporting type mismatch errors. It appears as if union type is widen'ed prematurely.

Compiler version

3.0.1-RC1

Minimized code

import scala.language.implicitConversions

type Timeframe = "1m" | "2m" | "1H"
type TimeframeN = 1 | 2 | 60

def manualConvertToN(tf: Timeframe): TimeframeN = tf match
  case "1m" => 1
  case "2m" => 2
  case "1H" => 60
  case "4H" => ??? // incorrect but compiles

given Conversion[Timeframe, TimeframeN] = 
  case "1m" => 1
  case "2m" => 2
  case "1H" => 60
  case "4H" => ??? // incorrect but compiles

given Conversion[TimeframeN, Timeframe] = 
  case 1 => "1m"
  case 2 => "2m"
  case 60 => "1H"
  case 240 => ??? // incorrect but compiles

Output

No errors on compilation

Expectation

Type mismatch errors. Hypothetical sample:

[error] -- [E007] Type Mismatch Error: /Users/swoorup.joshi/playground/test/src/main/scala/Main.scala:18:15
[error] 18 |  case "4H" => ???
[error]    |        ^^^
[error]    |        Found:    ("4H" : String)
[error]    |        Required: Timeframe

[error] -- [E007] Type Mismatch Error: /Users/swoorup.joshi/playground/test/src/main/scala/Main.scala:24:15
[error] 24 |  case "4H" => ???
[error]    |        ^^^
[error]    |        Found:    ("4H" : String)
[error]    |        Required: Timeframe

[error] -- [E007] Type Mismatch Error: /Users/swoorup.joshi/playground/test/src/main/scala/Main.scala:30:15
[error] 30 |  case 240 => ???
[error]    |       ^^^
[error]    |       Found:    (240 : Int)
[error]    |       Required: TimeframeN

Note that this doesn't happen if the returning subtype is invalid. In such cases, compiler reports a type mismatch error correctly.

[sinanspd]

EDIT: Never mind. Looks like this is in fact something the union type implementation supports. The following works:

type EX = 1 | "hello"
def f(e: EX) = e
f(1)  // 1
f(2)
1 |f(2)
  |  ^
  |  Found:    (2 : Int)
  |  Required: EX

Maybe I have missed this feature and this is actually expected to work but can the compiler expected to interpret values as types here? Correct me if I am wrong but this seems like a use case for Enum as you aren't actually restricting the subtypes but the set of values an instance of a type can have.

The following for example works

case class OneM()
case class TwoM()
case class FourM()
type Timeframe = OneM | TwoM

def manual(tf: Timeframe): Int = tf match                                                                                                              
     case _ @ OneM() => 1
     case _ @ TwoM() => 2
     case _ @ FourM() => 4

4 |case _ @ FourM() => 4
  |         ^
  |this case is unreachable since type Timeframe and class FourM are unrelated

but it seems like you are expecting the compiler to generate a type from the concrete values and union them.

In my opinion if there is a line that shouldn't compile here, it's

type Timeframe = "1m" | "2m" | "1H"
type TimeframeN = 1 | 2 | 60

It might not be "widening" the union too early per se but simply fetching the type of the values and resulting in a String | String | String , which inevitably widens to String. While this would be valid behavior, it could be argued that it is not very intuitive

[romanowski]

It seems that exclusivity checks does not pick up a case where additional singletons are provided.

scala> def foo(a: 1 | 2) = a match       // Compiles fine                                                                                                               
     |   case 1 => true
     |   case 2 => false
     |   case 4 => ???                          
     | 
def foo(a: 1 | 2): Boolean

scala> def bar(a: 1 | 2) = a match                                                                                                                       
     |   case 1 => true
     |   case 2 => false
     |   case _ => ???
     | 
4 |  case _ => ???
  |       ^
  |       Unreachable case

scala> def baz(a: 1 | 2) = a match                                                                                                                       
     |   case 1 => true
     |   case 2 => false
     |   case a if a < 0 => ???
     | 
4 |  case a if a < 0 => ???
  |       ^
  |       Unreachable case

[SethTisue]

you don't even need to bring a union into it:

scala> def foo(a: 1) = a match { case 1 => 1; case 2 => 2 }
def foo(a: 1): Int
                                                                                                                        
scala> def foo(a: 1) = a match { case 1 => 1; case _ => 2 }
def foo(a: 1): Int

neither warns, but both should warn

I wondered if this was a degenerate case of the union case, but maybe it's actually an orthogonal bug on a different theme (namely, inappropriate widening of the scrutinee type); on that theme, see scala/bug#11603 / scala/scala#9209, which Dale fixed in Scala 2 in 2020

UPDATE: I have now opened #13342 on this.

[SethTisue]

in Space.scala,

        // `covered == Empty` may happen for primitive types with auto-conversion
        // see tests/patmat/reader.scala  tests/patmat/byte.scala
        if (covered == Empty && !isNullLit(pat)) covered = curr

commenting this line out causes at least one Krzysztof's test cases to progress, so this may be the key piece of code. it was added by Fengyun in #4253

That piece of code seems to be about making a match such as (99: Int) match { case 'c' => ... work, where the scrutinee type (Int) and the widened type of the pattern literal (Char) are actually unrelated types

in the Scala 2 spec, SLS 8.1.4 says:

A literal pattern L matches any value that is equal (in terms of ==) to the literal. The type of L must conform to the expected type of the pattern.

As far as I can tell, there was an oversight here where "must conform" should say "must weakly conform", and that's why it's legal to match with a Char literal even when the scrutinee is Int.

But in Scala 3, we don't have weak conformance anymore. So there needs to be some other justification for considering such a match to be reachable. The justification is provided by https://docs.scala-lang.org/scala3/reference/dropped-features/weak-conformance-spec.html , which says that such conversions as Char -> Int are applied to "the alternatives of a ... match expression"

Perhaps we can find a more targeted way to support auto-conversion that doesn't negatively impact other scenarios.

[Swoorup]

Also wondering why we can't make this an error instead of just warning. Imo the compiler should be guiding the user why its not possible not allow holes like this through?

[dwijnand]

Dead code, such as unreachable cases or even just unused imports, can be misleading and so are worth linting. But by themselves they aren't hazardous, and stopping compilation might prove very annoying.

[Swoorup]

I think warnings dead code and unused imports are fine and think it forms validation compilation, but for unreachable cases, invalid patterns it seems odd to have it as a warning, at least by default.

Same with match types.

type M[X <: String] = X match 
  case "SA" => Int
  case 12 => String

If some invalid construct compiles, most likely someone will use it in their codebase. What about by default it being an error and ability to use it as warning. I have some warnings in my codebase like obsolete that I tend to ignore, but usually cases like this slips through as well.

[dwijnand]

When configurable warnings gets merged (soon hopefully) then you should be able to promote a reachability warning into a reachability error.

Match types are different though, and I'm not too sure yet how much more worrisome unreachable cases are there. Same with in-exhaustive match types... 🤔

[SethTisue]

But in Scala 3, we don't have weak conformance anymore. So there needs to be some other justification for considering such a match to be reachable. The justification is provided by docs.scala-lang.org/scala3/reference/dropped-features/weak-conformance-spec.html

Note that that documentation doesn't actually directly cover this. It applies to numeric expressions, not numeric patterns. It does mention pattern matching, but that language is about the right-hand-sides of matches (expressions), not the left-hand sides (patterns).

We're doing something similar to numeric patterns, and in the context of reachability-and-exhaustiveness analysis only. Ultimately the justification for this isn't the doc I cited, the justification is that numeric types are spec'ed as comparing equal at runtime, despite being unrelated by subtyping.

(Regardless, in the hypothetical future Scala 3 spec, in the equivalent of SLS 8.1.4, maybe the "The type of L must conform to the expected type of the pattern" sentence should simply be omitted? It seems like warnings/linting territory to me, rather than something that needs to be spec'ed with an unequivocal word such as "must". 🤷 )

[SethTisue]

Dale and I attempted a fix on the draft PR #13290, but our efforts may have stalled; see our recent remarks there.