feat: scala 3 derivation #8

i10416 · 2024-12-14T14:52:31Z

close #5
This PR adds Scala 3 derivation.

It pass all the test except one which uses Map, that does not seem to have K0.ProductInstances.

See core/src/test/scala-3/toml/CodecSpec.scala.

core/src/test/scala-3/toml/CodecSpec.scala

i10416 · 2024-12-14T16:13:50Z

core/src/main/scala/toml/Codec.scala

@@ -9,12 +9,12 @@ trait Codec[A] {
    defaults: Codec.Defaults,
    index:    Int
  ): Either[Parse.Error, A]
+  private[toml] def optional: Boolean = false


This field makes it easier to handle default value for missing field of type Option[?]. toml does not have the concept of null or nil, so we cannot simply convert null into null-value-like AST node.

core/src/test/scala-3/toml/CodecSpec.scala

i10416 · 2024-12-14T17:03:23Z

core/src/main/scala-3/toml/derivation/DerivedProductCodec.scala

+                case None =>
+                  Right(
+                    d.defaultParams.get(witnessName)
+                      .map(_.asInstanceOf[t])


defaultParams is Map[Strng, Any], so you need asInstanceOf

i10416 · 2024-12-14T17:06:31Z

core/src/main/scala-3/toml/derivation/DerivedProductCodec.scala

+                      .left.map((a,m) => (witnessName +: a, m))
+                    case None =>
+                      d.defaultParams.get(witnessName) match
+                        case None if codec.optional => Right(None.asInstanceOf[t])


optional returns true if and only if codec is of type Codec[Option[?]], so it is safe to use None.asInstanceOf[t] where t is of type Option[?].

i10416 · 2024-12-16T04:50:48Z

core/src/main/scala-3/toml/derivation/DerivedProductCodec.scala

+                case Right(t) => ((Value.Tbl(map - head),(tail,0),(path,e)), Some(t))
+                case Left((path,e)) => ((value, (labels,index), (head :: path, e)), None)
+            case Value.Tbl(map) =>
+              map.keySet.diff(labelling.elemLabels.toSet).headOption match


Scala 2 derivation does not allow redundant fields for type, so I follow that rule.

i10416 · 2024-12-16T04:51:57Z

core/src/main/scala-3/toml/derivation/DerivedProductCodec.scala

+  ): DerivedProductCodec[P]  with
+    def apply(value: Value, defaults: Defaults, index: Int): Either[Parse.Error, P] =
+      type Acc = (Value, (/*labels: */Seq[String], Index), Parse.Error)
+      inst.unfold[Acc]((value, (labelling.elemLabels, index), (Nil, "")))(


It uses unfold instead of constructA because Scala 2 derivation performs stateful operation on labels and index.

i10416 · 2024-12-16T04:53:08Z

core/src/main/scala-3/toml/derivation/DerivedProductCodec.scala

+              d.defaultParams.get(head) match
+                case None =>
+                  if codec.optional then
+                    ((value,(tail, index),(path,e)), Some(None.asInstanceOf[t]))


We can use None as default value for field of type Option[t].

i10416 · 2024-12-17T05:23:41Z

@keynmol Hello, I guess you're the maintainer of this project. Would you mind if I ask you to review this PR?

keynmol · 2024-12-26T13:02:26Z

Hi @i10416!

Sorry for slow response on this, blame holiday season.

I will look at this PR in the next couple of weeks, thanks for taking a stab at this!

I have my own version somewhere as well, so I'll compare issues and see how they can be merged to achieve perfection.

i10416 · 2024-12-26T17:49:58Z

I see.
The tricky parts, I think, are Option and Map[String, T]. I'm looking forward to your version so that I can get inspiration from it.

Have a nice holiday season!

keynmol

In general, this works, good job!

I added a few comments:

one method needs to be extensively refactored, very difficult to follow
as many things as possible should be made private[toml] (macros often prevent hiding some details, but hopefully various traits can be hidden here)
the project doesn't yet have a .scalafmt.conf, to avoid reformatting everything at once (I will do it after your PR), would you mind formatting just the larger files you added in the PR using this config?

version = "3.8.3"
runner.dialect = scala213source3

fileOverride {
  "glob:**.sbt" {
    runner.dialect = scala212source3
  }

  "glob:**/project/**.*" {
    runner.dialect = scala212source3
  }

  "glob:**/scala-3/**/*.scala" {
    runner.dialect = scala3
    rewrite.scala3.insertEndMarkerMinLines = 10
    rewrite.scala3.removeOptionalBraces = true
    rewrite.scala3.convertToNewSyntax = true
  }
}

This will be the config that I will use for the rest of the project

keynmol · 2025-01-11T08:56:54Z

core/src/main/scala-3/toml/derivation/DerivedProductCodec.scala

+    using inst: K0.ProductInstances[Codec, P],labelling: Labelling[P], d:DefaultParams[P]
+  ): DerivedProductCodec[P]  with
+    def apply(value: Value, defaults: Defaults, index: Int): Either[Parse.Error, P] =
+      type Acc = (Value, (/*labels: */Seq[String], Index), Parse.Error)


I think using a tuple here makes things more confusing than they have to be – instead, let's use a case class with meaningful field names and refer to them directly, instead of destructuring with @unchecked.

At the moment the logic is very different to review because of the amount of tuples

keynmol · 2025-01-11T09:54:24Z

core/src/main/scala-3/toml/derivation/syntax.scala

+package derivation
+import shapeless3.deriving.*
+
+trait DerivedSyntax {self: Codec.type =>


this can be made private[toml]

keynmol · 2025-01-11T09:55:30Z

core/src/main/scala-3/toml/derivation/ProductDefaultsMacro.scala

+private[toml] object macros:
+    inline def defaultParams[T]: Map[String, Any] = ${ defaultParmasImpl[T] }
+
+    def defaultParmasImpl[T](using quotes: Quotes, tpe: Type[T]): Expr[Map[String, Any]] =


typo - Parmas instead of Params

seems like the typo is copied from https://github.com/lampepfl/dotty-macro-examples/blob/main/defaultParamsInference/src/macro.scala

keynmol · 2025-01-11T10:05:26Z

core/src/main/scala-2/toml/derivation/syntax.scala

+package toml
+package derivation
+
+trait DerivedSyntax {self: Codec.type => }


can be made private[toml]

keynmol · 2025-01-11T10:05:32Z

core/src/main/scala-2/toml/TomlVersionSpecific.scala

-
-  def parse(toml: String, extensions: Set[Extension] = Set()): Either[Parse.Error, Value.Tbl]
-
+trait TomlVersionSpecific { self: Toml.type =>


may be this can be made private[toml]?

i10416 added 2 commits December 14, 2024 23:46

feat: scala 3 derivation baseline

a5ef05f

chore: remove variable name to avoid unexpected use

d9b75c2

i10416 commented Dec 14, 2024

View reviewed changes

core/src/test/scala-3/toml/CodecSpec.scala Outdated Show resolved Hide resolved

feat: pass more tests

66c028c

i10416 changed the title ~~feat: scala 3 derivation baseline~~ feat: scala 3 derivation Dec 14, 2024

i10416 commented Dec 14, 2024

View reviewed changes

core/src/test/scala-3/toml/CodecSpec.scala Show resolved Hide resolved

i10416 added 4 commits December 15, 2024 01:16

chore: unused import

9bb3f42

chore: unused variable

6bcb29e

chore: minor cleanup

242a7dd

improve: pass more and more tests

35f4dac

i10416 commented Dec 14, 2024

View reviewed changes

i10416 added 3 commits December 15, 2024 03:03

improve: pass another test

d85fb3b

chore: add note for future developers

6c1138f

refactor: use unfold for stateful operation instead of cosntructA

2e6abae

i10416 commented Dec 16, 2024

View reviewed changes

i10416 added 2 commits December 16, 2024 14:08

chore: move tests shared betweeen Scala 2 and 3

762b498

feat: derives syntax

64febda

keynmol requested changes Jan 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: scala 3 derivation #8

feat: scala 3 derivation #8

i10416 commented Dec 14, 2024 •

edited

Loading

i10416 Dec 14, 2024 •

edited

Loading

i10416 Dec 14, 2024

i10416 Dec 14, 2024

i10416 Dec 16, 2024 •

edited

Loading

i10416 Dec 16, 2024

i10416 Dec 16, 2024

i10416 commented Dec 17, 2024 •

edited

Loading

keynmol commented Dec 26, 2024

i10416 commented Dec 26, 2024 •

edited

Loading

keynmol left a comment

keynmol Jan 11, 2025

keynmol Jan 11, 2025

keynmol Jan 11, 2025

keynmol Jan 11, 2025

keynmol Jan 11, 2025


		def parse(toml: String, extensions: Set[Extension] = Set()): Either[Parse.Error, Value.Tbl]

		trait TomlVersionSpecific { self: Toml.type =>

feat: scala 3 derivation #8

Are you sure you want to change the base?

feat: scala 3 derivation #8

Conversation

i10416 commented Dec 14, 2024 • edited Loading

i10416 Dec 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i10416 Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i10416 commented Dec 17, 2024 • edited Loading

keynmol commented Dec 26, 2024

i10416 commented Dec 26, 2024 • edited Loading

keynmol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i10416 commented Dec 14, 2024 •

edited

Loading

i10416 Dec 14, 2024 •

edited

Loading

i10416 Dec 16, 2024 •

edited

Loading

i10416 commented Dec 17, 2024 •

edited

Loading

i10416 commented Dec 26, 2024 •

edited

Loading