scala-parser-combinators icon indicating copy to clipboard operation
scala-parser-combinators copied to clipboard

Incorrect match inexhaustive warnings of `NoSuccess` result.

Open counter2015 opened this issue 4 years ago • 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

https://github.com/scala/scala-parser-combinators/blob/0cb71847d7351794a86a1610a7b7c92d94e3d698/shared/src/main/scala/scala/util/parsing/combinator/Parsers.scala#L181-L187

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

counter2015 avatar Apr 27 '21 03:04 counter2015

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

joroKr21 avatar Apr 27 '21 03:04 joroKr21

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]

counter2015 avatar Apr 27 '21 03:04 counter2015

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.

joroKr21 avatar Apr 27 '21 03:04 joroKr21

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.

SethTisue avatar Apr 29 '21 19:04 SethTisue

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?

joroKr21 avatar Apr 29 '21 19:04 joroKr21

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

SethTisue avatar Apr 29 '21 20:04 SethTisue