"case BarWrapper(foo)" should fail if the type of foo is unrelated to the type of BarWrapper argument

[smarter]

This fails to compile, as expected:

class Foo

class Bar
case class BarWrapper(bar: Bar)

object Test {
  def test(barWrapper: BarWrapper): Unit = {
    val foo = new Foo

    barWrapper match {
      case BarWrapper(_: Foo) =>
      case _ =>
    }
  }
}

-- Error: try/casecomp0.scala --------------------------------------------------
11 |      case BarWrapper(_: Foo) =>
   |                         ^
   |                 will never match since `Foo` is not a subclass of `Bar`

But if you replace case BarWrapper(_: Foo) => by case BarWrapper(`foo`) =>, the file will compile without any error or warning in both scalac and dotty. I find this very concerning because it means that you cannot safely refactor any case class by changing the type of one of its parameter, for example in #1941 I replaced case class PostfixOp(od: Tree, op: Name) by case class PostfixOp(od: Tree, op: Ident), fixed the compilation errors, and expected everything to be fine. But the compiler failed to warn me about the now impossible pattern matches like case PostfixOp(_, nme.raw.STAR), so tests failed.

@odersky WDYT?

[DarkDimius]

But if you replace case BarWrapper(_: Foo) => by case BarWrapper(foo) =>

the pattern matcher will use equals here, not reference equality.
Ie, if foo has equals defined to always return true, it will match.
IE, though types are unrelated, code isn't statically unreachable.

[DarkDimius]

I think this is more related the universal-equlality than the pattern matching itself.

[smarter]

I don't think so, we are typing a value of type Foo with expected type Bar, this should fail.

SLS 8.1.8:

unapply's result type is Option[T], for some type T. In this case, the (only) argument pattern p_1 is typed in turn with expected type T

SLS 8.1.5:

A stable identifier pattern is a stable identifier r. The type of r must conform to the expected type of the pattern.

[DarkDimius]

Well, the pattern matching&typer doesn't implement it this way in neither dotty nor scalac:

def test(barWrapper: BarWrapper): Unit = {
      val foo: Foo = new Foo();
      {
        case <synthetic> val x1: BarWrapper = barWrapper;
        case6(){
          if (x1.ne(null))
            {
              <synthetic> val p2: Bar = x1.bar;
              if (foo.==(p2))
                matchEnd5(())
              else
                case7()
            }
          else
            case7()
        };
        case7(){
          matchEnd5(())
        };
        matchEnd5(x: Unit){
          x
        }
      }
    }

[smarter]

The spec says what should or should not typecheck, no matter how we implement it at runtime.

[DarkDimius]

I'm fine with this being a warning, but I don't think we should fail compilation.
There might be code out-there that uses this.

[smarter]

This can be simplified to:

class Foo
class Bar

object Test {
  def test(bar: Bar): Unit = {
    val foo = new Foo

    bar match {
      case `foo` =>
      case _ =>
    }
  }
}

There might be code out-there that uses this.

I would be very curious of what kind of code could take advantage of this, and could not be easily rewritten to not use this.

[DarkDimius]

https://issues.scala-lang.org/browse/SI-4577 - seems related

[DarkDimius]

in https://issues.scala-lang.org/browse/SI-4577 the behavior for singleton types was changed to use reference equality, but the stable identifier discussed here still uses equals.

ping @adriaanm

[smarter]

I think it's fine for stable identifiers match to use equals, that doesn't prevent us from disallowing the pattern match if the types are unrelated (as the spec says).

[retronym]

I tried to faithfully implement the spec in scalac in scala/scala#2742 but backed out. There is some discussion in http://issues.scala-lang.org/browse/SI-7655

[odersky]

We now don't get an error in the

case BarWrapper(_: Foo)

case. We get a warning instead:

12 |      case BarWrapper(_: Foo) =>
   |                      ^
   |    this case is unreachable since class Bar and class Foo are unrelated

For the case of

case BarWrapper(`foo`)

we don't get a warning. But if we declare Foo to be an Eq instance we do get an error:

17 |      case BarWrapper(`foo`) =>
   |                      ^^^^^
   |            Values of types Foo and Bar cannot be compared with == or !=

I think this is as it should be. Let multiversal equality handle non-sensical equality tests.

[som-snytt]

-- [E007] Type Mismatch Error: i1942.scala:13:22 -----------------------------------------------------------------------
13 |      case BarWrapper(`foo`) =>
   |                      ^^^^^
   |                      Found:    (foo : Foo)
   |                      Required: Bar
   |                      pattern type is incompatible with expected type

The other case is still a warning in 2.13.10 but errors in 3.2.1

-- Error: i1942.scala:12:22 --------------------------------------------------------------------------------------------
12 |      case BarWrapper(_: Foo) => // fruitless type test
   |                      ^
   |                      this case is unreachable since type Bar and class Foo are unrelated