Skip to content
This repository was archived by the owner on Jan 24, 2025. It is now read-only.

Change Diff back to invariant TC #316

Closed
ghostbuster91 opened this issue Oct 28, 2021 · 0 comments · Fixed by #318
Closed

Change Diff back to invariant TC #316

ghostbuster91 opened this issue Oct 28, 2021 · 0 comments · Fixed by #318

Comments

@ghostbuster91
Copy link
Collaborator

ghostbuster91 commented Oct 28, 2021

Context

Diff was made a contravariant typeclass quite some time in #92 due to #17

It all boils down to that simple line of code:
Option("test") should matchTo(Some("a"))

Without that change it wouldn't compile because should expects Matcher[Option[String]] and it gets Matcher[Some[String]].
Matcher is a contravariant type which means that we have a reversed subtyping relation between the class and its type parameter. Given A <: B we have Matcher[B] <: Matcher[A].

Unfortunately, that decision turned out to be a blocker in the long run when migrating to scala3 (see scala/scala3#13146 and softwaremill/magnolia#336)

In general, I think that it makes more sens for Diff to be invariant because its instances are derived from the primitives up, which means that we first get to know how to compare Somes to later build on top of that a Diff for Options.

There is no need for a special relation between Diff[A] and Diff[B] where A <: B because when we have Diff[B] and some As these As can be lifted to B type.

What about scalatest

While these days we have other great test frameworks available, scalatest still remains one of the most popular.
Without a contravariance scalatest users would need to adapt their code-bases either by getting value out of option:

fooOpt.value should matchTo(fooExpected)

or by specifying types explicitly:

fooOpt should matchTo(Some(fooExpected): Option[String])

or by using Option constructor:

fooOpt should matchTo(Option(fooExpected))

Why can't we have invariant Diff instances and usability in the scalatest?

It seems that we can. In order to make our problematic line compile, we need to change the complex types around our values to be covariant. That way we will invert the relation and in the end get Matcher[A] <: Matcher[B] where A <: B. Obviously that won't be a scalatest matcher anymore.

The downside of that is that we need to override the should method, which means

  • the matcher won't work with must matchers
  • eventually we need to call the original should matcher which means that we need to have an additional dependency.

Because of that we should create new artifacts: diffx-scalatest-should and diffx-scalatest-must which will contain fixes for particular matching styles.

Update: Above solution turned out to be impossible because of ambiguity. Because of that I decided to implement a simpler solution which is to have a single-word matcher like shouldMatchTo and mustMatchTo.

This was referenced Oct 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant