-
-
Notifications
You must be signed in to change notification settings - Fork 208
Support for NamedTuple in focus macros
#1610
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
Changes from all commits
531f084
95ea5ac
aba706c
c208903
52e581c
bff16f0
0cf4de5
0d80452
0c76f73
b2de61d
5a33472
da8fec7
5ee1fe9
8bcaa1d
79a36f2
798c39a
4eb5e3c
5719f34
b24195e
38dd55f
2bcb97c
fca5f65
8794ee7
7e1ec92
fb44037
bbdb930
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,11 +22,20 @@ inThisBuild( | |
| scalaVersion := scala2Version, | ||
| crossScalaVersions := Seq(scala2Version, scala3Version), | ||
| tlCiScalafmtCheck := true, | ||
| githubWorkflowBuild += WorkflowStep.Sbt( | ||
| List("docs/mdoc"), | ||
| name = Some("Run documentation"), | ||
| cond = Some(s"matrix.scala == '2.13' && matrix.project == 'rootJVM'") | ||
| ), | ||
| githubWorkflowBuild ++= | ||
| Vector( | ||
| WorkflowStep.Sbt( | ||
| List("docs/mdoc"), | ||
| name = Some("Run documentation"), | ||
| cond = Some(s"matrix.scala == '2.13' && matrix.project == 'rootJVM'") | ||
| ) | ||
| ) ++ scalaNextTest.projects.map { case (platform, project) => | ||
| WorkflowStep.Sbt( | ||
| List(s"${project.id}/test"), | ||
| name = Some(s"Run Scala Next Tests (${platform.identifier})"), | ||
| cond = Some(s"matrix.java == 'temurin@25' && matrix.scala == '3'") | ||
| ) | ||
| }, | ||
| githubWorkflowJavaVersions := Seq( | ||
| JavaSpec.temurin("11"), | ||
| JavaSpec.temurin("25") | ||
|
|
@@ -38,7 +47,6 @@ inThisBuild( | |
| ) | ||
| ) | ||
| ) | ||
|
|
||
| lazy val kindProjector = "org.typelevel" % "kind-projector" % "0.13.4" cross CrossVersion.full | ||
|
|
||
| lazy val buildSettings = Seq( | ||
|
|
@@ -105,9 +113,10 @@ lazy val buildSettings = Seq( | |
| } | ||
| ) | ||
|
|
||
| lazy val catsVersion = "2.13.0" | ||
| lazy val scala2Version = "2.13.18" | ||
| lazy val scala3Version = "3.3.8" | ||
| lazy val catsVersion = "2.13.0" | ||
| lazy val scala2Version = "2.13.18" | ||
| lazy val scala3Version = "3.3.8" | ||
| lazy val scalaNextVersion = "3.8.4" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have two scala versions here, as a library we want to stay on LTS but it seems 3.8 is only used for new test. Is this because named tuples are not present on LTS?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, that's exactly it. I needed a test project to test those new macros
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it mean that the new macros aren't available on a scala LTS version?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are available - there's a branching path that hinges on the availability of Note that a named tuple isn't a proper new language feature, it's just a bunch of sugar on top of |
||
|
|
||
| lazy val cats = Def.setting("org.typelevel" %%% "cats-core" % catsVersion) | ||
| lazy val catsFree = Def.setting("org.typelevel" %%% "cats-free" % catsVersion) | ||
|
|
@@ -303,6 +312,18 @@ lazy val test = crossProject(JVMPlatform, JSPlatform, NativePlatform) | |
| ) | ||
| ) | ||
|
|
||
| lazy val scalaNextTest = crossProject(JVMPlatform, JSPlatform, NativePlatform) | ||
| .dependsOn(test % "test->test") | ||
| .jvmSettings(monocleJvmSettings) | ||
| .jsSettings(monocleJsSettings) | ||
| .nativeSettings(monocleNativeSettings) | ||
| .enablePlugins(NoPublishPlugin) | ||
| .settings( | ||
| crossScalaVersions := Seq(scalaNextVersion), | ||
| libraryDependencies ++= Seq(munitDiscipline.value), | ||
| scalacOptions --= Seq("-release:8", "-Ykind-projector") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since this gets compiled under 3.8.4 I had to get rid of those two options (tbh only the -release one was a hard error due to 3.8.x shifting to JDK17, the kind projector one was a deprecation warning since it was promoted to an -X option) |
||
| ) | ||
|
|
||
| lazy val bench = project | ||
| .dependsOn(core.jvm, generic.jvm, macros.jvm) | ||
| .settings(moduleName := "monocle-bench") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package monocle.internal.focus | ||
|
|
||
| import scala.quoted.Quotes | ||
| import scala.quoted.* | ||
| import scala.annotation.tailrec | ||
|
|
||
| private[focus] trait FocusBase { | ||
| val macroContext: Quotes | ||
|
|
@@ -11,6 +12,8 @@ private[focus] trait FocusBase { | |
| type TypeRepr = macroContext.reflect.TypeRepr | ||
| type Position = macroContext.reflect.Position | ||
|
|
||
| import macroContext.reflect.* | ||
|
|
||
| case class LambdaConfig(argName: String, lambdaBody: Term) | ||
|
|
||
| enum FocusAction { | ||
|
|
@@ -22,6 +25,14 @@ private[focus] trait FocusBase { | |
| fromCompanion: Term, | ||
| toType: TypeRepr | ||
| ) | ||
| case SelectNamedTupleField( | ||
| fieldName: String, | ||
| fromDescription: NamedTuples.Description, | ||
| toType: TypeRepr, | ||
| // storing the whole NamedTuples helper class might feel weird but it's the best way one can describe that this focus action is only possible | ||
| // when named tuples are actually supported (i.e. on Scala > 3.7.x) | ||
| namedTuples: NamedTuples | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it might feel weird to carry the whole helper class around but IMO it's the cleanest way of saying 'this is only valid if named tuples are supported' (+ it has a bunch of uses in
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment about this in the code? |
||
| ) | ||
| case KeywordSome(toType: TypeRepr) | ||
| case KeywordAs(fromType: TypeRepr, toType: TypeRepr) | ||
| case KeywordEach(fromType: TypeRepr, toType: TypeRepr, eachInstance: Term) | ||
|
|
@@ -34,6 +45,8 @@ private[focus] trait FocusBase { | |
| s"SelectField($fieldName, ${fromType.show}, ${fromTypeArgs.map(_.show)}, ${toType.show})" | ||
| case SelectOnlyField(fieldName, fromType, fromTypeArgs, _, toType) => | ||
| s"SelectOnlyField($fieldName, ${fromType.show}, ${fromTypeArgs.map(_.show)}, ..., ${toType.show})" | ||
| case SelectNamedTupleField(fieldName, fromType, toType, _) => | ||
| s"SelectNamedTupleField($fieldName, ${fromType.show}, ${toType.show})" | ||
| case KeywordSome(toType) => s"KeywordSome(${toType.show})" | ||
| case KeywordAs(fromType, toType) => s"KeywordAs(${fromType.show}, ${toType.show})" | ||
| case KeywordEach(fromType, toType, _) => s"KeywordEach(${fromType.show}, ${toType.show}, ...)" | ||
|
|
@@ -58,4 +71,108 @@ private[focus] trait FocusBase { | |
| } | ||
|
|
||
| type FocusResult[+A] = Either[FocusError, A] | ||
|
|
||
| // unappliedNamedTuple is the type lambda [Names, Values] =>> NamedTuple[Names, Values], used to harvest its type symbol later on | ||
| final class NamedTuples private (private val unappliedNamedTuple: TypeRepr, val companion: Symbol) { | ||
| def isNamedTuple(tpe: TypeRepr) = | ||
| tpe.dealias.typeSymbol == unappliedNamedTuple.typeSymbol | ||
|
|
||
| // a call to NamedTuple.toTuple[Names <: Tuple, Values <: Tuple](tup: NamedTuple.NamedTuple[Names, Values]): Values | ||
| def toTuple(term: Term, description: NamedTuples.Description) = | ||
| Select | ||
| .unique(Ident(companion.termRef), "toTuple") | ||
| .appliedToTypes(description.namesTpe :: description.valuesTpe :: Nil) | ||
| .appliedTo(term) | ||
|
|
||
| def accessFieldByName(term: Term, action: FocusAction.SelectNamedTupleField): Term = { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this essentially comes down to something like this: // let's say the field we want to access is 'field2'
val someNamedTuple = (field1 = 1, field2 = 2)
someNamedTuple.toTuple._2 // but ONLY when the result of .toTuple actually has a ._N accessor, there are cases where it might not have it (see unsafeAccessFieldByIndex doc comment)And if you're curious why we aren't doing the same thing the compiler is doing underneath ( |
||
| val idxOfName = action.fromDescription.names.indexOf(action.fieldName) | ||
| val asTuple = toTuple(term, action.fromDescription) | ||
| unsafeAccessFieldByIndex(asTuple, action.fromDescription, idxOfName) | ||
| } | ||
|
|
||
| def reconstructWithUpdatedField(from: Term, action: FocusAction.SelectNamedTupleField, updatedValue: Term) = { | ||
| val updatedFieldIdx = action.fromDescription.names.indexOf(action.fieldName) | ||
| val asTuple = toTuple(from, action.fromDescription) | ||
| val values = | ||
| Vector.tabulate(action.fromDescription.values.size) { idx => | ||
| if (idx == updatedFieldIdx) updatedValue.asExpr | ||
| else unsafeAccessFieldByIndex(asTuple, action.fromDescription, idx).asExpr | ||
| } | ||
| construct(action.fromDescription, values) | ||
| } | ||
|
|
||
| // NamedTuple >: Tuple so to 'construct' a named tuple we can just upcast an ordinary Tuple to a NamedTuple | ||
| def construct(description: NamedTuples.Description, values: Seq[Expr[Any]]): Term = | ||
| Typed(Expr.ofTupleFromSeq(values).asTerm, TypeTree.of(using description.sourceType.asType)) | ||
|
|
||
| // there's a chance that we're operating on a non-normalized (non TupleN) tuple (for example when N is > 22 or when using NamedTuple.From) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have tests for tuples with more than 22 named params?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there are tests for named tuples derived from |
||
| // in which case we need to fall back to using Product methods since TupleXXL <: scala.Product and '*:' (tuple cons) <: Product AND doesn't get _N accessors | ||
| private def unsafeAccessFieldByIndex(asTuple: Term, description: NamedTuples.Description, index: Int) = { | ||
| val tupleAccessor = s"_${index + 1}" | ||
|
|
||
| if (asTuple.tpe.typeSymbol.fieldMember(tupleAccessor).exists) { | ||
| Select.unique(asTuple, tupleAccessor) | ||
| } else { | ||
| val tpeAtIndex = description.values(index) | ||
| (asTuple.asExpr, tpeAtIndex.asType) match { | ||
| case '{ $prod: scala.Product } -> '[tpe] => | ||
| '{ $prod.productElement(${ Expr(index) }).asInstanceOf[tpe] }.asTerm | ||
| } | ||
| } | ||
| } | ||
|
|
||
| def describe(sourceType: TypeRepr): Option[NamedTuples.Description] = | ||
| sourceType.dealias.simplified match { | ||
| case tpe @ AppliedType(_, namesTpe :: valuesTpe :: Nil) if isNamedTuple(tpe) => | ||
| Some( | ||
| NamedTuples.Description( | ||
| unrollStrings(namesTpe), | ||
| unroll(valuesTpe), | ||
| sourceType, | ||
| namesTpe, | ||
| valuesTpe | ||
| ) | ||
| ) | ||
| case _ => None | ||
| } | ||
|
|
||
| private def unrollStrings(tp: TypeRepr): Vector[String] = | ||
| unroll(tp).map { case ConstantType(StringConstant(l)) => l } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a partial match? coul iever produce a match error?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it gets fed a a |
||
|
|
||
| private def unroll(tpe: TypeRepr): Vector[TypeRepr] = { | ||
| @tailrec def loop(curr: Type[?], acc: Vector[TypeRepr]): Vector[TypeRepr] = | ||
| curr match { | ||
| case '[head *: tail] => | ||
| loop(Type.of[tail], acc.appended(TypeRepr.of[head])) | ||
| case '[EmptyTuple] => | ||
| acc | ||
| } | ||
|
|
||
| loop(tpe.asType, Vector.empty) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| object NamedTuples { | ||
| val Support: Option[NamedTuples] = { | ||
| val companion = Symbol.requiredModule("scala.NamedTuple") | ||
|
|
||
| companion | ||
| .declaredType("NamedTuple") | ||
| .headOption | ||
| .map(sym => NamedTuples(sym.typeRef, companion)) | ||
| } | ||
|
|
||
| case class Description private[NamedTuples] ( | ||
| names: Vector[String], | ||
| values: Vector[TypeRepr], | ||
| sourceType: TypeRepr, | ||
| namesTpe: TypeRepr, | ||
| valuesTpe: TypeRepr | ||
| ) { | ||
| def show: String = | ||
| s"Description($names, ${values.map(_.show)}, ${sourceType.show}, ${namesTpe.show}, ${valuesTpe.show})" | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,9 @@ private[focus] trait ParserBase { | |
| def unapply(term: Term): Option[FocusResult[(RemainingCode, FocusAction)]] | ||
| } | ||
|
|
||
| // the '.simplified' call here is needed because otherwise if an unreduced match type arrives at this call site we're greeted with a compiler barf, like: | ||
| // 'Cannot get type of value [...]' (note that this is especially important for terms that describe a named tuple field access which is typed as 'Elem[NamedTuple[N, V], n.type]' which IS a match type). | ||
| def getType(code: Term): TypeRepr = | ||
| code.tpe.widen.dealias | ||
| code.tpe.widen.dealias.simplified | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The As a reminder, the way the compiler extracts a field from a named tuple like extension [N <: Tuple, V <: Tuple](x: NamedTuple[N, V])
/** The value (without the name) at index `n` of this tuple. */
inline def apply(n: Int): Elem[NamedTuple[N, V], n.type] =
x.toTuple.apply(n).asInstanceOf[Elem[NamedTuple[N, V], n.type]]emphasis on the return type: Elem[NamedTuple[N, V], n.type] which IS a match type so we need to reduce those (which
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you put this as a comment on the file for future maintainers? |
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ private[focus] trait SelectParserBase extends ParserBase { | |
| // Match on a term that is an instance of a case class | ||
| object CaseClass { | ||
| def unapply(term: Term): Option[Term] = | ||
| term.tpe.classSymbol.flatMap { sym => | ||
| term.tpe.simplified.classSymbol.flatMap { sym => | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same reason for that |
||
| Option.when(sym.flags.is(Flags.Case))(term) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package monocle.internal.focus.features.selectfield | ||
|
|
||
| import monocle.internal.focus.FocusBase | ||
| import monocle.Lens | ||
| import monocle.Iso | ||
| import scala.quoted.Quotes | ||
| import scala.quoted.Expr | ||
|
|
||
| private[focus] trait SelectNamedTupleFieldGenerator { | ||
| this: FocusBase => | ||
|
|
||
| import macroContext.reflect.* | ||
|
|
||
| def generateSelectNamedTupleField(action: FocusAction.SelectNamedTupleField): Term = { | ||
| def generateGetter(from: Term): Term = action.namedTuples.accessFieldByName(from, action) | ||
|
|
||
| def generateSetter(from: Term, to: Term): Term = | ||
| action.namedTuples.reconstructWithUpdatedField(from, action, to) | ||
|
|
||
| def generateReverseGet(to: Expr[Any]): Term = | ||
| action.namedTuples.construct(action.fromDescription, Vector(to)) | ||
|
|
||
| (action.fromDescription.sourceType.asType, action.toType.asType) match { | ||
| case ('[f], '[t]) => | ||
| if (action.fromDescription.values.size == 1) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alright so you might be wondering why this weird check - That's because I've already got all of the information that I might need from a single |
||
| '{ | ||
| Iso.apply[f, t]((from: f) => ${ generateGetter('from.asTerm).asExprOf[t] })((to: t) => | ||
| ${ generateReverseGet('to).asExprOf[f] } | ||
| ) | ||
| }.asTerm | ||
| } else { | ||
| '{ | ||
| Lens.apply[f, t]((from: f) => ${ generateGetter('from.asTerm).asExprOf[t] })((to: t) => | ||
| (from: f) => ${ generateSetter('from.asTerm, 'to.asTerm).asExprOf[f] } | ||
| ) | ||
| }.asTerm | ||
| } | ||
| } | ||
|
|
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here's the what (and the why) on what I did to the build:
scalaNextTestis built with Scala 3.8.4 (it could be built with 3.7.x or even 3.6.x but named tuples are experimental in 3.6.x so 3.7.x is the lowest possible version we could really test against) but I didn't find much reasons to not go with the shinest and brightest version,Note that this DOES NOT mean the library needs to built against 3.8.4, this is just a small subproject with tests to test my impl.