Skip to content
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

"type mismatch" error since Scala 2.13.0-M4 #10960

Closed
xuwei-k opened this issue Jun 26, 2018 · 6 comments
Closed

"type mismatch" error since Scala 2.13.0-M4 #10960

xuwei-k opened this issue Jun 26, 2018 · 6 comments

Comments

@xuwei-k
Copy link

xuwei-k commented Jun 26, 2018

bug or by design?

  • compile failed with Scala 2.13.0-M4, 2.13.0-pre-1e904d7 (latest 2018-06-26)
  • compile succeed with Scala 2.13.0-M3, 2.12.6, 2.11.12, 2.10.7
package example

sealed trait Maybe[+A] {
  def map[B](f: A => B): Maybe[B] = this match {
    case Just(a) => Just(f(a))
    case a @ Empty => a
  }
}

case class Just[A](value: A) extends Maybe[A]
case object Empty extends Maybe[Nothing]
Maybe.scala:6:23: type mismatch;
[error]  found   : example.Maybe[A]
[error]  required: example.Maybe[B]
[error]     case a @ Empty => a
[error]                       ^
[error] one error found
@som-snytt
Copy link

Failed previously under -Xfuture. The future is now. The PR is 6502 like the chip.

The correct idiom is now case a: Empty.type => a. Otherwise, all you got was an equals test.

Also, -Xlint would have told you:

warning: The value matched by Empty is bound to a, which may be used under the
unsound assumption that it has type example.Empty.type, whereas we can only safely
count on it having type example.Maybe[A], as the pattern is matched using `==` (see scala/bug#1503).
    case a @ Empty => a
             ^
one warning found

I know everyone has -Xlint turned on all the time.

@eed3si9n
Copy link
Member

What should be the recommend rewrite here?

package example

sealed trait Maybe[+A] {
  def map[B](f: A => B): Maybe[B] = this match {
    case Just(a)       => Just(f(a))
    case a: Empty.type => a
  }
}

case class Just[A](value: A) extends Maybe[A]
case object Empty extends Maybe[Nothing]

or

sealed trait Maybe[+A] {
  def map[B](f: A => B): Maybe[B] = this match {
    case Just(a) => Just(f(a))
    case Empty   => Empty
  }
}

case class Just[A](value: A) extends Maybe[A]
case object Empty extends Maybe[Nothing]

?

@som-snytt
Copy link

The first idiom is eq and a check cast, which I think is the usual intuition for matching a singleton. The second idiom is equals, with getstatic module load every time you mention Empty.

Probably scalafix should generate case empty: Empty.type =>.

Now I don't remember when I started the quinoa.

@sjrd
Copy link
Member

sjrd commented Jun 27, 2018

A scalafix rewrite should preserve the meaning of the existing code if at all possible. In this case, this means:

case Empty =>
  val a = this.asInstanceOf[this.type with Empty.type]
  a

Yes, it's unsound, which precisely matches how unsound the previous meaning was.

@eed3si9n
Copy link
Member

eed3si9n commented Jun 27, 2018

When I asked "What should be the recommend rewrite", I didn't necessarily mean the mechanical ones, but in general, I feel like we should try to conserve the likely-intent of the code, not the surprising behavior of the code.

The most straightforward read of the code

  this match {
    //... 
    case a @ Empty => a
  }

is that the author tried to pattern match on this with Empty, and use the value typed to it. The fact that it uses == and that == doesn't always mean equality is a surprising pitfall.

The point of #1503 is to prevent the users from making unsound assumption. Re-injecting cast would undo the guard. Maybe the best we can do is resurrect the warning message "The value matched by Empty is bound to a" as the error message when types do not match up (not sure it's possible), and suggest case a: Empty.type as the eq way.

A hypothetical error message:

Maybe.scala:6:23: type mismatch. the bound value `a` matched by Empty is typed to example.Maybe[A]
because the pattern is matched using `==`: if example.Empty.type is desired, use `case a: Empty.type` instead
[error]  found   : example.Maybe[A]
[error]  required: example.Maybe[B]
[error]     case a @ Empty => a
[error]                       ^
[error] one error found

@som-snytt
Copy link

I was going to say that if Scalafix doesn't fix something, it's not doing its job.

But I like the idea of a lint option to specify whether you prefer improved behavior or reproduced behavior.

English "fix" has the primary meaning to set in place. The sense of repair means you have a screw loose, that requires fastening. Nicely, it's related to dike, which is also multivalent, and ditch, Teich.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants