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

Scala 3 #490

Merged
merged 18 commits into from
Feb 13, 2024
Merged

Scala 3 #490

merged 18 commits into from
Feb 13, 2024

Conversation

goshacodes
Copy link
Collaborator

@goshacodes goshacodes commented Sep 7, 2023

Pull Request Checklist

  • I agree to licence my contributions under the MIT licence
  • I have added copyright headers to new files

Purpose

Migration to Scala 3.

Write additional tests for:

  • trait with parameters in constructor.
  • parameters with & or | type.

Migration notes:

  1. Methods without parameters should be represented as function (Scala 3 restriction)
trait Turtle:
  def getAngle: Double

val t = mock[Turtle]

// this one no longer compiles
(t.getAngle _).expects(*).returns(1.0)

// this one should be used instead
(() => t.getAngle).expects(*).returns(1.0)
  1. Type should be specified for methods with by-name parameters
trait TestTrait:
  def byNameParam(x: => Int): String

val t = mock[TestTrait]

// this one no longer compiles
(t.byNameParam _).expects(*).returns("")

// this one should be used instead
(t.byNameParam(_: Int)).expects(*).returns("")
  1. Vars support becomes super scarse
    Abstract vars are not supported anymore, use scala.compiletime.uninitialized instead.
    There are multiple issues when trying to override vars, so mocking vars feature is dropped.
trait X:
  var y: Int  // No more compiles
  var y: Int = scala.compile.uninitialized // Should be used instead 
  1. Java class mocking is not available without workaround
public class JavaClass {
    public int simpleMethod(String b) { return 4; }
}
val m = mock[JavaClass] // not works anymore

class JavaClassExtended extends JavaClass

val mm = mock[JavaClassExtended] // should be used instead

@goshacodes goshacodes mentioned this pull request Sep 8, 2023
@lrytz
Copy link

lrytz commented Sep 13, 2023

Facing this one right now

Is this problem still current?

@goshacodes
Copy link
Collaborator Author

goshacodes commented Sep 14, 2023

No, it is fixed

Facing this one right now

Is this problem still current?

@goshacodes
Copy link
Collaborator Author

I'm superbusy till next wednesday. If someone want to help, I can add a collaborator to my fork
telegram: @GeorgiIkov
discord: @goshacodes

@goshacodes goshacodes changed the title [WIP] Scala 3 Scala 3 Oct 17, 2023
@goshacodes goshacodes changed the title Scala 3 [WIP] Scala 3 Nov 3, 2023
@tmccombs
Copy link

Anything I can do to help?

@kovalyovGA
Copy link

kovalyovGA commented Nov 23, 2023

@tmccombs Hi. I've found a problem after resolving all tests.
I need to rewrite path dependent types support, but I don't sure how for now.
This is not working, as I remember. And also more nested structures with path dependent types:

trait Foo:
   trait Bar
   trait Baz[T]
  def foo(x: Option[Bar]): Option[Bar]
  def bar(x: Baz[Bar]): Baz[Bar]

And I have no time :(

@goshacodes
Copy link
Collaborator Author

I've solved last issue

@goshacodes goshacodes changed the title [WIP] Scala 3 Scala 3 Nov 29, 2023
@goshacodes
Copy link
Collaborator Author

goshacodes commented Nov 29, 2023

@paulbutcher @barkhorn Hi guys, I need to run new workflow, merge and publish candidate

@paulbutcher
Copy link
Owner

I'm afraid that I handed the reigns entirely over to Philipp some while ago.

@barkhorn
Copy link
Collaborator

Might need to revisit that situation. I’ve not been using Scala for some years now so the focus was just on small dependency updates and contributed patches. It’s nice to see people porting it to Scala 3 for sure!

@barkhorn
Copy link
Collaborator

Kicked off the workflow

@goshacodes
Copy link
Collaborator Author

goshacodes commented Dec 4, 2023

So what do we do? I think you can make me a collaborator, so I'll handle everything myself

@paulbutcher
Copy link
Owner

@barkhorn are you OK with passing access along to @goshacodes? He's showing willing which is a good start 😜

@goshacodes
Copy link
Collaborator Author

goshacodes commented Dec 10, 2023

@barkhorn I summon you 😄
pokemon ball image

@KuceraMartin
Copy link

Hi, is someone working on the code review? Can we do something to help/facilitate it?

@goshacodes
Copy link
Collaborator Author

I guess nobody is going to review it, so we should leave it as it is and publish a candidate like M1 or RC1

@goshacodes
Copy link
Collaborator Author

@barkhorn

@goshacodes
Copy link
Collaborator Author

@barkhorn @paulbutcher Guys, all this is ridiculous, I think we can manage it without waiting another few years passing by

@maxcom
Copy link

maxcom commented Jan 31, 2024

Please release milestone artifacts with Scala 3 support.

@SethTisue
Copy link
Contributor

I'm guessing the maintainers aren't seeing these notifications. Has anyone tried contacting them by other means?

@barkhorn
Copy link
Collaborator

I can take a look at the weekend, but don't have proper access to things at the moment. If anyone else wants to start put some comments in for now, that would be appreciated. Given the size of the change, would make sense to have some more eyes on it.

.github/workflows/scala.yml Outdated Show resolved Hide resolved
}
},
scalacOptions ++= Seq("-deprecation", "-unchecked", "-feature", "-Xcheckinit", "-target:jvm-1.8")
scalaVersion := "3.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
scalaVersion := "3.3.0",
scalaVersion := "3.3.1",

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug in 3.3.0 which allows us not to use @experimental on every test suite. I think we should use it.

Copy link
Contributor

@SethTisue SethTisue Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth a comment, then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

build.sbt Outdated Show resolved Hide resolved
@SethTisue
Copy link
Contributor

I noticed that the assertDoesNotCompile calls in the tests were commented out — what's the reason for that?

@goshacodes
Copy link
Collaborator Author

I'll add a comment, thought I did.

scalatest/scalatest#2283

@henricook
Copy link

Thanks for your activity here both, I was about to start writing scalamock out of my project to get to Scala 3 but what you're doing here gives me hope 🙏🏻

assertResult("it worked") { m.byNameParam(42) }
}
}

/*

//! TODO - in scala 3 we can't distinguish abstract var setters with non abstract vars setters
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should go into the documentation pages as a known caveat

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to readme for now

@@ -0,0 +1,189 @@
// Copyright (c) 2011-2015 ScalaMock Contributors (https://github.com/paulbutcher/ScalaMock/graphs/contributors)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd remove the year in this line on new files, but not a big deal

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

assert(new MatchEpsilon(1.0) == 1.0001)
assert(new MatchEpsilon(1.0) == 1.0001f)
assert(new MatchEpsilon(1.0) == 1)
assert(new MatchEpsilon(1.0).equals(1.0))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is .equals() different to ==?
what is this change meant to do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scala 3 check types at compile time when using ==. So it is a compile time error
image

@@ -216,16 +199,16 @@ class MockTest extends AnyFreeSpec with MockFactory with Matchers {
}
}

//! TODO - currently doesn't work because we can't override concrete vars
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth adding this to the docs, including a brief example, and, if suitable, a workaround?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration notes to readme of this repo

"MockFactory" should "be mixed only with Any*Spec and not Async*Spec traits" in {
assertCompiles("new AnyFlatSpec with MockFactory")
assertDoesNotCompile("new AsyncFlatSpec with MockFactory")
//assertDoesNotCompile("new AsyncFlatSpec with MockFactory")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of commenting this out, can we mark the test as skipped please?
or at least add a FIXME as that first assertion still holds true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added FIXME with issue

- name: Set up JDK
uses: actions/setup-java@v3
with:
java-version: 11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is JDK 11 still the best runtime?
17/21 are available as LTS versions now but the class-version may be too high for Scala?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we can user latter version

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Play framework still supports JDK 11 and I use ScalaMock in a library released for Play, it would be awesome supporting JDK 11 for a while

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left 11

@barkhorn
Copy link
Collaborator

barkhorn commented Feb 4, 2024

A lot of work has clearly gone into that, thank for cleaning up in general. It looks good to me.

As a general comment, I think it would be great if the documentation could follow to explain any caveats with some of the changes. That doesn't have to be right now but I hope someone would pick that up.

@goshacodes
Copy link
Collaborator Author

Should I do anything else so we could proceed?

@barkhorn barkhorn merged commit bb5d373 into paulbutcher:master Feb 13, 2024
@barkhorn
Copy link
Collaborator

something has changed in one of the sbt plugins i think. couldn't release to sonatype just now because of missing signature (why is that even still a requirement...). I'll take a look over the coming days to push it out

@barkhorn
Copy link
Collaborator

released! 🚢

@henricook
Copy link

I'm probably just jumping the gun before the repositories have updated but it's:

"org.scalamock" %% "scalamock" % "6.0.0-M1" right?

@barkhorn
Copy link
Collaborator

it's on maven central as usual, so you can reference it the same way: https://central.sonatype.com/artifact/org.scalamock/scalamock_3

@henricook
Copy link

henricook commented Feb 13, 2024

Thanks, I didn't have the _3 in my previous dep line

Edit: "org.scalamock" %% "scalamock" % "6.0.0-M1" eventually started working for 2.13 - I was just jumping the gun on the repository publishing the artifacts

@rbscgh
Copy link

rbscgh commented Mar 14, 2024

great work @goshacodes! really uplifting news this got out 💟

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

Successfully merging this pull request may close these issues.

Scala 3 Support