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

Reimplement Macros with Scala 3 metaprogramming #689

Merged
merged 14 commits into from
Mar 15, 2022

Conversation

cchantep
Copy link
Member

@cchantep cchantep commented Jan 4, 2022

Pull Request Checklist

Purpose

Reimplement Macros with Scala 3 metaprogramming.

  • Restore support of non case class (previously thanks to custom extractor/unapply, now using ProductOf/Conversion)
  • Restore support of non case object
  • Restore Value class support
  • Restore custom naming support (field & discriminator)

Background Context

First, supports same compile-time features as those provided by the Scala 2 macros.

Simple Dotty derivation not sufficient, use new meta-programming to wrap/adapt (mirror still used to resolve the class fields).

References

Are there any relevant issues / PRs / mailing lists discussions?

@cchantep cchantep force-pushed the task/scala3 branch 20 times, most recently from 4f31838 to 63913dc Compare January 8, 2022 21:38
@@ -139,7 +139,7 @@ private[json] trait JsValueMacros {

}

trait JsMacrosWithOptions extends JsMacros {
trait JsMacrosWithOptions[Opts <: Json.MacroOptions] extends JsMacros {
Copy link
Member Author

Choose a reason for hiding this comment

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

Required for the Scala3 equivalent, not a compatibility issue for me, as compile-time

@@ -12,179 +12,8 @@ import org.scalacheck.Gen
class MacroScala2Spec extends AnyWordSpec with Matchers with org.scalatestplus.scalacheck.ScalaCheckPropertyChecks {
import MacroScala2Spec._

"Reads" should {
Copy link
Member Author

Choose a reason for hiding this comment

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

Merge back to MacroSpec

@@ -33,36 +33,34 @@ class JsonSharedSpec extends AnyWordSpec with Matchers with org.scalatestplus.sc
"JSON" should {
"equals JsObject independently of field order" in json { js =>
js.obj(
"field1" -> 123,
Copy link
Member Author

Choose a reason for hiding this comment

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

Source formatting

@@ -9,6 +9,7 @@ import Matchers._
import org.scalatest.wordspec.AnyWordSpec

object TestFormats {

Copy link
Member Author

Choose a reason for hiding this comment

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

Source formatting + restore common specs for Scala 2/3

@@ -11,11 +11,11 @@ object Common extends AutoPlugin {

override def globalSettings =
Seq(
organization := "com.typesafe.play",
Copy link
Member Author

Choose a reason for hiding this comment

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

Source formatting

@cchantep cchantep force-pushed the task/scala3 branch 3 times, most recently from 8ff60d8 to 03b2c25 Compare January 8, 2022 22:48
@cchantep cchantep marked this pull request as ready for review January 8, 2022 22:56
@cchantep cchantep changed the title Draft: Reimplement Macros with Scala 3 metaprogramming Reimplement Macros with Scala 3 metaprogramming Jan 8, 2022
.scalafmt.conf Outdated
}

project.excludePaths = [
"glob:**/docs/**/code*/*.scala"
Copy link
Member Author

Choose a reason for hiding this comment

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

Make sure special comment to extract snipped are unchanged (special comment a doc delimiters)

}

"no custom ProductOf" in {
"Macros.writer[CustomNoProductOf]".mustNot(typeCheck)
Copy link
Member Author

Choose a reason for hiding this comment

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

Conversion alone is not sufficient

@AlexITC
Copy link

AlexITC commented Jan 14, 2022

Are there any plans to consider merging this anytime soon? @playframework/contributors

@BillyAutrey
Copy link
Member

Are there any plans to consider merging this anytime soon? @playframework/contributors

Just for transparency here, we asked that Cédric split up the PR. Separating the large amount of formatting changes from the relevant code makes it easier to review.

@cchantep
Copy link
Member Author

Up

@mkurz
Copy link
Member

mkurz commented Mar 10, 2022

Please fix the conflicts ,thanks!

@cchantep
Copy link
Member Author

Please fix the conflicts ,thanks!

Approval required

@godenji
Copy link

godenji commented Mar 14, 2022

Thanks for pushing this forward @cchantep

After this is merged (hopefully soon!), when can we expect a new release? I've been working off of the 2.10.0-RC5 release in a Scala 3 project; it's 100+ commits behind main at this point...

@mkurz
Copy link
Member

mkurz commented Mar 15, 2022

OK, I am going to merge this and release 2.10.0-RC6 so people can start testing

@mkurz mkurz merged commit d841976 into playframework:main Mar 15, 2022
@cchantep
Copy link
Member Author

It would have been nice that I was able to merge it myself (as core contributor for "quite sometime") ...

@cchantep cchantep deleted the task/scala3 branch March 15, 2022 10:52
@mkurz
Copy link
Member

mkurz commented Mar 15, 2022

It would have been nice that I was able to merge

You can do now.

@xuwei-k
Copy link
Contributor

xuwei-k commented Mar 16, 2022

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 this pull request may close these issues.

7 participants