Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Incorrect match inexhaustive warnings of NoSuccess result. #377

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.

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

No branches or pull requests

3 participants