From 00c82bb6c6384513a736faf1437cbf28b6efb856 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 15 Dec 2022 18:36:00 +0100 Subject: [PATCH 1/4] [Proof of Concept] Code generation via rewriting errors in macro annotations This commit prototypes the proposal in https://contributors.scala-lang.org/t/scala-3-macro-annotations-and-code-generation/6035 to use a macro annotation to do code generation. Since the -rewrite mechanism isn't currently accessible to macros, we just print an error indicating what part of the source code needs to be rewritten. We implement support for a subset of `@data`: only the `withField` methods are added (note that equals/hashCode/toString wouldn't need to be code-generated since they are already defined in `AnyRef`, so a macro annotation can override them as demonstrated in the existing `tests/run-macros/annot-mod-class-data` test). This is only one possible path forward. Alternatively, we could recommend using scalafix for this usecase, and then the compiler itself wouldn't have to grow code generation abilities, but libraries defining macro annotation might not like to be tied to an external tool. --- tests/neg-macros/annot-codegen.check | 13 +++ tests/neg-macros/annot-codegen/Macro_1.scala | 83 ++++++++++++++++++++ tests/neg-macros/annot-codegen/Test_2.scala | 9 +++ 3 files changed, 105 insertions(+) create mode 100644 tests/neg-macros/annot-codegen.check create mode 100644 tests/neg-macros/annot-codegen/Macro_1.scala create mode 100644 tests/neg-macros/annot-codegen/Test_2.scala diff --git a/tests/neg-macros/annot-codegen.check b/tests/neg-macros/annot-codegen.check new file mode 100644 index 000000000000..526f3cb7faad --- /dev/null +++ b/tests/neg-macros/annot-codegen.check @@ -0,0 +1,13 @@ + +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:9:4 -------------------------------------------------------------- +9 | new Bar("", two, 1, 1.0) // error: body needs to be replaced + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | Replace the underline code by: + | data.generated[Bar]() +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:3:80 ------------------------------------------------------------- +3 |@data class Bar(val one: String, val two: Int, val three: Int, val four: Double): // error: additional methods needed + | ^ + | @data requires the following additional method(s): + | + | def withThree(three: scala.Int): Bar = data.generated[Bar]() + | def withFour(four: scala.Double): Bar = data.generated[Bar]() diff --git a/tests/neg-macros/annot-codegen/Macro_1.scala b/tests/neg-macros/annot-codegen/Macro_1.scala new file mode 100644 index 000000000000..e1835f316c15 --- /dev/null +++ b/tests/neg-macros/annot-codegen/Macro_1.scala @@ -0,0 +1,83 @@ +import scala.annotation.{experimental, MacroAnnotation} +import scala.quoted.* +import scala.collection.mutable.ArrayBuffer + +@experimental +class data extends MacroAnnotation: + import data.* + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + import quotes.reflect.* + tree match + case cdef: ClassDef => + val classPatches = ArrayBuffer[String]() + + val cls = tree.symbol + val expectedBody = s"data.generated[${cls.name}]()" // FIXME: handle type parameters + + val params = paramNames(cls) + for param <- params do + val withParam = With(param) + val paramType = cls.declaredField(param).info + val existingOpt = + cls.declaredMethod(withParam).find(o => + val paramss = o.paramSymss + paramss.size == 1 && paramss(0).size == 1 + && paramss(0)(0).name == param // FIXME: if the parameter name is incorrect, propose rewriting it + && paramss(0)(0).info == paramType // FIXME: if the parameter type changed, propose rewriting it + ) + existingOpt match + case Some(existing) => existing.tree match + case tree: DefDef => + tree.rhs match + case Some(rhs) => rhs.asExpr match + case '{data.generated[t]()} => + // The correct method is already present, nothing to do + case _ => + report.error(s"Replace the underline code by:\n$expectedBody", rhs.pos) + case _ => + report.error("FIXME: Passing -Yretain-trees is currently needed for this macro to work") + case _ => + case _ => + // The method is not present + classPatches += + s"def $withParam($param: ${paramType.show}): ${cls.name} = $expectedBody" + + + val ctr = cdef.constructor + val endPos = Position(ctr.pos.sourceFile, ctr.pos.end, ctr.pos.end) + // TODO: if the class has no existing body, we also need to add braces or ':' + if classPatches.nonEmpty then + report.error("@data requires the following additional method(s):\n\n" + + classPatches.mkString("\n"), endPos) + case _ => + report.error("Annotation only supports `class`") + List(tree) + +object data: + inline def generated[T](): T = ${generatedImpl[T]} + private def generatedImpl[T: Type](using Quotes): Expr[T] = + import quotes.reflect.* + val meth = Symbol.spliceOwner.owner + val cls = meth.owner + val params = paramNames(cls) + meth.name match + case With(paramName) => + val localParam = meth.paramSymss(0)(0) + val body = + Apply(Select(New(TypeIdent(cls)), cls.primaryConstructor), + params.map(p => if p == paramName then Ref(localParam) else Ref(cls.declaredField(p)))) + body.asExprOf[T] + case _ => + report.errorAndAbort("@data.generated used in invalid context") + + def paramNames(using Quotes)(cls: quotes.reflect.Symbol): List[String] = + cls.primaryConstructor.paramSymss.head.map(_.name) + +private object With: + def apply(paramName: String): String = + s"with${paramName.head.toUpper}${paramName.tail}" + def unapply(methodName: String): Option[String] = methodName match + case s"with$rest" => + Some(s"${rest.head.toLower}${rest.tail}") + case _ => + None diff --git a/tests/neg-macros/annot-codegen/Test_2.scala b/tests/neg-macros/annot-codegen/Test_2.scala new file mode 100644 index 000000000000..8a6a62f37592 --- /dev/null +++ b/tests/neg-macros/annot-codegen/Test_2.scala @@ -0,0 +1,9 @@ +// scalac: -Yretain-trees + +@data class Bar(val one: String, val two: Int, val three: Int, val four: Double): // error: additional methods needed + // This definition is OK and will be left as is + def withOne(one: String): Bar = + data.generated[Bar]() + + def withTwo(two: Int): Bar = + new Bar("", two, 1, 1.0) // error: body needs to be replaced From 09b36c1c3aa8997c1aba1245c878fa255cdbefd7 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 20 Dec 2022 10:42:58 +0100 Subject: [PATCH 2/4] Remove need for -Yretain-trees --- tests/neg-macros/annot-codegen.check | 8 ++++---- tests/neg-macros/annot-codegen/Macro_1.scala | 15 +++++++-------- tests/neg-macros/annot-codegen/Test_2.scala | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/tests/neg-macros/annot-codegen.check b/tests/neg-macros/annot-codegen.check index 526f3cb7faad..65cbe5505c8e 100644 --- a/tests/neg-macros/annot-codegen.check +++ b/tests/neg-macros/annot-codegen.check @@ -1,11 +1,11 @@ --- Error: tests/neg-macros/annot-codegen/Test_2.scala:9:4 -------------------------------------------------------------- -9 | new Bar("", two, 1, 1.0) // error: body needs to be replaced +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:7:4 -------------------------------------------------------------- +7 | new Bar("", two, 1, 1.0) // error: body needs to be replaced | ^^^^^^^^^^^^^^^^^^^^^^^^ | Replace the underline code by: | data.generated[Bar]() --- Error: tests/neg-macros/annot-codegen/Test_2.scala:3:80 ------------------------------------------------------------- -3 |@data class Bar(val one: String, val two: Int, val three: Int, val four: Double): // error: additional methods needed +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:1:80 ------------------------------------------------------------- +1 |@data class Bar(val one: String, val two: Int, val three: Int, val four: Double): // error: additional methods needed | ^ | @data requires the following additional method(s): | diff --git a/tests/neg-macros/annot-codegen/Macro_1.scala b/tests/neg-macros/annot-codegen/Macro_1.scala index e1835f316c15..4006e4b3f024 100644 --- a/tests/neg-macros/annot-codegen/Macro_1.scala +++ b/tests/neg-macros/annot-codegen/Macro_1.scala @@ -19,24 +19,23 @@ class data extends MacroAnnotation: val withParam = With(param) val paramType = cls.declaredField(param).info val existingOpt = - cls.declaredMethod(withParam).find(o => - val paramss = o.paramSymss - paramss.size == 1 && paramss(0).size == 1 + cdef.body.find(stat => + val paramss = stat.symbol.paramSymss + stat.symbol.name == withParam + && paramss.size == 1 && paramss(0).size == 1 && paramss(0)(0).name == param // FIXME: if the parameter name is incorrect, propose rewriting it && paramss(0)(0).info == paramType // FIXME: if the parameter type changed, propose rewriting it ) existingOpt match - case Some(existing) => existing.tree match - case tree: DefDef => - tree.rhs match + case Some(tree: DefDef) => + tree.rhs match case Some(rhs) => rhs.asExpr match case '{data.generated[t]()} => // The correct method is already present, nothing to do case _ => report.error(s"Replace the underline code by:\n$expectedBody", rhs.pos) case _ => - report.error("FIXME: Passing -Yretain-trees is currently needed for this macro to work") - case _ => + report.error(s"Replace the underline code by:\n${tree.show} = $expectedBody", tree.pos) case _ => // The method is not present classPatches += diff --git a/tests/neg-macros/annot-codegen/Test_2.scala b/tests/neg-macros/annot-codegen/Test_2.scala index 8a6a62f37592..9d7c5bbc9629 100644 --- a/tests/neg-macros/annot-codegen/Test_2.scala +++ b/tests/neg-macros/annot-codegen/Test_2.scala @@ -1,5 +1,3 @@ -// scalac: -Yretain-trees - @data class Bar(val one: String, val two: Int, val three: Int, val four: Double): // error: additional methods needed // This definition is OK and will be left as is def withOne(one: String): Bar = @@ -7,3 +5,5 @@ def withTwo(two: Int): Bar = new Bar("", two, 1, 1.0) // error: body needs to be replaced + + def withThree(three: Int): Bar // error: body needs to be data.generate[Bar]() From 59e33bb5182529d21e06dd4ae9476e6b87d1be9c Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 20 Dec 2022 11:19:31 +0100 Subject: [PATCH 3/4] Add partial support for type parameters --- tests/neg-macros/annot-codegen.check | 24 +++++++- tests/neg-macros/annot-codegen/Macro_1.scala | 63 +++++++++++--------- tests/neg-macros/annot-codegen/Test_2.scala | 6 ++ 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/tests/neg-macros/annot-codegen.check b/tests/neg-macros/annot-codegen.check index 65cbe5505c8e..83f844f6131f 100644 --- a/tests/neg-macros/annot-codegen.check +++ b/tests/neg-macros/annot-codegen.check @@ -4,10 +4,32 @@ | ^^^^^^^^^^^^^^^^^^^^^^^^ | Replace the underline code by: | data.generated[Bar]() +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:9:6 -------------------------------------------------------------- +9 | def withThree(three: Int): Bar // error: body needs to be data.generate[Bar]() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Replace the underline code by: + | def withThree(three: scala.Int): Bar = data.generated[Bar]() -- Error: tests/neg-macros/annot-codegen/Test_2.scala:1:80 ------------------------------------------------------------- 1 |@data class Bar(val one: String, val two: Int, val three: Int, val four: Double): // error: additional methods needed | ^ | @data requires the following additional method(s): | - | def withThree(three: scala.Int): Bar = data.generated[Bar]() | def withFour(four: scala.Double): Bar = data.generated[Bar]() +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:12:32 ------------------------------------------------------------ +12 | def withOne(one: T): Baz[T] = ??? // error + | ^^^ + | Replace the underline code by: + | data.generated[Baz[Baz.this.T]]() +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:13:6 ------------------------------------------------------------- +13 | def withTwo(two: Int): Baz[T] // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Replace the underline code by: + | def withTwo(two: scala.Int): Baz[Baz.this.T] = data.generated[Baz[Baz.this.T]]() +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:14:38 ------------------------------------------------------------ +14 | def withThree(three: Int): Baz[T] = data.generated[Baz[T]]() // error // FIXME: should be supported + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | constructor Baz in class Baz does not take parameters +-- Error: tests/neg-macros/annot-codegen/Test_2.scala:15:36 ------------------------------------------------------------ +15 | def withFour(four: Int): Baz[T] = data.generated[Nothing]() // error // FIXME: wrong error + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | constructor Baz in class Baz does not take parameters diff --git a/tests/neg-macros/annot-codegen/Macro_1.scala b/tests/neg-macros/annot-codegen/Macro_1.scala index 4006e4b3f024..3727b04e2e0d 100644 --- a/tests/neg-macros/annot-codegen/Macro_1.scala +++ b/tests/neg-macros/annot-codegen/Macro_1.scala @@ -12,34 +12,40 @@ class data extends MacroAnnotation: val classPatches = ArrayBuffer[String]() val cls = tree.symbol - val expectedBody = s"data.generated[${cls.name}]()" // FIXME: handle type parameters + val typeParams = cdef.body.map(_.symbol).filter(_.isType) + val clsTpe = + if typeParams.isEmpty then cls.typeRef + else AppliedType(cls.typeRef, typeParams.map(_.typeRef)) + clsTpe.asType match + case '[t] => + val expectedBody = '{ data.generated[t]() }.show - val params = paramNames(cls) - for param <- params do - val withParam = With(param) - val paramType = cls.declaredField(param).info - val existingOpt = - cdef.body.find(stat => - val paramss = stat.symbol.paramSymss - stat.symbol.name == withParam - && paramss.size == 1 && paramss(0).size == 1 - && paramss(0)(0).name == param // FIXME: if the parameter name is incorrect, propose rewriting it - && paramss(0)(0).info == paramType // FIXME: if the parameter type changed, propose rewriting it - ) - existingOpt match - case Some(tree: DefDef) => - tree.rhs match - case Some(rhs) => rhs.asExpr match - case '{data.generated[t]()} => - // The correct method is already present, nothing to do - case _ => - report.error(s"Replace the underline code by:\n$expectedBody", rhs.pos) + val params = paramNames(cls) + for param <- params do + val withParam = With(param) + val paramType = cls.declaredField(param).info + val existingOpt = + cdef.body.find(stat => + val paramss = stat.symbol.paramSymss + stat.symbol.name == withParam + && paramss.size == 1 && paramss(0).size == 1 + && paramss(0)(0).name == param // FIXME: if the parameter name is incorrect, propose rewriting it + && paramss(0)(0).info == paramType // FIXME: if the parameter type changed, propose rewriting it + ) + existingOpt match + case Some(tree: DefDef) => + tree.rhs match + case Some(rhs) => rhs.asExpr match + case '{data.generated[`t`]()} => + // The correct method is already present, nothing to do + case _ => + report.error(s"Replace the underline code by:\n$expectedBody", rhs.pos) + case _ => + report.error(s"Replace the underline code by:\n${tree.show} = $expectedBody", tree.pos) case _ => - report.error(s"Replace the underline code by:\n${tree.show} = $expectedBody", tree.pos) - case _ => - // The method is not present - classPatches += - s"def $withParam($param: ${paramType.show}): ${cls.name} = $expectedBody" + // The method is not present + classPatches += + s"def $withParam($param: ${paramType.show}): ${Type.show[t]} = $expectedBody" val ctr = cdef.constructor @@ -53,6 +59,7 @@ class data extends MacroAnnotation: List(tree) object data: + // TODO: Is T necessary? Could this be transparent? inline def generated[T](): T = ${generatedImpl[T]} private def generatedImpl[T: Type](using Quotes): Expr[T] = import quotes.reflect.* @@ -62,7 +69,7 @@ object data: meth.name match case With(paramName) => val localParam = meth.paramSymss(0)(0) - val body = + val body = // FIXME handle type parameters Apply(Select(New(TypeIdent(cls)), cls.primaryConstructor), params.map(p => if p == paramName then Ref(localParam) else Ref(cls.declaredField(p)))) body.asExprOf[T] @@ -70,7 +77,7 @@ object data: report.errorAndAbort("@data.generated used in invalid context") def paramNames(using Quotes)(cls: quotes.reflect.Symbol): List[String] = - cls.primaryConstructor.paramSymss.head.map(_.name) + cls.primaryConstructor.paramSymss.dropWhile(_.headOption.exists(_.isType)).head.map(_.name) private object With: def apply(paramName: String): String = diff --git a/tests/neg-macros/annot-codegen/Test_2.scala b/tests/neg-macros/annot-codegen/Test_2.scala index 9d7c5bbc9629..8181d80da85e 100644 --- a/tests/neg-macros/annot-codegen/Test_2.scala +++ b/tests/neg-macros/annot-codegen/Test_2.scala @@ -7,3 +7,9 @@ new Bar("", two, 1, 1.0) // error: body needs to be replaced def withThree(three: Int): Bar // error: body needs to be data.generate[Bar]() + +@data class Baz[T](val one: T, val two: Int, val three: Int, val four: Int): + def withOne(one: T): Baz[T] = ??? // error + def withTwo(two: Int): Baz[T] // error + def withThree(three: Int): Baz[T] = data.generated[Baz[T]]() // error // FIXME: should be supported + def withFour(four: Int): Baz[T] = data.generated[Nothing]() // error // FIXME: wrong error From b4e0d9911d153b5e2829ad651a6bb9e70272cfbf Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 3 Jan 2023 20:08:13 +0100 Subject: [PATCH 4/4] Use the same quote for matching and printing using Quotes#matches We can't just pattern match on it because of #16543. --- tests/neg-macros/annot-codegen/Macro_1.scala | 54 ++++++++++---------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/tests/neg-macros/annot-codegen/Macro_1.scala b/tests/neg-macros/annot-codegen/Macro_1.scala index 3727b04e2e0d..18e75c185752 100644 --- a/tests/neg-macros/annot-codegen/Macro_1.scala +++ b/tests/neg-macros/annot-codegen/Macro_1.scala @@ -16,36 +16,34 @@ class data extends MacroAnnotation: val clsTpe = if typeParams.isEmpty then cls.typeRef else AppliedType(cls.typeRef, typeParams.map(_.typeRef)) - clsTpe.asType match - case '[t] => - val expectedBody = '{ data.generated[t]() }.show + val expectedBody = + clsTpe.asType match + case '[t] => '{ data.generated[t]() } - val params = paramNames(cls) - for param <- params do - val withParam = With(param) - val paramType = cls.declaredField(param).info - val existingOpt = - cdef.body.find(stat => - val paramss = stat.symbol.paramSymss - stat.symbol.name == withParam - && paramss.size == 1 && paramss(0).size == 1 - && paramss(0)(0).name == param // FIXME: if the parameter name is incorrect, propose rewriting it - && paramss(0)(0).info == paramType // FIXME: if the parameter type changed, propose rewriting it - ) - existingOpt match - case Some(tree: DefDef) => - tree.rhs match - case Some(rhs) => rhs.asExpr match - case '{data.generated[`t`]()} => - // The correct method is already present, nothing to do - case _ => - report.error(s"Replace the underline code by:\n$expectedBody", rhs.pos) - case _ => - report.error(s"Replace the underline code by:\n${tree.show} = $expectedBody", tree.pos) + val params = paramNames(cls) + for param <- params do + val withParam = With(param) + val paramType = cls.declaredField(param).info + val existingOpt = + cdef.body.find(stat => + val paramss = stat.symbol.paramSymss + stat.symbol.name == withParam + && paramss.size == 1 && paramss(0).size == 1 + && paramss(0)(0).name == param // FIXME: if the parameter name is incorrect, propose rewriting it + && paramss(0)(0).info == paramType // FIXME: if the parameter type changed, propose rewriting it + ) + existingOpt match + case Some(tree: DefDef) => + tree.rhs match + case Some(rhs) => + if !rhs.asExpr.matches(expectedBody) then + report.error(s"Replace the underline code by:\n${expectedBody.show}", rhs.pos) case _ => - // The method is not present - classPatches += - s"def $withParam($param: ${paramType.show}): ${Type.show[t]} = $expectedBody" + report.error(s"Replace the underline code by:\n${tree.show} = ${expectedBody.show}", tree.pos) + case _ => + // The method is not present + classPatches += + s"def $withParam($param: ${paramType.show}): ${clsTpe.show} = ${expectedBody.show}" val ctr = cdef.constructor