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

Scalamock regressed in 2.13 nightly build #11774

Closed
SethTisue opened this issue Oct 17, 2019 · 4 comments · Fixed by scala/scala#8476
Closed

Scalamock regressed in 2.13 nightly build #11774

SethTisue opened this issue Oct 17, 2019 · 4 comments · Fixed by scala/scala#8476
Assignees

Comments

@SethTisue
Copy link
Member

in the Scala 2.13 community build,

-# October 2, 2019
-nightly=2.13.2-bin-4ec48ff
+# October 16, 2019
+nightly=2.13.2-bin-99afc54

caused https://scala-ci.typesafe.com/job/scala-2.13.x-integrate-community-build/2723/artifact/logs/scalamock-build.log:

[scalamock] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.16/project-builds/scalamock-2cb62f40ad491654bda9f00d2d2ffdc041172de7/shared/src/main/scala/org/scalamock/matchers/MockParameter.scala:32:39: type mismatch;
[scalamock] [error]  found   : AnyRef
[scalamock] [error]  required: T
[scalamock] [error]   def this(v: T) = this(v.asInstanceOf[AnyRef])
[scalamock] [error]                                       ^

the problem is reproducible in the community build with ./narrow scalamock; ./run.sh

and outside the community build by hub clone paulbutcher/ScalaMock then ++2.13.2-bin-99afc54 and scalamockJVM/compile

looking at git log --merges 4ec48ff..99afc54 in scala/scala, scala/scala#8458 looks like the likely culprit

@lrytz @adriaanm can you look at this and see whether you think that PR is at fault, or whether the Scalamock code should be changed?

@lrytz
Copy link
Member

lrytz commented Oct 17, 2019

Minimization

class C[T](x: AnyRef, y: Boolean = false) {
  def this(x: T) = this(x.asInstanceOf[AnyRef])
}

Regressed with scala/scala#8458 indeed.

@adriaanm
Copy link
Contributor

Hmm, I was worried about this case after we merged. Since you're not allowed to specify type params for a constructor invocation, the rules for applicability may actually have been correct to consider the class's type param as bound outside the method signature.

@adriaanm
Copy link
Contributor

Maybe it's enough to actually enforce that rule when doing the constructor call, but leave the new flexibility in place for isApplicable (the bug fixed by the PR was about invoking the constructor using new, not using this, as in this regression)

@SethTisue
Copy link
Member Author

not setting the milestone, since the regression never appeared in any release.

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

Successfully merging a pull request may close this issue.

3 participants