-
Notifications
You must be signed in to change notification settings - Fork 131
Add an irrefutable version of the NoSuccess extractor #496
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
Conversation
65ca2c4 to
97d5463
Compare
Binary compatibility checks fail because of the new definition, I assume if this goes to a version 2.2.x it should be fine?
I assume if this goes to a version 2.2.x it should be fine
We currently have versionPolicyIntention set to backwards-compat-only BinaryCompatible (rather than to the stricter BinaryAndSourceCompatible for bidirectional compat), so I think we need to take seriously the possibility that MiMa is right to reject the change.
The failure looks like:
[error] * abstract method NoSuccessE()scala.util.parsing.combinator.Parsers#NoSuccessE# in interface scala.util.parsing.combinator.Parsers is present only in current version
[error] filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.util.parsing.combinator.Parsers.NoSuccessE")
If I understand ReversedMissingMethodProblem correctly, it's complaining because a method was added to an interface. Doing so isn't backwards binary compatible because existing classes that implement that interface will lack an implementation for the new method.
Must the new extractor be nested inside Parsers?
97d5463 to
45f437d
Compare
Thanks Seth, I failed to challenge my assumptions...
Added a nested extractor case NoSuccess.I(msg, next), that has better ergonomics as well. Should NoSuccess.unapply be deprecated to help people find the new one?
Um, hmm, judgment call... I'm a bit tempted to say just Scaladoc it rather than deprecate, but really, I'm on the fence. @Philippus opinion?
The exhaustivity already functions as a heads up. So doc it, I agree.
NoSuccess.unapply cannot be changed due to binary compatibility.
45f437d to
cb53ee1
Compare
@lrytz shall I roll a release?
Yes thanks!
....2.0 ### What changes were proposed in this pull request? This pr aims upgrade `scala-parser-combinators from` from 2.1.1 to 2.2.0. ### Why are the changes needed? scala/scala-parser-combinators#496 add `NoSuccess.I` to helps users avoid exhaustiveness warnings in their pattern matches, especially on Scala 2.13 and 3. The full release note as follows: - https://github.com/scala/scala-parser-combinators/releases/tag/v2.2.0 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions Closes #40083 from LuciferYang/SPARK-42489. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Sean Owen <srowen@gmail.com>
NoSuccess.unapply cannot be changed due to binary compatibility.