scala/scala-parser-combinators

Incorrect match inexhaustive warnings of `NoSuccess` result.

counter2015 opened this issue · 6 comments

In scala 2.13.5, the match expression will raise inexhaustive warning for following code

// warning: match may not be exhaustive. It would fail on the following inputs: Error(_, _), Failure(_, _)
parse(freq, "johnny 121") match {
            case Success(matched,_) => println(matched)
            case NoSuccess(msg,_) => println(s"NoSuccess: $msg")
          }

It looks like it's a problem here

object NoSuccess {
def unapply[T](x: ParseResult[T]) = x match {
case Failure(msg, next) => Some((msg, next))
case Error(msg, next) => Some((msg, next))
case _ => None
}
}

An alternative fix is change it to

  object NoSuccess {
    def unapply(x: NoSuccess) = x match {
      case Failure(msg, next)   => Some((msg, next))
      case Error(msg, next)     => Some((msg, next))
    }
  }

And then may also need to revert commit c1fbc3c

But it will fail binary compatibility check.

see also: scala/bug#12384

Will it actually fail the binary compatibility check? Why can't we refine the return type from Option to Some?

Will it actually fail the binary compatibility check?

@joroKr21 It can be compiled, but failed test.

[error] scala-parser-combinators: Failed binary compatibility check against org.scala-lang.modules:scala-parser-combinators_2.13:1.2.0-RC1! Found 1 potential problems
[error]  * method unapply(scala.util.parsing.combinator.Parsers#ParseResult)scala.Option in object scala.util.parsing.combinator.Parsers#NoSuccess's type is different in current version, where it is (scala.util.parsing.combinator.Parsers#NoSuccess)scala.Some instead of (scala.util.parsing.combinator.Parsers#ParseResult)scala.Option
[error]    filter with: ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.util.parsing.combinator.Parsers#NoSuccess.unapply")
[error] scala-parser-combinators: Failed binary compatibility check against org.scala-lang.modules:scala-parser-combinators_sjs1_2.13:1.2.0-RC1! Found 1 potential problems
[error]  * method unapply(scala.util.parsing.combinator.Parsers#ParseResult)scala.Option in object scala.util.parsing.combinator.Parsers#NoSuccess's type is different in current version, where it is (scala.util.parsing.combinator.Parsers#NoSuccess)scala.Some instead of (scala.util.parsing.combinator.Parsers#ParseResult)scala.Option
[error]    filter with: ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.util.parsing.combinator.Parsers#NoSuccess.unapply")
[info] Fast optimizing /Users/counter/dev/scala-parser-combinators/js/target/scala-2.13/scala-parser-combinators-test-fastopt
[info] Passed: Total 59, Failed 0, Errors 0, Passed 59
[info] Passed: Total 59, Failed 0, Errors 0, Passed 59
[error] stack trace is suppressed; run last parserCombinatorsJVM / mimaReportBinaryIssues for the full output
[error] stack trace is suppressed; run last parserCombinatorsJS / mimaReportBinaryIssues for the full output
[error] (parserCombinatorsJVM / mimaReportBinaryIssues) Failed binary compatibility check against org.scala-lang.modules:scala-parser-combinators_2.13:1.2.0-RC1! Found 1 potential problems
[error] (parserCombinatorsJS / mimaReportBinaryIssues) Failed binary compatibility check against org.scala-lang.modules:scala-parser-combinators_sjs1_2.13:1.2.0-RC1! Found 1 potential problems

Why can't we refine the return type from Option to Some?

I guess it's because Some[T] <: Option[T]

I guess it's because Some[T] <: Option[T]

Ah no I think the problem is the change of the parameter from ParseResult to NoSuccess.
I didn't notice it before. We should be able to make the return type more specific, but not the parameter.

Note that we aim to release 1.2.0 not too long after Scala 3.0.0 final comes out, so time is starting to run a bit short if anyone wants to submit a binary incompatible change.

so time is starting to run a bit short if anyone wants to submit a binary incompatible change.

Are you saying that's an option?

The 1.2.x series is still in RCs, so sure.