From b9c067be89894ce9f846e8daa9a41c7b5820fbb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Sun, 1 Dec 2024 17:39:45 +0100 Subject: [PATCH 01/14] Target test for extension copy (#234) --- .../quicklens/test/ExtensionCopyTest.scala | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala new file mode 100644 index 0000000..d85f588 --- /dev/null +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala @@ -0,0 +1,50 @@ +package com.softwaremill.quicklens +package test + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +object ExtensionCopyTest { + case class V(x: Double, y: Double) + + opaque type Vec = V + + object Vec { + def apply(x: Double, y: Double): Vec = V(x, y) + } + + extension (v: Vec) { + def x: Double = v.x + def y: Double = v.y + def copy(x: Double = v.x, y: Double = v.y): Vec = V(x, y) + } +} + +class ExtensionCopyTest extends AnyFlatSpec with Matchers { + it should "modify a class with an extension copy method" in { + case class V(x: Double, y: Double) + + class Vec(val v: V) + + object Vec { + def apply(x: Double, y: Double): Vec = new Vec(V(x, y)) + } + + extension (v: Vec) { + def x: Double = v.v.x + def y: Double = v.v.y + def copy(x: Double = v.x, y: Double = v.y): Vec = new Vec(V(x, y)) + } + val a = Vec(1, 2) + val b = a.modify(_.x).using(_ + 1) + println(b) + } + + it should "modify an opaque type with an extension copy method" in { + import ExtensionCopyTest.* + + val a = Vec(1, 2) + val b = a.modify(_.x).using(_ + 1) + println(b) + } +} From cf75c7fdc3d4868a8d0fc39638a23f3994eeb3ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Mon, 2 Dec 2024 09:02:41 +0100 Subject: [PATCH 02/14] Disable unchecked warning --- .../src/main/scala-3/com/softwaremill/quicklens/package.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/package.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/package.scala index 179801b..467b481 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/package.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/package.scala @@ -154,7 +154,7 @@ package object quicklens { def map[A](fa: M[A], f: A => A): M[A] = { val mapped = fa.view.mapValues(f) (fa match { - case sfa: SortedMap[K, A] => sfa.sortedMapFactory.from(mapped)(using sfa.ordering) + case sfa: SortedMap[K, A]@unchecked => sfa.sortedMapFactory.from(mapped)(using sfa.ordering) case _ => mapped.to(fa.mapFactory) }).asInstanceOf[M[A]] } From 0ba4cf87304bd1d0d3993bca52815898862bada8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Mon, 2 Dec 2024 09:21:50 +0100 Subject: [PATCH 03/14] Find extension copy in companion --- build.sbt | 2 +- .../quicklens/QuicklensMacros.scala | 128 ++++++++++++------ .../quicklens/test/ExtensionCopyTest.scala | 63 +++++++-- 3 files changed, 142 insertions(+), 51 deletions(-) diff --git a/build.sbt b/build.sbt index b2cd19b..61042f7 100644 --- a/build.sbt +++ b/build.sbt @@ -14,7 +14,7 @@ excludeLintKeys in Global ++= Set(ideSkipProject) val commonSettings = commonSmlBuildSettings ++ ossPublishSettings ++ Seq( organization := "com.softwaremill.quicklens", updateDocs := UpdateVersionInDocs(sLog.value, organization.value, version.value, List(file("README.md"))), - scalacOptions ++= Seq("-deprecation", "-feature", "-unchecked"), // useful for debugging macros: "-Ycheck:all" + scalacOptions ++= Seq("-deprecation", "-feature", "-unchecked"), // useful for debugging macros: "-Ycheck:all", "-Xcheck-macros" ideSkipProject := (scalaVersion.value != scalaIdeaVersion) ) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index 20e640e..0a652b9 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -133,6 +133,8 @@ object QuicklensMacros { /** Method call with one type parameter and using clause */ case a @ Apply(TypeApply(Apply(TypeApply(Ident(s), _), idents), typeTrees), List(givn)) if methodSupported(s) => idents.flatMap(toPath(_, focus)) :+ PathSymbol.FunctionDelegate(s, givn, typeTrees.last, List.empty) + case Apply(Ident(ident), Seq(deep)) => // this is an extension method, which is called e.g. as x(_$1) + toPath(deep, focus) :+ PathSymbol.Field(ident) /** Field access */ case Apply(deep, idents) => toPath(deep, focus) ++ idents.flatMap(toPath(_, focus)) @@ -179,21 +181,77 @@ object QuicklensMacros { case lst => report.errorAndAbort(multipleMatchingMethods(sym.name, name, lst)) } - def methodSymbolByNameAndArgsOrError(sym: Symbol, name: String, argsMap: Map[String, Term]): Symbol = { + def filterMethodsByNameAndArgs(allMethods: List[Symbol], argsMap: Map[String, Term]): Option[Symbol] = { val argNames = argsMap.keys - sym.methodMember(name).filter{ msym => + allMethods.filter { msym => // for copy, we filter out the methods that don't have the desired parameter names val paramNames = msym.paramSymss.flatten.filter(_.isTerm).map(_.name) argNames.forall(paramNames.contains) } match - case List(m) => m - case Nil => report.errorAndAbort(noSuchMember(sym.name, name)) - case lst @ (m :: _) => + case List(m) => Some(m) + case Nil => None + case lst@(m :: _) => // if we have multiple matching copy methods, pick the synthetic one, if it exists, otherwise, pick any method val syntheticCopies = lst.filter(_.flags.is(Flags.Synthetic)) syntheticCopies match - case List(mSynth) => mSynth - case _ => m + case List(mSynth) => Some(mSynth) + case _ => Some(m) + } + + def methodSymbolByNameAndArgs(sym: Symbol, name: String, argsMap: Map[String, Term]): Option[Symbol] = { + val memberMethods = sym.methodMember(name) + filterMethodsByNameAndArgs(memberMethods, argsMap) + } + + /** + * @param argsMap normal methods receive one parameter list, extensions methods two, the first one contains the value + * on which the extension is called + * */ + def callMethod(obj: Term, copy: Symbol, argsMap: List[Map[String, Term]]) = { + val objTpe = obj.tpe.widenAll + val objSymbol = objTpe.matchingTypeSymbol + + val typeParams = objTpe match { + case AppliedType(_, typeParams) => Some(typeParams) + case _ => None + } + val copyTree: DefDef = copy.tree.asInstanceOf[DefDef] + val copyParams: List[(String, Option[Term])] = copyTree.termParamss.zip(argsMap) + .map((params, args) => params.params.map(_.name).map(name => name -> args.get(name))) + .flatten.toList + + val args = copyParams.zipWithIndex.map { case ((n, v), _i) => + val i = _i + 1 + def defaultMethod = + val methodSymbol = methodSymbolByNameOrError(objSymbol, copy.name + "$default$" + i.toString) + // default values in extensions are obtained by calling a method receiving the extension parameter + val defaultMethodArgs = argsMap.dropRight(1).headOption.toList.flatMap(_.values) + //println(s"defaultMethodArgs ${obj.show} ${methodSymbol.name} $defaultMethodArgs") + if defaultMethodArgs.nonEmpty then + Apply(Select(obj, methodSymbol), defaultMethodArgs) + else + // note: this is not always correct, -Xcheck-macros shows errors here + // sometimes we should call a method with empry parameter list instead + obj.select(methodSymbol) + + // for extension methods, might need sth more like this: (or probably some weird implicit conversion) + // val defaultGetter = obj.select(symbolMethodByNameOrError(objSymbol, n)) + n -> v.getOrElse(defaultMethod) + }.toMap + + val argLists = copyTree.termParamss.take(argsMap.size).map(list => list.params.map(p => args(p.name))) + + if copyTree.termParamss.drop(argLists.size).exists(_.params.exists(!_.symbol.flags.is(Flags.Implicit))) then + report.errorAndAbort( + s"Implementation limitation: Only the first parameter list of the modified case classes can be non-implicit. ${copyTree.termParamss.drop(1)}" + ) + + val applyOn = typeParams match { + // if the object's type is parametrised, we need to call .copy with the same type parameters + case Some(typeParams) => TypeApply(Select(obj, copy), typeParams.map(Inferred(_))) + case _ => Select(obj, copy) + } + argLists.foldLeft(applyOn)((applied, list) => Apply(applied, list)) } def termMethodByNameUnsafe(term: Term, name: String): Symbol = { @@ -210,8 +268,19 @@ object QuicklensMacros { (sym.flags.is(Flags.Sealed) && (sym.flags.is(Flags.Trait) || sym.flags.is(Flags.Abstract))) } + def findExtensionMethod(using Quotes)(sym: Symbol, methodName: String): List[Symbol] = { + // TODO: can we check parameter types somehow? + def isExtensionMethod(sym: Symbol): Boolean = sym.isDefDef && sym.paramSymss.headOption.exists(_.sizeIs == 1) + + // TODO: try to search in symbol parent object as well + val symbols = Seq(sym.companionModule).filter(_ != Symbol.noSymbol) + + symbols.flatMap(_.declaredMethods).filter(sym => sym.name == methodName).filter(isExtensionMethod).toList + } + def isProductLike(sym: Symbol): Boolean = { - sym.methodMember("copy").size >= 1 + // just assume true - we can always fail if there is no copy + sym.methodMember("copy").nonEmpty || findExtensionMethod(sym, "copy").nonEmpty } def caseClassCopy( @@ -248,6 +317,7 @@ object QuicklensMacros { } val elseThrow = '{ throw new IllegalStateException() }.asTerm + ifThens.foldRight(elseThrow) { case ((ifCond, ifThen), ifElse) => If(ifCond, ifThen, ifElse) } @@ -260,36 +330,18 @@ object QuicklensMacros { val namedArg = NamedArg(field.name, resTerm) field.name -> namedArg }.toMap - val copy = methodSymbolByNameAndArgsOrError(objSymbol, "copy", argsMap) - - val typeParams = objTpe match { - case AppliedType(_, typeParams) => Some(typeParams) - case _ => None - } - val copyTree: DefDef = copy.tree.asInstanceOf[DefDef] - val copyParamNames: List[String] = copyTree.termParamss.headOption.map(_.params).toList.flatten.map(_.name) - - val args = copyParamNames.zipWithIndex.map { (n, _i) => - val i = _i + 1 - val defaultMethod = obj.select(methodSymbolByNameOrError(objSymbol, "copy$default$" + i.toString)) - // for extension methods, might need sth more like this: (or probably some weird implicit conversion) - // val defaultGetter = obj.select(symbolMethodByNameOrError(objSymbol, n)) - argsMap.getOrElse( - n, - defaultMethod - ) - }.toList - - if copyTree.termParamss.drop(1).exists(_.params.exists(!_.symbol.flags.is(Flags.Implicit))) then - report.errorAndAbort( - s"Implementation limitation: Only the first parameter list of the modified case classes can be non-implicit." - ) - - typeParams match { - // if the object's type is parametrised, we need to call .copy with the same type parameters - case Some(typeParams) => Apply(TypeApply(Select(obj, copy), typeParams.map(Inferred(_))), args) - case _ => Apply(Select(obj, copy), args) - } + methodSymbolByNameAndArgs(objSymbol, "copy", argsMap) match + case Some(copy) => + callMethod(obj, copy, List(argsMap)) + case None => + val objCompanion = objSymbol.companionModule + methodSymbolByNameAndArgs(objCompanion, "copy", argsMap) match + case Some(copy) => + // now try to call the extension as a method, assume the object is its first parameter + val extensionParameter = copy.paramSymss.headOption.map(_.headOption).flatten + val argsWithObj = List(extensionParameter.map(name => name.name -> obj).toMap, argsMap) + callMethod(Ref(objCompanion), copy, argsWithObj) + case None => report.errorAndAbort(noSuchMember(objSymbol.name, "copy")) } else report.errorAndAbort(s"Unsupported source object: must be a case class or sealed trait, but got: $objSymbol of type ${objTpe.show} (${obj.show})") } diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala index d85f588..a24f08e 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala @@ -11,35 +11,73 @@ object ExtensionCopyTest { object Vec { def apply(x: Double, y: Double): Vec = V(x, y) - } - extension (v: Vec) { - def x: Double = v.x - def y: Double = v.y - def copy(x: Double = v.x, y: Double = v.y): Vec = V(x, y) + extension (v: Vec) { + def x: Double = v.x + def y: Double = v.y + def copy(x: Double = v.x, y: Double = v.y): Vec = V(x, y) + } } } class ExtensionCopyTest extends AnyFlatSpec with Matchers { + /* + it should "modify a simple class with an extension copy method" in { + class VecSimple(xp: Double, yp: Double) { + val xMember = xp + val yMember = yp + } + + object VecSimple { + def apply(x: Double, y: Double): VecSimple = new VecSimple(x, y) + } + + extension (v: VecSimple) { + def copy(x: Double = v.xMember, y: Double = v.yMember): VecSimple = new VecSimple(x, y) + } + val a = VecSimple(1, 2) + val b = a.modify(_.xMember).using(_ + 1) + println(b) + } + */ + + it should "modify a simple class with an extension copy method in companion" in { + class VecCompanion(xp: Double, yp: Double) { + val x = xp + val y = yp + } + + object VecCompanion { + def apply(x: Double, y: Double): VecCompanion = new VecCompanion(x, y) + extension (v: VecCompanion) { + def copy(x: Double = v.x, y: Double = v.y): VecCompanion = new VecCompanion(x, y) + } + } + + val a = VecCompanion(1, 2) + val b = a.modify(_.x).using(_ + 1) + println(b) + } + /* + it should "modify a class with an extension copy method" in { case class V(x: Double, y: Double) - class Vec(val v: V) + class VecClass(val v: V) - object Vec { - def apply(x: Double, y: Double): Vec = new Vec(V(x, y)) + object VecClass { + def apply(x: Double, y: Double): VecClass = new VecClass(V(x, y)) } - extension (v: Vec) { + extension (v: VecClass) { def x: Double = v.v.x def y: Double = v.v.y - def copy(x: Double = v.x, y: Double = v.y): Vec = new Vec(V(x, y)) + def copy(x: Double = v.x, y: Double = v.y): VecClass = new VecClass(V(x, y)) } - val a = Vec(1, 2) + val a = VecClass(1, 2) val b = a.modify(_.x).using(_ + 1) println(b) } - it should "modify an opaque type with an extension copy method" in { import ExtensionCopyTest.* @@ -47,4 +85,5 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { val b = a.modify(_.x).using(_ + 1) println(b) } + */ } From d4f10581ba62066788aef7587f25974abc3c2170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Tue, 3 Dec 2024 11:59:38 +0100 Subject: [PATCH 04/14] More accurate error --- .../scala-3/com/softwaremill/quicklens/QuicklensMacros.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index 0a652b9..ccbff52 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -343,7 +343,7 @@ object QuicklensMacros { callMethod(Ref(objCompanion), copy, argsWithObj) case None => report.errorAndAbort(noSuchMember(objSymbol.name, "copy")) } else - report.errorAndAbort(s"Unsupported source object: must be a case class or sealed trait, but got: $objSymbol of type ${objTpe.show} (${obj.show})") + report.errorAndAbort(s"Unsupported source object: must be a case class, sealed trait or class with copy method, but got: $objSymbol of type ${objTpe.show} (${obj.show})") } def applyFunctionDelegate( From b7506f56cc9b3a2662310b9f10dd1a71971f49b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Tue, 3 Dec 2024 17:04:50 +0100 Subject: [PATCH 05/14] Use extensions to access members --- .../quicklens/QuicklensMacros.scala | 52 ++++++++++++++----- .../quicklens/test/ExtensionCopyTest.scala | 20 +++---- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index ccbff52..d2c8993 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -133,8 +133,14 @@ object QuicklensMacros { /** Method call with one type parameter and using clause */ case a @ Apply(TypeApply(Apply(TypeApply(Ident(s), _), idents), typeTrees), List(givn)) if methodSupported(s) => idents.flatMap(toPath(_, focus)) :+ PathSymbol.FunctionDelegate(s, givn, typeTrees.last, List.empty) - case Apply(Ident(ident), Seq(deep)) => // this is an extension method, which is called e.g. as x(_$1) - toPath(deep, focus) :+ PathSymbol.Field(ident) + case Apply(obj, Seq(deep)) => // this is an extension method, which is called e.g. as x(_$1) + obj match + case Ident(ident) => + toPath(deep, focus) :+ PathSymbol.Field(ident) + case Select(term, member) => + toPath(deep, focus) :+ PathSymbol.Field(member) + case other => + report.errorAndAbort(unsupportedShapeInfo(focus.asTerm)) /** Field access */ case Apply(deep, idents) => toPath(deep, focus) ++ idents.flatMap(toPath(_, focus)) @@ -168,17 +174,38 @@ object QuicklensMacros { Symbol.noSymbol } - def symbolAccessorByNameOrError(sym: Symbol, name: String): Symbol = { - val mem = sym.fieldMember(name) - if mem != Symbol.noSymbol then mem - else methodSymbolByNameOrError(sym, name) + def symbolAccessorByNameOrError(obj: Term, name: String): Term = { + val objTpe = obj.tpe.widenAll + val objSymbol = objTpe.matchingTypeSymbol + val mem = objSymbol.fieldMember(name) + if (mem != Symbol.noSymbol) + Select(obj, mem) + else + //Select(obj, mem) + objSymbol.methodMember(name) match + case List(m) => + Select(obj, m) + case Nil => + findExtensionMethod(objSymbol, name) match { + case List((owner, extension)) => + Apply(Select(owner, extension), List(obj)) + case syms => + reportMethodError(objSymbol, name, syms.map(_._2)) + } + case lst => + report.errorAndAbort(multipleMatchingMethods(objSymbol.name, name, lst)) + } + + def reportMethodError(sym: Symbol, name: String, lst: List[Symbol]): Nothing = { + lst match + case Nil => report.errorAndAbort(noSuchMember(sym.name, name)) + case lst => report.errorAndAbort(multipleMatchingMethods(sym.name, name, lst)) } def methodSymbolByNameOrError(sym: Symbol, name: String): Symbol = { sym.methodMember(name) match case List(m) => m - case Nil => report.errorAndAbort(noSuchMember(sym.name, name)) - case lst => report.errorAndAbort(multipleMatchingMethods(sym.name, name, lst)) + case lst => reportMethodError(sym, name, lst) } def filterMethodsByNameAndArgs(allMethods: List[Symbol], argsMap: Map[String, Term]): Option[Symbol] = { @@ -268,18 +295,17 @@ object QuicklensMacros { (sym.flags.is(Flags.Sealed) && (sym.flags.is(Flags.Trait) || sym.flags.is(Flags.Abstract))) } - def findExtensionMethod(using Quotes)(sym: Symbol, methodName: String): List[Symbol] = { + def findExtensionMethod(using Quotes)(sym: Symbol, methodName: String): List[(Term, Symbol)] = { // TODO: can we check parameter types somehow? def isExtensionMethod(sym: Symbol): Boolean = sym.isDefDef && sym.paramSymss.headOption.exists(_.sizeIs == 1) // TODO: try to search in symbol parent object as well val symbols = Seq(sym.companionModule).filter(_ != Symbol.noSymbol) - symbols.flatMap(_.declaredMethods).filter(sym => sym.name == methodName).filter(isExtensionMethod).toList + symbols.flatMap(s => s.declaredMethods.map(Ref(s) -> _)).filter((_, m) => m.name == methodName && isExtensionMethod(m)).toList } def isProductLike(sym: Symbol): Boolean = { - // just assume true - we can always fail if there is no copy sym.methodMember("copy").nonEmpty || findExtensionMethod(sym, "copy").nonEmpty } @@ -323,8 +349,8 @@ object QuicklensMacros { } } else if isProduct(objSymbol) || isProductLike(objSymbol) then { val argsMap: Map[String, Term] = fields.map { (field, trees) => - val fieldMethod = symbolAccessorByNameOrError(objSymbol, field.name) - val resTerm: Term = trees.foldLeft[Term](Select(obj, fieldMethod)) { (term, tree) => + val fieldMethod = symbolAccessorByNameOrError(obj, field.name) + val resTerm: Term = trees.foldLeft[Term](fieldMethod) { (term, tree) => mapToCopy(owner, mod, term, tree) } val namedArg = NamedArg(field.name, resTerm) diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala index a24f08e..3363b52 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala @@ -58,32 +58,32 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { val b = a.modify(_.x).using(_ + 1) println(b) } - /* - - it should "modify a class with an extension copy method" in { + it should "modify a class with extension methods in companion" in { case class V(x: Double, y: Double) class VecClass(val v: V) object VecClass { def apply(x: Double, y: Double): VecClass = new VecClass(V(x, y)) - } - extension (v: VecClass) { - def x: Double = v.v.x - def y: Double = v.v.y - def copy(x: Double = v.x, y: Double = v.y): VecClass = new VecClass(V(x, y)) + extension (v: VecClass) { + def x: Double = v.v.x + def y: Double = v.v.y + def copy(x: Double = v.x, y: Double = v.y): VecClass = new VecClass(V(x, y)) + } } + val a = VecClass(1, 2) val b = a.modify(_.x).using(_ + 1) println(b) } - it should "modify an opaque type with an extension copy method" in { + /* + it should "modify an opaque type with extension methods" in { import ExtensionCopyTest.* val a = Vec(1, 2) val b = a.modify(_.x).using(_ + 1) println(b) } - */ + */ } From 04864788ebc0f466360990936ebec51f02d63938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Tue, 3 Dec 2024 18:40:47 +0100 Subject: [PATCH 06/14] Opaque types support --- .../quicklens/QuicklensMacros.scala | 33 ++++++++++++------- .../quicklens/test/ExplicitCopyTest.scala | 2 +- .../quicklens/test/ExtensionCopyTest.scala | 28 ++++++++-------- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index d2c8993..30464cf 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -177,21 +177,20 @@ object QuicklensMacros { def symbolAccessorByNameOrError(obj: Term, name: String): Term = { val objTpe = obj.tpe.widenAll val objSymbol = objTpe.matchingTypeSymbol - val mem = objSymbol.fieldMember(name) + // opaque types could find members of underlying types - do not ask them (see https://github.com/scala/scala3/issues/22143) + val mem = if !objSymbol.flags.is(Flags.Deferred) then objSymbol.fieldMember(name) else Symbol.noSymbol if (mem != Symbol.noSymbol) Select(obj, mem) else - //Select(obj, mem) objSymbol.methodMember(name) match case List(m) => Select(obj, m) case Nil => - findExtensionMethod(objSymbol, name) match { + findExtensionMethod(objSymbol, name) match case List((owner, extension)) => Apply(Select(owner, extension), List(obj)) case syms => reportMethodError(objSymbol, name, syms.map(_._2)) - } case lst => report.errorAndAbort(multipleMatchingMethods(objSymbol.name, name, lst)) } @@ -226,8 +225,10 @@ object QuicklensMacros { } def methodSymbolByNameAndArgs(sym: Symbol, name: String, argsMap: Map[String, Term]): Option[Symbol] = { - val memberMethods = sym.methodMember(name) - filterMethodsByNameAndArgs(memberMethods, argsMap) + if !sym.flags.is(Flags.Deferred) then + val memberMethods = sym.methodMember(name) + filterMethodsByNameAndArgs(memberMethods, argsMap) + else None } /** @@ -253,7 +254,6 @@ object QuicklensMacros { val methodSymbol = methodSymbolByNameOrError(objSymbol, copy.name + "$default$" + i.toString) // default values in extensions are obtained by calling a method receiving the extension parameter val defaultMethodArgs = argsMap.dropRight(1).headOption.toList.flatMap(_.values) - //println(s"defaultMethodArgs ${obj.show} ${methodSymbol.name} $defaultMethodArgs") if defaultMethodArgs.nonEmpty then Apply(Select(obj, methodSymbol), defaultMethodArgs) else @@ -295,12 +295,20 @@ object QuicklensMacros { (sym.flags.is(Flags.Sealed) && (sym.flags.is(Flags.Trait) || sym.flags.is(Flags.Abstract))) } + def findCompanionLikeObject(objSymbol: Symbol): Option[Symbol] = { + def optSymbol(objSymbol: Symbol) = Option.when(!objSymbol.isNoSymbol)(objSymbol) + optSymbol(objSymbol.companionModule).orElse { + // for opaque types, the companion type is not found by objSymbol.companionModule + // try to find an object by name in the owner scope + optSymbol(objSymbol.owner.fieldMember(objSymbol.name)).filter(_.flags.is(Flags.Module)) + } + } def findExtensionMethod(using Quotes)(sym: Symbol, methodName: String): List[(Term, Symbol)] = { // TODO: can we check parameter types somehow? def isExtensionMethod(sym: Symbol): Boolean = sym.isDefDef && sym.paramSymss.headOption.exists(_.sizeIs == 1) - // TODO: try to search in symbol parent object as well - val symbols = Seq(sym.companionModule).filter(_ != Symbol.noSymbol) + // TODO: try to search in symbol parent scope as well, as extension methods could be located there as well + val symbols = findCompanionLikeObject(sym).filter(_ != Symbol.noSymbol).toList symbols.flatMap(s => s.declaredMethods.map(Ref(s) -> _)).filter((_, m) => m.name == methodName && isExtensionMethod(m)).toList } @@ -360,13 +368,13 @@ object QuicklensMacros { case Some(copy) => callMethod(obj, copy, List(argsMap)) case None => - val objCompanion = objSymbol.companionModule - methodSymbolByNameAndArgs(objCompanion, "copy", argsMap) match + val objCompanion = findCompanionLikeObject(objSymbol) + objCompanion.flatMap(methodSymbolByNameAndArgs(_, "copy", argsMap)) match case Some(copy) => // now try to call the extension as a method, assume the object is its first parameter val extensionParameter = copy.paramSymss.headOption.map(_.headOption).flatten val argsWithObj = List(extensionParameter.map(name => name.name -> obj).toMap, argsMap) - callMethod(Ref(objCompanion), copy, argsWithObj) + callMethod(Ref(objCompanion.get), copy, argsWithObj) case None => report.errorAndAbort(noSuchMember(objSymbol.name, "copy")) } else report.errorAndAbort(s"Unsupported source object: must be a case class, sealed trait or class with copy method, but got: $objSymbol of type ${objTpe.show} (${obj.show})") @@ -429,6 +437,7 @@ object QuicklensMacros { def mapToCopy(owner: Symbol, mod: Expr[A => A], objTerm: Term, pathTree: PathTree): Term = pathTree match { case PathTree.Empty => val apply = termMethodByNameUnsafe(mod.asTerm, "apply") + // TODO: calling extension may be necessary here Apply(Select(mod.asTerm, apply), List(objTerm)) case PathTree.Node(children) => accumulateToCopy(owner, mod, objTerm, children) diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala index f1ecce1..5651991 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala @@ -1,4 +1,5 @@ package com.softwaremill.quicklens +package test import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -90,5 +91,4 @@ class ExplicitCopyTest extends AnyFlatSpec with Matchers { // val f = Frozen("A", 0) // f.modify(_.state).setTo('B') // } - } diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala index 3363b52..f24effa 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala @@ -5,17 +5,17 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers object ExtensionCopyTest { - case class V(x: Double, y: Double) + case class V(x: Double, y: Double, z: Double) opaque type Vec = V object Vec { - def apply(x: Double, y: Double): Vec = V(x, y) + def apply(x: Double, y: Double): Vec = V(x, y, 0) extension (v: Vec) { def x: Double = v.x def y: Double = v.y - def copy(x: Double = v.x, y: Double = v.y): Vec = V(x, y) + def copy(x: Double = v.x, y: Double = v.y): Vec = V(x, y, 0) } } } @@ -55,11 +55,12 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { } val a = VecCompanion(1, 2) - val b = a.modify(_.x).using(_ + 1) - println(b) + val b = a.modify(_.x).using(_ + 10) + assert(b.x == 11) } + it should "modify a class with extension methods in companion" in { - case class V(x: Double, y: Double) + case class V(xm: Double, ym: Double) class VecClass(val v: V) @@ -67,23 +68,22 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { def apply(x: Double, y: Double): VecClass = new VecClass(V(x, y)) extension (v: VecClass) { - def x: Double = v.v.x - def y: Double = v.v.y + def x: Double = v.v.xm + def y: Double = v.v.ym def copy(x: Double = v.x, y: Double = v.y): VecClass = new VecClass(V(x, y)) } } val a = VecClass(1, 2) - val b = a.modify(_.x).using(_ + 1) - println(b) + val b = a.modify(_.x).using(_ + 10) + assert(b.x == 11) } - /* + it should "modify an opaque type with extension methods" in { import ExtensionCopyTest.* val a = Vec(1, 2) - val b = a.modify(_.x).using(_ + 1) - println(b) + val b = a.modify(_.x).using(_ + 10) + assert(b.x == 11) } - */ } From 6e5670c88ee31afc756634ab6d06033f248239bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Wed, 4 Dec 2024 17:54:44 +0100 Subject: [PATCH 07/14] Check value in modification tests --- .../quicklens/test/ExplicitCopyTest.scala | 12 ++++++++---- .../quicklens/test/ExtensionCopyTest.scala | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala index 5651991..bc8f338 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala @@ -34,7 +34,8 @@ class ExplicitCopyTest extends AnyFlatSpec with Matchers { def paths(paths: Paths): Docs = copy(paths = paths) } val docs = Docs() - docs.modify(_.paths.pathItems).using(m => m + ("a" -> PathItem())) + val r = docs.modify(_.paths.pathItems).using(m => m + ("a" -> PathItem())) + r.paths.pathItems should contain ("a" -> PathItem()) } it should "modify a case class with an additional explicit copy" in { @@ -43,7 +44,8 @@ class ExplicitCopyTest extends AnyFlatSpec with Matchers { } val f = Frozen("A", 0) - f.modify(_.state).setTo("B") + val r = f.modify(_.state).setTo("B") + r.state shouldEqual "B" } it should "modify a case class with an ambiguous additional explicit copy" in { @@ -52,7 +54,8 @@ class ExplicitCopyTest extends AnyFlatSpec with Matchers { } val f = Frozen("A", 0) - f.modify(_.state).setTo("B") + val r = f.modify(_.state).setTo("B") + r.state shouldEqual "B" } it should "modify a class with two explicit copy methods" in { @@ -62,7 +65,8 @@ class ExplicitCopyTest extends AnyFlatSpec with Matchers { } val f = new Frozen("A", 0) - f.modify(_.state).setTo("B") + val r = f.modify(_.state).setTo("B") + r.state shouldEqual "B" } it should "modify a case class with an ambiguous additional explicit copy and pick the synthetic one first" in { diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala index f24effa..0fbb832 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala @@ -56,7 +56,7 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { val a = VecCompanion(1, 2) val b = a.modify(_.x).using(_ + 10) - assert(b.x == 11) + b.x shouldEqual 11 } it should "modify a class with extension methods in companion" in { @@ -76,7 +76,7 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { val a = VecClass(1, 2) val b = a.modify(_.x).using(_ + 10) - assert(b.x == 11) + b.x shouldEqual 11 } it should "modify an opaque type with extension methods" in { @@ -84,6 +84,6 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { val a = Vec(1, 2) val b = a.modify(_.x).using(_ + 10) - assert(b.x == 11) + b.x shouldEqual 11 } } From 2d6d7ba4766ea511708b62fa620987b8fa1f1715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Sun, 1 Dec 2024 17:20:48 +0100 Subject: [PATCH 08/14] Add test for #258 --- .../quicklens/test/ExplicitCopyTest.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala index bc8f338..d5dee42 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExplicitCopyTest.scala @@ -82,6 +82,19 @@ class ExplicitCopyTest extends AnyFlatSpec with Matchers { accessed shouldEqual 0 } + it should "not compile when modifying a field which is not present as a copy parameter" in { + """ + case class Content(x: String) + + class A(val c: Content) { + def copy(x: String = c.x): A = new A(Content(x)) + } + + val a = new A(Content("A")) + val am = a.modify(_.c).setTo(Content("B")) + """ shouldNot compile + } + // TODO: Would be nice to be able to handle this case. Based on the types, it // is obvious, that the explicit copy should be picked, but I'm not sure if we // can get that information From baf9bccff38dca0045c8c50865296abfb748b79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 5 Dec 2024 00:39:39 +0100 Subject: [PATCH 09/14] Improve error message (#258) --- .../softwaremill/quicklens/QuicklensMacros.scala | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index 30464cf..763cfb5 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -58,6 +58,9 @@ object QuicklensMacros { def noSuchMember(tpeStr: String, name: String) = s"$tpeStr has no member named $name" + def noSuitableMember(tpeStr: String, name: String, argNames: Iterable[String]) = + s"$tpeStr has no member $name with parameters ${argNames.mkString("(", ", ", ")")}" + def multipleMatchingMethods(tpeStr: String, name: String, syms: Seq[Symbol]) = val symsStr = syms.map(s => s" - $s: ${s.termRef.dealias.widen.show}").mkString("\n", "\n", "") s"Multiple methods named $name found in $tpeStr: $symsStr" @@ -224,11 +227,12 @@ object QuicklensMacros { case _ => Some(m) } - def methodSymbolByNameAndArgs(sym: Symbol, name: String, argsMap: Map[String, Term]): Option[Symbol] = { + def methodSymbolByNameAndArgs(sym: Symbol, name: String, argsMap: Map[String, Term]): Either[String, Symbol] = { if !sym.flags.is(Flags.Deferred) then val memberMethods = sym.methodMember(name) filterMethodsByNameAndArgs(memberMethods, argsMap) - else None + .toRight(if memberMethods.isEmpty then noSuchMember(sym.name, name) else noSuitableMember(sym.name, name, argsMap.keys)) + else Left(s"Deferred type ${sym.name}") } /** @@ -365,17 +369,17 @@ object QuicklensMacros { field.name -> namedArg }.toMap methodSymbolByNameAndArgs(objSymbol, "copy", argsMap) match - case Some(copy) => + case Right(copy) => callMethod(obj, copy, List(argsMap)) - case None => + case Left(error) => val objCompanion = findCompanionLikeObject(objSymbol) - objCompanion.flatMap(methodSymbolByNameAndArgs(_, "copy", argsMap)) match + objCompanion.flatMap(methodSymbolByNameAndArgs(_, "copy", argsMap).toOption) match case Some(copy) => // now try to call the extension as a method, assume the object is its first parameter val extensionParameter = copy.paramSymss.headOption.map(_.headOption).flatten val argsWithObj = List(extensionParameter.map(name => name.name -> obj).toMap, argsMap) callMethod(Ref(objCompanion.get), copy, argsWithObj) - case None => report.errorAndAbort(noSuchMember(objSymbol.name, "copy")) + case None => report.errorAndAbort(error) } else report.errorAndAbort(s"Unsupported source object: must be a case class, sealed trait or class with copy method, but got: $objSymbol of type ${objTpe.show} (${obj.show})") } From ba60326d8a0f33001c1ce76e76b1b18b3b76fc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 5 Dec 2024 00:26:47 +0100 Subject: [PATCH 10/14] Extension lookup using provided terms --- .../quicklens/QuicklensMacros.scala | 58 +++++++++++-------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index 763cfb5..61194e9 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -113,10 +113,12 @@ object QuicklensMacros { enum PathSymbol: case Field(name: String) + case Extension(term: Term, name: String) case FunctionDelegate(name: String, givn: Term, typeTree: TypeTree, args: List[Term]) def equiv(other: Any): Boolean = (this, other) match case (Field(name1), Field(name2)) => name1 == name2 + case (Extension(term1, name1), Extension(term2, name2)) => term1 == term2 && name1 == name2 case (FunctionDelegate(name1, _, typeTree1, args1), FunctionDelegate(name2, _, typeTree2, args2)) => name1 == name2 && typeTree1.tpe == typeTree2.tpe && args1 == args2 case _ => false @@ -141,7 +143,7 @@ object QuicklensMacros { case Ident(ident) => toPath(deep, focus) :+ PathSymbol.Field(ident) case Select(term, member) => - toPath(deep, focus) :+ PathSymbol.Field(member) + toPath(deep, focus) :+ PathSymbol.Extension(term, member) case other => report.errorAndAbort(unsupportedShapeInfo(focus.asTerm)) /** Field access */ @@ -165,16 +167,22 @@ object QuicklensMacros { def widenAll: TypeRepr = tpe.widen.dealias.poorMansLUB - def matchingTypeSymbol: Symbol = tpe.widenAll match { - case AndType(l, r) => - val lSym = l.matchingTypeSymbol - if l.matchingTypeSymbol != Symbol.noSymbol then lSym else r.matchingTypeSymbol - case tpe if isProduct(tpe.typeSymbol) || isSum(tpe.typeSymbol) => - tpe.typeSymbol - case tpe if isProductLike(tpe.typeSymbol) => - tpe.typeSymbol - case _ => - Symbol.noSymbol + def matchingTypeSymbol: Symbol = { + def recurse(tpe: TypeRepr): Symbol = { + tpe.widenAll match { + case AndType(l, r) => + val lSym = recurse(l) + if lSym != Symbol.noSymbol then lSym else recurse(r) + case tpe if isProduct(tpe.typeSymbol) || isSum(tpe.typeSymbol) => + tpe.typeSymbol + case tpe if isProductLike(tpe.typeSymbol) => + tpe.typeSymbol + case _ => + Symbol.noSymbol + } + } + val rSym = recurse(tpe) + if rSym != Symbol.noSymbol then rSym else tpe.typeSymbol // if everything else fails, try the original type, maybe it will have all we need } def symbolAccessorByNameOrError(obj: Term, name: String): Term = { @@ -188,14 +196,8 @@ object QuicklensMacros { objSymbol.methodMember(name) match case List(m) => Select(obj, m) - case Nil => - findExtensionMethod(objSymbol, name) match - case List((owner, extension)) => - Apply(Select(owner, extension), List(obj)) - case syms => - reportMethodError(objSymbol, name, syms.map(_._2)) case lst => - report.errorAndAbort(multipleMatchingMethods(objSymbol.name, name, lst)) + reportMethodError(objSymbol, name, lst) } def reportMethodError(sym: Symbol, name: String, lst: List[Symbol]): Nothing = { @@ -325,7 +327,7 @@ object QuicklensMacros { owner: Symbol, mod: Expr[A => A], obj: Term, - fields: Seq[(PathSymbol.Field, Seq[PathTree])] + fields: Seq[(PathSymbol.Field | PathSymbol.Extension, Seq[PathTree])] ): Term = { val objTpe = obj.tpe.widenAll val objSymbol = objTpe.matchingTypeSymbol @@ -361,12 +363,18 @@ object QuicklensMacros { } } else if isProduct(objSymbol) || isProductLike(objSymbol) then { val argsMap: Map[String, Term] = fields.map { (field, trees) => - val fieldMethod = symbolAccessorByNameOrError(obj, field.name) + val (fieldMethod, name) = field match { + case PathSymbol.Field(name) => + symbolAccessorByNameOrError (obj, name) -> name + case PathSymbol.Extension(term, name) => + val extensionMethod = symbolAccessorByNameOrError (term, name) + Apply(extensionMethod, List(obj)) -> name + } val resTerm: Term = trees.foldLeft[Term](fieldMethod) { (term, tree) => mapToCopy(owner, mod, term, tree) } - val namedArg = NamedArg(field.name, resTerm) - field.name -> namedArg + val namedArg = NamedArg(name, resTerm) + name -> namedArg }.toMap methodSymbolByNameAndArgs(objSymbol, "copy", argsMap) match case Right(copy) => @@ -421,9 +429,9 @@ object QuicklensMacros { case Nil => objTerm - case (_: PathSymbol.Field, _) :: _ => - val (fs, funs) = pathSymbols.span(_._1.isInstanceOf[PathSymbol.Field]) - val fields = fs.collect { case (p: PathSymbol.Field, trees) => p -> trees } + case (_: (PathSymbol.Field | PathSymbol.Extension), _) :: _ => + val (fs, funs) = pathSymbols.span(s => s._1.isInstanceOf[PathSymbol.Field] | s._1.isInstanceOf[PathSymbol.Extension]) + val fields = fs.collect { case (p: (PathSymbol.Field | PathSymbol.Extension), trees) => p -> trees } val withCopiedFields: Term = caseClassCopy(owner, mod, objTerm, fields) accumulateToCopy(owner, mod, withCopiedFields, funs) From 4a0c653e582d57b98807b550a06787fcb2a67b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Fri, 6 Dec 2024 17:26:37 +0100 Subject: [PATCH 11/14] Comment reason why the test is commented out --- .../softwaremill/quicklens/test/ExtensionCopyTest.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala index 0fbb832..5c85919 100644 --- a/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala +++ b/quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ExtensionCopyTest.scala @@ -21,8 +21,9 @@ object ExtensionCopyTest { } class ExtensionCopyTest extends AnyFlatSpec with Matchers { - /* it should "modify a simple class with an extension copy method" in { + // this test does compile at the moment, because we search extensions in companions only + /* class VecSimple(xp: Double, yp: Double) { val xMember = xp val yMember = yp @@ -36,10 +37,10 @@ class ExtensionCopyTest extends AnyFlatSpec with Matchers { def copy(x: Double = v.xMember, y: Double = v.yMember): VecSimple = new VecSimple(x, y) } val a = VecSimple(1, 2) - val b = a.modify(_.xMember).using(_ + 1) - println(b) + val b = a.modify(_.xMember).using(_ + 10) + b.xMember shouldEqual 11 + */ } - */ it should "modify a simple class with an extension copy method in companion" in { class VecCompanion(xp: Double, yp: Double) { From 643bab052f7a3c5a5a93ad78b9278ed35ed171b1 Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Mon, 9 Dec 2024 17:11:08 +0100 Subject: [PATCH 12/14] Some refactors --- .../quicklens/QuicklensMacros.scala | 148 ++++++++---------- 1 file changed, 63 insertions(+), 85 deletions(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index 61194e9..152b3a2 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -112,9 +112,10 @@ object QuicklensMacros { case (symbol :: tail) => PathTree.Node(Seq(symbol -> Seq(tail.toPathTree))) enum PathSymbol: - case Field(name: String) - case Extension(term: Term, name: String) - case FunctionDelegate(name: String, givn: Term, typeTree: TypeTree, args: List[Term]) + case Field(override val name: String) + case Extension(term: Term, override val name: String) + case FunctionDelegate(override val name: String, givn: Term, typeTree: TypeTree, args: List[Term]) + def name: String def equiv(other: Any): Boolean = (this, other) match case (Field(name1), Field(name2)) => name1 == name2 @@ -138,14 +139,9 @@ object QuicklensMacros { /** Method call with one type parameter and using clause */ case a @ Apply(TypeApply(Apply(TypeApply(Ident(s), _), idents), typeTrees), List(givn)) if methodSupported(s) => idents.flatMap(toPath(_, focus)) :+ PathSymbol.FunctionDelegate(s, givn, typeTrees.last, List.empty) - case Apply(obj, Seq(deep)) => // this is an extension method, which is called e.g. as x(_$1) - obj match - case Ident(ident) => - toPath(deep, focus) :+ PathSymbol.Field(ident) - case Select(term, member) => - toPath(deep, focus) :+ PathSymbol.Extension(term, member) - case other => - report.errorAndAbort(unsupportedShapeInfo(focus.asTerm)) + /** Extension method, which is called e.g. as x(_$1) */ + case Apply(obj@Select(term, member), Seq(deep)) if obj.symbol.flags.is(Flags.ExtensionMethod) => + toPath(deep, focus) :+ PathSymbol.Extension(term, member) /** Field access */ case Apply(deep, idents) => toPath(deep, focus) ++ idents.flatMap(toPath(_, focus)) @@ -167,49 +163,46 @@ object QuicklensMacros { def widenAll: TypeRepr = tpe.widen.dealias.poorMansLUB - def matchingTypeSymbol: Symbol = { - def recurse(tpe: TypeRepr): Symbol = { - tpe.widenAll match { - case AndType(l, r) => - val lSym = recurse(l) - if lSym != Symbol.noSymbol then lSym else recurse(r) - case tpe if isProduct(tpe.typeSymbol) || isSum(tpe.typeSymbol) => - tpe.typeSymbol - case tpe if isProductLike(tpe.typeSymbol) => - tpe.typeSymbol - case _ => - Symbol.noSymbol - } - } - val rSym = recurse(tpe) - if rSym != Symbol.noSymbol then rSym else tpe.typeSymbol // if everything else fails, try the original type, maybe it will have all we need + def matchingTypeSymbol: Symbol = tpe.widenAll match { + case AndType(l, r) => + val lSym = l.matchingTypeSymbol + if lSym != Symbol.noSymbol then lSym else r.matchingTypeSymbol + case tpe if isProduct(tpe.typeSymbol) || isSum(tpe.typeSymbol) || isProductLike(tpe.typeSymbol) => + tpe.typeSymbol + case _ => + Symbol.noSymbol } + extension (term: Term) + def appliedToIfNeeded(args: List[Term]): Term = + if args.isEmpty then term else term.appliedToArgs(args) + def symbolAccessorByNameOrError(obj: Term, name: String): Term = { val objTpe = obj.tpe.widenAll val objSymbol = objTpe.matchingTypeSymbol - // opaque types could find members of underlying types - do not ask them (see https://github.com/scala/scala3/issues/22143) - val mem = if !objSymbol.flags.is(Flags.Deferred) then objSymbol.fieldMember(name) else Symbol.noSymbol - if (mem != Symbol.noSymbol) - Select(obj, mem) + // opaque types can find members of underlying types - ignore them (see https://github.com/scala/scala3/issues/22143) + val fieldMemberSym = objSymbol.fieldMember(name) + if !objSymbol.flags.is(Flags.Deferred) && fieldMemberSym.exists then + Select(obj, fieldMemberSym) else objSymbol.methodMember(name) match case List(m) => Select(obj, m) case lst => - reportMethodError(objSymbol, name, lst) + report.errorAndAbort(reportMethodError(objSymbol, name, lst)) } - def reportMethodError(sym: Symbol, name: String, lst: List[Symbol]): Nothing = { - lst match - case Nil => report.errorAndAbort(noSuchMember(sym.name, name)) - case lst => report.errorAndAbort(multipleMatchingMethods(sym.name, name, lst)) + def reportMethodError(sym: Symbol, name: String, lst: List[Symbol], maybeArgNames: Option[Iterable[String]] = None): String = { + (lst, maybeArgNames) match + case (Nil, _) => noSuchMember(sym.name, name) + case (lst, None) => multipleMatchingMethods(sym.name, name, lst) + case (lst, Some(argNames)) => noSuitableMember(sym.name, name, argNames) } def methodSymbolByNameOrError(sym: Symbol, name: String): Symbol = { sym.methodMember(name) match case List(m) => m - case lst => reportMethodError(sym, name, lst) + case lst => report.errorAndAbort(reportMethodError(sym, name, lst)) } def filterMethodsByNameAndArgs(allMethods: List[Symbol], argsMap: Map[String, Term]): Option[Symbol] = { @@ -233,7 +226,7 @@ object QuicklensMacros { if !sym.flags.is(Flags.Deferred) then val memberMethods = sym.methodMember(name) filterMethodsByNameAndArgs(memberMethods, argsMap) - .toRight(if memberMethods.isEmpty then noSuchMember(sym.name, name) else noSuitableMember(sym.name, name, argsMap.keys)) + .toRight(reportMethodError(sym, name, memberMethods, Some(argsMap.keys))) else Left(s"Deferred type ${sym.name}") } @@ -242,13 +235,11 @@ object QuicklensMacros { * on which the extension is called * */ def callMethod(obj: Term, copy: Symbol, argsMap: List[Map[String, Term]]) = { + require(argsMap.size == 1 || argsMap.size == 2, s"argsMap.size should be either 1 or 2, got: ${argsMap.size} ($argsMap)") val objTpe = obj.tpe.widenAll val objSymbol = objTpe.matchingTypeSymbol - val typeParams = objTpe match { - case AppliedType(_, typeParams) => Some(typeParams) - case _ => None - } + val typeParams = objTpe.typeArgs val copyTree: DefDef = copy.tree.asInstanceOf[DefDef] val copyParams: List[(String, Option[Term])] = copyTree.termParamss.zip(argsMap) .map((params, args) => params.params.map(_.name).map(name => name -> args.get(name))) @@ -256,35 +247,23 @@ object QuicklensMacros { val args = copyParams.zipWithIndex.map { case ((n, v), _i) => val i = _i + 1 - def defaultMethod = + def defaultMethod: Term = val methodSymbol = methodSymbolByNameOrError(objSymbol, copy.name + "$default$" + i.toString) - // default values in extensions are obtained by calling a method receiving the extension parameter - val defaultMethodArgs = argsMap.dropRight(1).headOption.toList.flatMap(_.values) - if defaultMethodArgs.nonEmpty then - Apply(Select(obj, methodSymbol), defaultMethodArgs) - else - // note: this is not always correct, -Xcheck-macros shows errors here - // sometimes we should call a method with empry parameter list instead - obj.select(methodSymbol) - - // for extension methods, might need sth more like this: (or probably some weird implicit conversion) - // val defaultGetter = obj.select(symbolMethodByNameOrError(objSymbol, n)) + // default values in extension methods take the extension receiver as the first parameter + val defaultMethodArgs = argsMap.dropRight(1).flatMap(_.values) + obj.select(methodSymbol).appliedToIfNeeded(defaultMethodArgs) n -> v.getOrElse(defaultMethod) }.toMap - val argLists = copyTree.termParamss.take(argsMap.size).map(list => list.params.map(p => args(p.name))) + val argLists: List[List[Term]] = copyTree.termParamss.take(argsMap.size).map(list => list.params.map(p => args(p.name))) if copyTree.termParamss.drop(argLists.size).exists(_.params.exists(!_.symbol.flags.is(Flags.Implicit))) then report.errorAndAbort( s"Implementation limitation: Only the first parameter list of the modified case classes can be non-implicit. ${copyTree.termParamss.drop(1)}" ) - val applyOn = typeParams match { - // if the object's type is parametrised, we need to call .copy with the same type parameters - case Some(typeParams) => TypeApply(Select(obj, copy), typeParams.map(Inferred(_))) - case _ => Select(obj, copy) - } - argLists.foldLeft(applyOn)((applied, list) => Apply(applied, list)) + val withTypeParamsApplied = obj.select(copy).appliedToTypes(typeParams) + argLists.foldLeft(withTypeParamsApplied)(Apply(_, _)) } def termMethodByNameUnsafe(term: Term, name: String): Symbol = { @@ -301,26 +280,25 @@ object QuicklensMacros { (sym.flags.is(Flags.Sealed) && (sym.flags.is(Flags.Trait) || sym.flags.is(Flags.Abstract))) } - def findCompanionLikeObject(objSymbol: Symbol): Option[Symbol] = { - def optSymbol(objSymbol: Symbol) = Option.when(!objSymbol.isNoSymbol)(objSymbol) - optSymbol(objSymbol.companionModule).orElse { - // for opaque types, the companion type is not found by objSymbol.companionModule - // try to find an object by name in the owner scope - optSymbol(objSymbol.owner.fieldMember(objSymbol.name)).filter(_.flags.is(Flags.Module)) - } + def findCompanionLikeObject(objSymbol: Symbol): Symbol = { + if objSymbol.companionModule.exists then + objSymbol.companionModule + else + val namedFromOwnerScope = objSymbol.owner.fieldMember(objSymbol.name) + if namedFromOwnerScope.flags.is(Flags.Module) then namedFromOwnerScope + else Symbol.noSymbol } - def findExtensionMethod(using Quotes)(sym: Symbol, methodName: String): List[(Term, Symbol)] = { - // TODO: can we check parameter types somehow? - def isExtensionMethod(sym: Symbol): Boolean = sym.isDefDef && sym.paramSymss.headOption.exists(_.sizeIs == 1) - // TODO: try to search in symbol parent scope as well, as extension methods could be located there as well - val symbols = findCompanionLikeObject(sym).filter(_ != Symbol.noSymbol).toList - - symbols.flatMap(s => s.declaredMethods.map(Ref(s) -> _)).filter((_, m) => m.name == methodName && isExtensionMethod(m)).toList + def hasExtensionNamed(sym: Symbol, methodName: String): List[Symbol] = { + val companionSymbol = findCompanionLikeObject(sym) + if companionSymbol.exists then + companionSymbol.methodMember(methodName).filter(s => s.name == methodName && s.flags.is(Flags.ExtensionMethod)) + else + Nil } def isProductLike(sym: Symbol): Boolean = { - sym.methodMember("copy").nonEmpty || findExtensionMethod(sym, "copy").nonEmpty + sym.methodMember("copy").nonEmpty || hasExtensionNamed(sym, "copy").nonEmpty } def caseClassCopy( @@ -363,30 +341,30 @@ object QuicklensMacros { } } else if isProduct(objSymbol) || isProductLike(objSymbol) then { val argsMap: Map[String, Term] = fields.map { (field, trees) => - val (fieldMethod, name) = field match { + val fieldMethod = field match { case PathSymbol.Field(name) => - symbolAccessorByNameOrError (obj, name) -> name + symbolAccessorByNameOrError(obj, name) case PathSymbol.Extension(term, name) => - val extensionMethod = symbolAccessorByNameOrError (term, name) - Apply(extensionMethod, List(obj)) -> name + val extensionMethod = symbolAccessorByNameOrError(term, name) + Apply(extensionMethod, List(obj)) } val resTerm: Term = trees.foldLeft[Term](fieldMethod) { (term, tree) => mapToCopy(owner, mod, term, tree) } - val namedArg = NamedArg(name, resTerm) - name -> namedArg + val namedArg = NamedArg(field.name, resTerm) + field.name -> namedArg }.toMap methodSymbolByNameAndArgs(objSymbol, "copy", argsMap) match case Right(copy) => callMethod(obj, copy, List(argsMap)) case Left(error) => val objCompanion = findCompanionLikeObject(objSymbol) - objCompanion.flatMap(methodSymbolByNameAndArgs(_, "copy", argsMap).toOption) match + methodSymbolByNameAndArgs(objCompanion, "copy", argsMap).toOption match case Some(copy) => // now try to call the extension as a method, assume the object is its first parameter val extensionParameter = copy.paramSymss.headOption.map(_.headOption).flatten val argsWithObj = List(extensionParameter.map(name => name.name -> obj).toMap, argsMap) - callMethod(Ref(objCompanion.get), copy, argsWithObj) + callMethod(Ref(objCompanion), copy, argsWithObj) case None => report.errorAndAbort(error) } else report.errorAndAbort(s"Unsupported source object: must be a case class, sealed trait or class with copy method, but got: $objSymbol of type ${objTpe.show} (${obj.show})") @@ -430,7 +408,7 @@ object QuicklensMacros { objTerm case (_: (PathSymbol.Field | PathSymbol.Extension), _) :: _ => - val (fs, funs) = pathSymbols.span(s => s._1.isInstanceOf[PathSymbol.Field] | s._1.isInstanceOf[PathSymbol.Extension]) + val (fs, funs) = pathSymbols.partition((ps, _) => ps.isInstanceOf[PathSymbol.Field] || ps.isInstanceOf[PathSymbol.Extension]) val fields = fs.collect { case (p: (PathSymbol.Field | PathSymbol.Extension), trees) => p -> trees } val withCopiedFields: Term = caseClassCopy(owner, mod, objTerm, fields) accumulateToCopy(owner, mod, withCopiedFields, funs) From e6768728c0a9baab5a3d705fc75b9040f9800309 Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Mon, 9 Dec 2024 17:46:18 +0100 Subject: [PATCH 13/14] Revert back to the non-reordering fied/fun merging strategy --- .../scala-3/com/softwaremill/quicklens/QuicklensMacros.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index 152b3a2..d3ba9c2 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -408,7 +408,7 @@ object QuicklensMacros { objTerm case (_: (PathSymbol.Field | PathSymbol.Extension), _) :: _ => - val (fs, funs) = pathSymbols.partition((ps, _) => ps.isInstanceOf[PathSymbol.Field] || ps.isInstanceOf[PathSymbol.Extension]) + val (fs, funs) = pathSymbols.span((ps, _) => ps.isInstanceOf[PathSymbol.Field] || ps.isInstanceOf[PathSymbol.Extension]) val fields = fs.collect { case (p: (PathSymbol.Field | PathSymbol.Extension), trees) => p -> trees } val withCopiedFields: Term = caseClassCopy(owner, mod, objTerm, fields) accumulateToCopy(owner, mod, withCopiedFields, funs) From e7dc9ba770f8ec441f3040ab8f9ec88072da144f Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Tue, 17 Dec 2024 16:30:04 +0100 Subject: [PATCH 14/14] Remove leftover TODO --- .../scala-3/com/softwaremill/quicklens/QuicklensMacros.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala index d3ba9c2..6215ed3 100644 --- a/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala +++ b/quicklens/src/main/scala-3/com/softwaremill/quicklens/QuicklensMacros.scala @@ -427,7 +427,6 @@ object QuicklensMacros { def mapToCopy(owner: Symbol, mod: Expr[A => A], objTerm: Term, pathTree: PathTree): Term = pathTree match { case PathTree.Empty => val apply = termMethodByNameUnsafe(mod.asTerm, "apply") - // TODO: calling extension may be necessary here Apply(Select(mod.asTerm, apply), List(objTerm)) case PathTree.Node(children) => accumulateToCopy(owner, mod, objTerm, children)