improve error message for variance errors (in Scala 2 there is an explaining note and a hint to use +A)

[bjornregnell]

In Scala 3.0.0:

scala> case class Box[A](a: A)
// defined case class Box

scala> var xs = Vector.empty[Box[Any]]
var xs: Vector[Box[Any]] = Vector()

scala> def f[A](b: Box[A]) = xs = xs :+ b
1 |def f[A](b: Box[A]) = xs = xs :+ b
  |                                 ^
  |                                 Found:    (b : Box[A])
  |                                 Required: Box[Any]

In Scala 2 there is both a note and a hint:

Welcome to Scala 2.13.6 (OpenJDK 64-Bit Server VM, Java 11.0.11).
Type in expressions for evaluation. Or try :help.

scala> case class Box[A](a: A)
class Box

scala> var xs = Vector.empty[Box[Any]]
var xs: scala.collection.immutable.Vector[Box[Any]] = Vector()

scala> def f[A](b: Box[A]) = xs = xs :+ b
                                        ^
       error: type mismatch;
        found   : Box[A]
        required: Box[Any]
       Note: A <: Any, but class Box is invariant in type A.
       You may wish to define A as +A instead. (SLS 4.5)

It would be nice with something similar in Scala 3. (If the hint is helpful in most cases.)

[smarter]

The code responsible for this in Scala 2 is https://github.com/scala/scala/blob/f611f6c686bed09f1b8c67c47f5e88d122a6eec7/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala#L214-L223
One interesting special-case there is:

                // Don't suggest they change the class declaration if it's somewhere
                // under scala.* or defined in a java class, because attempting either
                // would be fruitless.

Which I think could be extended to not suggesting a class declaration change for any class not defined in the current package (or another package with a common prefix with the current package)

[bjornregnell]

It sounds great for the hint on +A solution only to be given when it is relevant! But the explanation that "A<: Any but class X[A] is invariant" should be helpful in any case? (However, the "SLS 4.5" thing in the Scala 2 message is not very apparent what it means.)

[smarter]

Yes, for things under scala. and java. the message becomes:

   Note: Int <: Any, but Java-defined trait List is invariant in type E.
  You may wish to investigate a wildcard type such as `_ <: Any`. (SLS 3.2.10)

[bjornregnell]

Cool! I definitely think such situated and helpful messages should be ported to dotty.

(Although the SLS acronym is too cryptic IMHO, at least it could be "See lang. spec. 3.2.10" or something, but I'm a bit doubtful to reference such a hard read, esp. in a non-googleable way. Perhaps better a short url to official tutorial material on scala-lang.org and cool if we could have stable urls for language concept definitions+explanations.)

[dwijnand]

Which I think could be extended to not suggesting a class declaration change for any class not defined in the current package (or another package with a common prefix with the current package)

I kind of like the current logic, as it allows for suggestions to happen across different OSS libraries.

[bjornregnell]

Yeah, issues and PR:s are welcome to OSS projects. But still we need to help users in practice with the intricacy of variance, which we can't expect that all have studied -- a helpful error message that 1) help to understand why this is an error 2) give one or more hints on what could be done 3) point to further reading.

(In general: These three things could actually be part of an endeavor to go through all errors to see if the messages could be more helpful. There has already been great improvement in formatting and contents of error messages in Scala 3, but maybe we eventually can take this to the next level and further advance explanation+solution hint+pointer.)

[dwijnand]

I think you misunderstood what I meant by the current logic: I was only speaking about the special case Guillaume was mentioning. As an example, I think it would be useful if fs2 developers saw that message about a class in cats, so potentially they could get cats to widen the variance.

[smarter]

I think fs2 developers already understand how variance work so it doesn't matter much if the message talks about +A or using wildcards, the target audience for this message is complete beginners we should help without confusing.

[dwijnand]

I'm not so sure, but ok, pick two other libraries and it might still be useful for that detail to cross the no-shared-package-prefix boundary, IMO.

[smarter]

Well if you implement this message, you get to choose what it says 😁