paulbutcher / ScalaMock

Native Scala mocking framework
http://scalamock.org/
MIT License
504 stars 100 forks source link

Update scala 3 version, drop support for scala 2 #527

Open hughsimpson opened 3 months ago

hughsimpson commented 3 months ago

Pull Request Checklist

Removes scala 2, updates scala 3 to 3.4.2, adds -experimental compiler flag to negate need for bug/workaround

hughsimpson commented 2 months ago

Hadn't updated the scala 3 version in the GitHub workflow so that test failed. Pushed an update there

hughsimpson commented 2 months ago

So anyway this whole pr was way off base and I do apologise for that. I guess I figured it might still be worth leaving open on the off-chance, and because it had already been tagged (presumably because of my initial false claim about the downstream impact 😬). I was tempted to close it or mark as draft but... idk. Potentially scala 3.4 warrants discussing even though I originally introduced the idea for bad reasons

barkhorn commented 2 months ago

Hey, i don't know about that. It will need a review for sure if there's a potential impact. But we should be able to follow new Scala versions in principle unless there's some breaking changes. After all, if someone wants a ScalaMock for an older versions, Sonatype (to my knowledge) doesn't purge versions so there's always a fallback

martijnhoekstra commented 2 months ago

As a library, I'd suggest staying on LTS versions, as not to force downstreams to update

goshacodes commented 1 month ago

I think there is no problem of bumping version up to latest 3.4 or even 3.5.0 if we can get rid of annotating everything as @experimental, this is crucial, essentially for library users. Scala 3 offers backward compatibility for all versions.

Anyway we are exploiting a bug in the compiler to make it work.

If we decide to go experimental way, I would also consider using TupledFunction to make scalamock simpler

goshacodes commented 1 month ago

Also, in my opinion, we should drop support for Scala 2.12 and Scala 2.13, since library (scala 2) is stable enough and nobody would fix anything for scala 2 anyway. This would unlock futher library development. I have already made the migration as smooth as possible and now we should really move on or the library becomes dead soon

hughsimpson commented 1 month ago

@goshacodes I love that energy. Happy to drop scala 2 support in this pr. Will look into your suggestions on this after I've addressed the ones on the other two.

barkhorn commented 1 month ago

Agree on Scala 2.x support, we have a maintenance-5.x branch that could be used to patch bugs there, but it's been pretty hard to work out some of the macro-related problems in the past, I certainly tried :D

goshacodes commented 1 month ago

Let's proceed then. @hughsimpson please drop scala 2 in this PR

hughsimpson commented 1 month ago

@goshacodes Dropped scala 2, haven't looked into TupledFunction yet - probably won't have the bandwidth for a few days now - but that can probably come later anyway?

goshacodes commented 1 month ago

@goshacodes Dropped scala 2, haven't looked into TupledFunction yet - probably won't have the bandwidth for a few days now - but that can probably come later anyway?

At first lets rewrite everything to scala 3 with scalafix (or similar utility, not sure which one does it)

With tupled functions research is needed anyway, If you want to take it, it will be great

goshacodes commented 1 month ago

@barkhorn Could you please close all scala-2 related issues with wont-fix to cleanup everything, after this MR is merged

Also this is not related, but If you know, could you please tell us, what does proxy package meant for? Not found anything related in the docs or in the issues

goshacodes commented 1 month ago

@hughsimpson opened an issue about TupledFunction https://github.com/paulbutcher/ScalaMock/issues/533

hughsimpson commented 1 month ago

Not sure if I'll get to it or not... Once the next release is cut with the bugfix prs I'll probably be snowed under trying to finalise some chunky migrations, so will probably vanish off the reservation for a while tbqh. But if it's still waiting for me when that work is done I'll be diving straight in

barkhorn commented 1 month ago

@barkhorn Could you please close all scala-2 related issues with wont-fix to cleanup everything, after this MR is merged

Also this is not related, but If you know, could you please tell us, what does proxy package meant for? Not found anything related in the docs or in the issues

The proxy package is using Java reflection, specifically proxies, to dynamically instantiate a mock object. It's very limited compared to macro mocking but the implementation is very concise (https://github.com/paulbutcher/ScalaMock/blob/master/shared/src/main/scala-2/org/scalamock/proxy/ProxyMockFactory.scala), and there may be corner cases where it was/is used to work around some of the macro problems with the Scala 2 macro implementation. https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Proxy.html

With a Scala 3 focus, there is probably not a great rationale anymore for this package

hughsimpson commented 3 weeks ago

I don't mind ripping out a bit more whilst I'm in here. @barkhorn @goshacodes you think this pr should go further? Or merge as it stands and perform the rest of the actions in follow-up?

goshacodes commented 3 weeks ago

I think we should merge as it is now and handle it later. Just asked)