From b1a0cdc3be40f379be6b63d574715446b72cb28a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Mon, 30 Nov 2020 14:02:56 +0100 Subject: [PATCH 1/7] Implement bean property generation --- .../dotty/tools/dotc/core/Definitions.scala | 2 + .../tools/dotc/transform/BeanProperties.scala | 61 +++++++++++++++++++ .../tools/dotc/transform/PostTyper.scala | 3 + 3 files changed, 66 insertions(+) create mode 100644 compiler/src/dotty/tools/dotc/transform/BeanProperties.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index cca01dd32a48..038ec31786c6 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -890,6 +890,8 @@ class Definitions { // Annotation classes @tu lazy val AnnotationDefaultAnnot: ClassSymbol = requiredClass("scala.annotation.internal.AnnotationDefault") + @tu lazy val BeanPropertyAnnot: ClassSymbol = requiredClass("scala.beans.BeanProperty") + @tu lazy val BooleanBeanPropertyAnnot: ClassSymbol = requiredClass("scala.beans.BooleanBeanProperty") @tu lazy val BodyAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Body") @tu lazy val ChildAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Child") @tu lazy val ContextResultCountAnnot: ClassSymbol = requiredClass("scala.annotation.internal.ContextResultCount") diff --git a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala new file mode 100644 index 000000000000..f25791a73650 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala @@ -0,0 +1,61 @@ +package dotty.tools.dotc +package transform + +import core._ +import ast.tpd._ +import Contexts.Context +import SymDenotations._ +import Symbols.newSymbol +import Decorators._ +import Flags._ +import Names._ +import Types._ + +import DenotTransformers._ + +class BeanProperties(thisPhase: DenotTransformer): + def addBeanMethods(impl: Template)(using Context): Template = + val origBody = impl.body + cpy.Template(impl)(body = impl.body.flatMap { + case v: ValDef => generateAccessors(v) + case _ => Nil + } ::: origBody) + + def generateAccessors(valDef: ValDef)(using Context): List[Tree] = + import Symbols.defn + + def generateGetter(valDef: ValDef)(using Context) : Tree = + val prefix = if valDef.symbol.denot.hasAnnotation(defn.BooleanBeanPropertyAnnot) then "is" else "get" + val meth = newSymbol( + owner = summon[Context].owner, + name = prefixedName(prefix, valDef.name), + flags = Method | Permanent | Synthetic, + info = MethodType(Nil, valDef.denot.info)) + .enteredAfter(thisPhase).asTerm + meth.addAnnotations(valDef.symbol.annotations) + val body: Tree = ref(valDef.symbol) + DefDef(meth, body) + + def maybeGenerateSetter(valDef: ValDef)(using Context): Option[Tree] = + Option.when(valDef.denot.asSymDenotation.flags.is(Mutable)) { + val owner = summon[Context].owner + val meth = newSymbol( + owner, + name = prefixedName("set", valDef.name), + flags = Method | Permanent | Synthetic, + info = MethodType(valDef.name :: Nil, valDef.denot.info :: Nil, defn.UnitType) + ).enteredAfter(thisPhase).asTerm + meth.addAnnotations(valDef.symbol.annotations) + def body(params: List[List[Tree]]): Tree = Assign(ref(valDef.symbol), params.head.head) + DefDef(meth, body) + } + + def prefixedName(prefix: String, valName: Name) = + (prefix + valName.lastPart.toString.capitalize).toTermName + + extension(a: SymDenotation) def isApplicable = a.hasAnnotation(defn.BeanPropertyAnnot) || a.hasAnnotation(defn.BooleanBeanPropertyAnnot) + + if valDef.denot.symbol.isApplicable then + generateGetter(valDef) +: maybeGenerateSetter(valDef) ++: Nil + else Nil + end generateAccessors diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index ba0337a797f3..09806b2b1679 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -75,6 +75,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase val superAcc: SuperAccessors = new SuperAccessors(thisPhase) val synthMbr: SyntheticMembers = new SyntheticMembers(thisPhase) + val beanProps: BeanProperties = new BeanProperties(thisPhase) private def newPart(tree: Tree): Option[New] = methPart(tree) match { case Select(nu: New, _) => Some(nu) @@ -322,8 +323,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase withNoCheckNews(templ.parents.flatMap(newPart)) { forwardParamAccessors(templ) synthMbr.addSyntheticMembers( + beanProps.addBeanMethods( superAcc.wrapTemplate(templ)( super.transform(_).asInstanceOf[Template])) + ) } case tree: ValDef => val tree1 = cpy.ValDef(tree)(rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) From 02faed3ae4745be11f19eb13b86a11db483e5c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Tue, 1 Dec 2020 17:59:36 +0100 Subject: [PATCH 2/7] Basic test case --- tests/run/beans.check | 5 +++++ tests/run/beans/A_2.scala | 9 +++++++++ tests/run/beans/LibraryAnnotation_1.java | 7 +++++++ tests/run/beans/Test_3.java | 15 +++++++++++++++ tests/run/beans/Test_4.scala | 4 ++++ 5 files changed, 40 insertions(+) create mode 100644 tests/run/beans.check create mode 100644 tests/run/beans/A_2.scala create mode 100644 tests/run/beans/LibraryAnnotation_1.java create mode 100644 tests/run/beans/Test_3.java create mode 100644 tests/run/beans/Test_4.scala diff --git a/tests/run/beans.check b/tests/run/beans.check new file mode 100644 index 000000000000..5a150fc9a7b9 --- /dev/null +++ b/tests/run/beans.check @@ -0,0 +1,5 @@ +4 +true +[@beans.LibraryAnnotation_1()] +some text +other text diff --git a/tests/run/beans/A_2.scala b/tests/run/beans/A_2.scala new file mode 100644 index 000000000000..5f081035549b --- /dev/null +++ b/tests/run/beans/A_2.scala @@ -0,0 +1,9 @@ +class A { + @scala.beans.BeanProperty val x = 4 + @scala.beans.BooleanBeanProperty val y = true + @scala.beans.BeanProperty var mutableOneWithLongName = "some text" + + @scala.beans.BeanProperty + @beans.LibraryAnnotation_1 + val retainingAnnotation = 5 +} diff --git a/tests/run/beans/LibraryAnnotation_1.java b/tests/run/beans/LibraryAnnotation_1.java new file mode 100644 index 000000000000..16b213bbfb40 --- /dev/null +++ b/tests/run/beans/LibraryAnnotation_1.java @@ -0,0 +1,7 @@ +package beans; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +public @interface LibraryAnnotation_1 {} \ No newline at end of file diff --git a/tests/run/beans/Test_3.java b/tests/run/beans/Test_3.java new file mode 100644 index 000000000000..9122355841a3 --- /dev/null +++ b/tests/run/beans/Test_3.java @@ -0,0 +1,15 @@ +import java.util.Arrays; + +class JavaTest { + public A run() throws ReflectiveOperationException{ + A a = new A(); + System.out.println(a.getX()); + System.out.println(a.isY()); + + System.out.println(Arrays.asList(a.getClass().getMethod("getRetainingAnnotation").getAnnotations())); + + System.out.println(a.getMutableOneWithLongName()); + a.setMutableOneWithLongName("other text"); + return a; + } +} \ No newline at end of file diff --git a/tests/run/beans/Test_4.scala b/tests/run/beans/Test_4.scala new file mode 100644 index 000000000000..807d91312b12 --- /dev/null +++ b/tests/run/beans/Test_4.scala @@ -0,0 +1,4 @@ +object Test: + def main(args: Array[String]) = + val a = JavaTest().run() + println(a.mutableOneWithLongName) \ No newline at end of file From cd7006e98fe93e0d79d4659a6885a696150c443d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Wed, 2 Dec 2020 13:56:10 +0100 Subject: [PATCH 3/7] Add test for overriding bean property --- tests/run/beans.check | 1 + tests/run/beans/A_2.scala | 12 ++++++++++-- tests/run/beans/Test_3.java | 1 + 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/run/beans.check b/tests/run/beans.check index 5a150fc9a7b9..eb70eefb720c 100644 --- a/tests/run/beans.check +++ b/tests/run/beans.check @@ -1,5 +1,6 @@ 4 true +10 [@beans.LibraryAnnotation_1()] some text other text diff --git a/tests/run/beans/A_2.scala b/tests/run/beans/A_2.scala index 5f081035549b..309abc5b4e64 100644 --- a/tests/run/beans/A_2.scala +++ b/tests/run/beans/A_2.scala @@ -1,4 +1,4 @@ -class A { +class A: @scala.beans.BeanProperty val x = 4 @scala.beans.BooleanBeanProperty val y = true @scala.beans.BeanProperty var mutableOneWithLongName = "some text" @@ -6,4 +6,12 @@ class A { @scala.beans.BeanProperty @beans.LibraryAnnotation_1 val retainingAnnotation = 5 -} + +trait T: + @scala.beans.BeanProperty val x: Int + +class T1 extends T: + override val x = 5 + +class T2 extends T1: + override val x = 10 \ No newline at end of file diff --git a/tests/run/beans/Test_3.java b/tests/run/beans/Test_3.java index 9122355841a3..4ac967419e50 100644 --- a/tests/run/beans/Test_3.java +++ b/tests/run/beans/Test_3.java @@ -5,6 +5,7 @@ public A run() throws ReflectiveOperationException{ A a = new A(); System.out.println(a.getX()); System.out.println(a.isY()); + System.out.println(new T2().getX()); System.out.println(Arrays.asList(a.getClass().getMethod("getRetainingAnnotation").getAnnotations())); From f6b18ac3be9e8f8971272076944a661e99260029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Thu, 3 Dec 2020 15:15:18 +0100 Subject: [PATCH 4/7] Add filtering out synthetic accessors from unpickled templates --- .../src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 9 ++++++--- tests/neg/beanAccessorsLeaking/A_1.scala | 2 ++ tests/neg/beanAccessorsLeaking/B_2.scala | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 tests/neg/beanAccessorsLeaking/A_1.scala create mode 100644 tests/neg/beanAccessorsLeaking/B_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 2bbdc68f9cc2..9b6094bf88e8 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -582,12 +582,15 @@ class TreeUnpickler(reader: TastyReader, else newSymbol(ctx.owner, name, flags, completer, privateWithin, coord) } - sym.annotations = annotFns.map(_(sym.owner)) + val annots = annotFns.map(_(sym.owner)) + sym.annotations = annots if sym.isOpaqueAlias then sym.setFlag(Deferred) + val isSyntheticBeanAccessor = flags.isAllOf(Method | Synthetic) && + annots.exists(a => a.matches(defn.BeanPropertyAnnot) || a.matches(defn.BooleanBeanPropertyAnnot)) val isScala2MacroDefinedInScala3 = flags.is(Macro, butNot = Inline) && flags.is(Erased) ctx.owner match { - case cls: ClassSymbol if !isScala2MacroDefinedInScala3 || cls == defn.StringContextClass => - // Enter all members of classes that are not Scala 2 macros. + case cls: ClassSymbol if (!isScala2MacroDefinedInScala3 || cls == defn.StringContextClass) && !isSyntheticBeanAccessor => + // Enter all members of classes that are not Scala 2 macros or synthetic accessors. // // For `StringContext`, enter `s`, `f` and `raw` // These definitions will be entered when defined in Scala 2. It is fine to enter them diff --git a/tests/neg/beanAccessorsLeaking/A_1.scala b/tests/neg/beanAccessorsLeaking/A_1.scala new file mode 100644 index 000000000000..720c113d5289 --- /dev/null +++ b/tests/neg/beanAccessorsLeaking/A_1.scala @@ -0,0 +1,2 @@ +class A: + @scala.beans.BeanProperty val x = 6 \ No newline at end of file diff --git a/tests/neg/beanAccessorsLeaking/B_2.scala b/tests/neg/beanAccessorsLeaking/B_2.scala new file mode 100644 index 000000000000..bb551770a49f --- /dev/null +++ b/tests/neg/beanAccessorsLeaking/B_2.scala @@ -0,0 +1,2 @@ +class B(val a: A): + def x = a.getX() // error \ No newline at end of file From a0c523dcf0164b2665ff91ae11cf154e8f3a6ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Thu, 3 Dec 2020 15:23:50 +0100 Subject: [PATCH 5/7] Remove Permanent flag from sythetic accessors --- .../dotty/tools/dotc/transform/BeanProperties.scala | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala index f25791a73650..e60d455a293e 100644 --- a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala +++ b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala @@ -3,7 +3,7 @@ package transform import core._ import ast.tpd._ -import Contexts.Context +import Contexts._ import SymDenotations._ import Symbols.newSymbol import Decorators._ @@ -27,9 +27,9 @@ class BeanProperties(thisPhase: DenotTransformer): def generateGetter(valDef: ValDef)(using Context) : Tree = val prefix = if valDef.symbol.denot.hasAnnotation(defn.BooleanBeanPropertyAnnot) then "is" else "get" val meth = newSymbol( - owner = summon[Context].owner, + owner = ctx.owner, name = prefixedName(prefix, valDef.name), - flags = Method | Permanent | Synthetic, + flags = Method | Synthetic, info = MethodType(Nil, valDef.denot.info)) .enteredAfter(thisPhase).asTerm meth.addAnnotations(valDef.symbol.annotations) @@ -38,7 +38,7 @@ class BeanProperties(thisPhase: DenotTransformer): def maybeGenerateSetter(valDef: ValDef)(using Context): Option[Tree] = Option.when(valDef.denot.asSymDenotation.flags.is(Mutable)) { - val owner = summon[Context].owner + val owner = ctx.owner val meth = newSymbol( owner, name = prefixedName("set", valDef.name), @@ -53,9 +53,8 @@ class BeanProperties(thisPhase: DenotTransformer): def prefixedName(prefix: String, valName: Name) = (prefix + valName.lastPart.toString.capitalize).toTermName - extension(a: SymDenotation) def isApplicable = a.hasAnnotation(defn.BeanPropertyAnnot) || a.hasAnnotation(defn.BooleanBeanPropertyAnnot) - - if valDef.denot.symbol.isApplicable then + val symbol = valDef.denot.symbol + if symbol.hasAnnotation(defn.BeanPropertyAnnot) || symbol.hasAnnotation(defn.BooleanBeanPropertyAnnot) then generateGetter(valDef) +: maybeGenerateSetter(valDef) ++: Nil else Nil end generateAccessors From 9f08c03ef32bf995e632cc41a55d09ff29032e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Mon, 7 Dec 2020 13:33:24 +0100 Subject: [PATCH 6/7] Fix missing coordinates --- .../tools/dotc/transform/BeanProperties.scala | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala index e60d455a293e..1a003312cd8d 100644 --- a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala +++ b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala @@ -3,6 +3,7 @@ package transform import core._ import ast.tpd._ +import Annotations._ import Contexts._ import SymDenotations._ import Symbols.newSymbol @@ -10,6 +11,7 @@ import Decorators._ import Flags._ import Names._ import Types._ +import util.Spans._ import DenotTransformers._ @@ -24,26 +26,28 @@ class BeanProperties(thisPhase: DenotTransformer): def generateAccessors(valDef: ValDef)(using Context): List[Tree] = import Symbols.defn - def generateGetter(valDef: ValDef)(using Context) : Tree = - val prefix = if valDef.symbol.denot.hasAnnotation(defn.BooleanBeanPropertyAnnot) then "is" else "get" + def generateGetter(valDef: ValDef, annot: Annotation)(using Context) : Tree = + val prefix = if annot matches defn.BooleanBeanPropertyAnnot then "is" else "get" val meth = newSymbol( owner = ctx.owner, name = prefixedName(prefix, valDef.name), flags = Method | Synthetic, - info = MethodType(Nil, valDef.denot.info)) - .enteredAfter(thisPhase).asTerm + info = MethodType(Nil, valDef.denot.info), + coord = annot.tree.span + ).enteredAfter(thisPhase).asTerm meth.addAnnotations(valDef.symbol.annotations) val body: Tree = ref(valDef.symbol) DefDef(meth, body) - def maybeGenerateSetter(valDef: ValDef)(using Context): Option[Tree] = + def maybeGenerateSetter(valDef: ValDef, annot: Annotation)(using Context): Option[Tree] = Option.when(valDef.denot.asSymDenotation.flags.is(Mutable)) { val owner = ctx.owner val meth = newSymbol( owner, name = prefixedName("set", valDef.name), flags = Method | Permanent | Synthetic, - info = MethodType(valDef.name :: Nil, valDef.denot.info :: Nil, defn.UnitType) + info = MethodType(valDef.name :: Nil, valDef.denot.info :: Nil, defn.UnitType), + coord = annot.tree.span ).enteredAfter(thisPhase).asTerm meth.addAnnotations(valDef.symbol.annotations) def body(params: List[List[Tree]]): Tree = Assign(ref(valDef.symbol), params.head.head) @@ -54,7 +58,9 @@ class BeanProperties(thisPhase: DenotTransformer): (prefix + valName.lastPart.toString.capitalize).toTermName val symbol = valDef.denot.symbol - if symbol.hasAnnotation(defn.BeanPropertyAnnot) || symbol.hasAnnotation(defn.BooleanBeanPropertyAnnot) then - generateGetter(valDef) +: maybeGenerateSetter(valDef) ++: Nil - else Nil + symbol.getAnnotation(defn.BeanPropertyAnnot) + .orElse(symbol.getAnnotation(defn.BooleanBeanPropertyAnnot)) + .toList.flatMap { annot => + generateGetter(valDef, annot) +: maybeGenerateSetter(valDef, annot) ++: Nil + } end generateAccessors From d3741a9b0d6ec7bdf30b1e24be26e82cdf51d3f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Wed, 3 Feb 2021 16:48:08 +0100 Subject: [PATCH 7/7] Apply suggestions from code review for bean properties accessors Co-authored-by: Guillaume Martres --- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 2 +- compiler/src/dotty/tools/dotc/transform/BeanProperties.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 9b6094bf88e8..3a5764596af3 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -590,7 +590,7 @@ class TreeUnpickler(reader: TastyReader, val isScala2MacroDefinedInScala3 = flags.is(Macro, butNot = Inline) && flags.is(Erased) ctx.owner match { case cls: ClassSymbol if (!isScala2MacroDefinedInScala3 || cls == defn.StringContextClass) && !isSyntheticBeanAccessor => - // Enter all members of classes that are not Scala 2 macros or synthetic accessors. + // Enter all members of classes that are not Scala 2 macros or synthetic bean accessors. // // For `StringContext`, enter `s`, `f` and `raw` // These definitions will be entered when defined in Scala 2. It is fine to enter them diff --git a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala index 1a003312cd8d..baeb9cdbea98 100644 --- a/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala +++ b/compiler/src/dotty/tools/dotc/transform/BeanProperties.scala @@ -45,7 +45,7 @@ class BeanProperties(thisPhase: DenotTransformer): val meth = newSymbol( owner, name = prefixedName("set", valDef.name), - flags = Method | Permanent | Synthetic, + flags = Method | Synthetic, info = MethodType(valDef.name :: Nil, valDef.denot.info :: Nil, defn.UnitType), coord = annot.tree.span ).enteredAfter(thisPhase).asTerm