Warn on adding a type constructor to a sum type

[dwijnand]

Adding a type constructor to a sum type is a separate compilation incompatible change because it can fail pattern matching.

V1

sealed trait Foo
case object Bar extends Foo

object Lib {
  def foo: Foo = Bar
}

object Main extends App {
  Lib.foo match {
    case Bar => println("Bar")
  }
}

V2

 sealed trait Foo
 case object Bar extends Foo
+case object Baz extends Foo
 
 object Lib {
-  def foo: Foo = Bar
+  def foo: Foo = Baz
 }

Problem

$ sbt
> mimaReportBinaryIssues
[info] [...] found 0 potential binary incompatibilities while checking against [...]

$ scala Main
scala.MatchError: Baz (of class Baz$)
	at Main.main(Main.scala)

Expectation

MiMa emits a warning.

[dwijnand]

Of note, this issue is different from the rest of MiMa because it's about avoiding a match error rather than a link error. (Maybe there are other's like it?)

So I gathered some feedback on this (please correct any inaccuracies):

• @adriaanm agreed to the issue but given the difference wasn't sure it was MiMa's domain
• @SethTisue agreed MiMa should warn about this (having agreed it's slightly different)
• in https://github.com/akka/akka/pull/25854#issuecomment-435788578 @raboof agreed MiMa should warn about, but that it's more subjective (and hard) if it should be ignored (cf https://github.com/lightbend/migration-manager/issues/1 about BC issue severity)

[mdedetrich]

My 2 cents on this, I see this as out of scope of mima's responsibilities. To me, mima has always been about diagnosing potential linking errors and not compilation/source errors. Accepting this as mima's responsibility also opens up a massive can of worms as it makes it even more difficult for library maintainers to not do "breaking" changes since there are quite a few changes which are source breaking but not ABI breaking (i.e. changing the parameter to a method can be source breaking due to named parameters but is not ABI breaking).

Not against adding this as a separate warning but it should be treated very differently to the current mima reporting, i.e. it shouldn't be part of mimaBinaryIssueFilters but some other setting.

[armanbilge]

To me, mima has always been about diagnosing potential linking errors and not compilation/source errors.

Agree. This is a grey area, since a MatchError is a runtime (not compile-time) error, but not a linking error.

[som-snytt]

My ha'penny is that MiMa should warn when the types indicate recompilation of clients. MatchError means you missed the exhaustivity check at compile time. The linked issue (thanks) is also MiMa's fault for not warning. (Sorry, MiMa.)

[ghostbuster91]

I agree that this is something different as MatchError is not a linking error. But this is a very technical perspective. From the functional perspective mima promises (or should promise imo) safe evolution of the api, that won't break clients in runtime.

Because of that I would be happy to see this check being added to mima. After all what other alternative do we have? Creating a new project for checking this type of incompatibilities?