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

Switch to Scala 3 universal equality for assertEquals() #223

Closed
wants to merge 1 commit into from

Conversation

olafurpg
Copy link
Member

Previously, MUnit had a subtyping constraint on assertEquals(a, b) so that it would fail to compile if a was not a subtype of b. This was a suboptimal solution because the compile error messages could become cryptic in some cases. Now, MUnit uses the new Eql[A, B] multiversal equality in Scala 3 (http://dotty.epfl.ch/docs/reference/contextual/multiversal-equality.html). For Scala 2, we drop the subtyping constraint so that it's no longer a compile error to compare unrelated types. If the values are different, then users will see a runtime error instead.

Fixes #189

Previously, MUnit had a subtyping constraint on `assertEquals(a, b)` so
that it would fail to compile if `a` was not a subtype of `b`. This was
a suboptimal solution because the compile error messages could become
cryptic in some cases. Now, MUnit uses the new `Eql[A, B]` universal
equality in Scala 3. For Scala 2, we drop the subtyping constraint so
that it's no longer a compile error to compare unrelated types. If the
values are different, then users will see a runtime error instead.
*
* http://dotty.epfl.ch/docs/reference/contextual/multiversal-equality.html
*/
sealed abstract class Eql[A, B]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc/ @OlivierBlanvillain @smarter what is the recommended Eql "polyfill" for Scala 2 in cross-built library APIs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the implementation in the docs

package scala
import annotation.implicitNotFound

@implicitNotFound("Values of types ${L} and ${R} cannot be compared with == or !=")
sealed trait Eql[-L, -R]

object Eql {
  object derived extends Eql[Any, Any]
}

We can probably copy paste this, and add implicit to the derived object. We can also probably drop the implicitNotFound annotation for Scala 2 (since it will never be triggered)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're the first person to do it so there's no recommendation yet :). Personally I think I would use separate source files for Scala 2 and 3 in the places where I would want to use Eql.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with a separate Compare[A, B] type-class that I use in both Scala 2 and Scala 3, and in Scala 3 I provide default instances based on Eql[A, B].

Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 just a suggestion about Eql for scala 2

@olafurpg
Copy link
Member Author

I should probably have marked this PR as a draft. I’m still experimenting with different alternatives to see if we can support custom equality type classes like cats.Eq

@olafurpg
Copy link
Member Author

Superseded by #225

@olafurpg olafurpg closed this Oct 19, 2020
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

Successfully merging this pull request may close these issues.

Wrong order of arguments for assertEquals
3 participants