Skip to content

Incorrect match inexhaustive warnings of NoSuccess result. #377

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
counter2015 opened this issue Apr 27, 2021 · 6 comments
Open

Incorrect match inexhaustive warnings of NoSuccess result. #377

counter2015 opened this issue Apr 27, 2021 · 6 comments

Comments

@counter2015
Copy link
Contributor

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

@joroKr21
Copy link
Member

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

@counter2015
Copy link
Contributor Author

counter2015 commented Apr 27, 2021

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]

@joroKr21
Copy link
Member

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.

@SethTisue
Copy link
Member

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.

@joroKr21
Copy link
Member

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?

@SethTisue
Copy link
Member

SethTisue commented Apr 29, 2021

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants