From df750191e3c718ec650dc2000a4f55c728224be1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Tue, 8 Dec 2020 14:09:08 +0100 Subject: [PATCH 1/3] Add support for repeatable annotations --- compiler/src/dotty/tools/dotc/Compiler.scala | 3 +- .../dotty/tools/dotc/core/Definitions.scala | 2 + .../transform/RepeatableAnnotations.scala | 48 +++++++++++++++++++ tests/neg/repeatable/FirstLevel_0.java | 9 ++++ tests/neg/repeatable/Plain_0.java | 9 ++++ tests/neg/repeatable/SecondLevel_0.java | 8 ++++ tests/neg/repeatable/Test_1.scala | 16 +++++++ tests/run/repeatable.check | 6 +++ tests/run/repeatable/FirstLevel_0.java | 9 ++++ tests/run/repeatable/Plain_0.java | 9 ++++ tests/run/repeatable/SecondLevel_0.java | 8 ++++ tests/run/repeatable/Test_1.scala | 20 ++++++++ 12 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala create mode 100644 tests/neg/repeatable/FirstLevel_0.java create mode 100644 tests/neg/repeatable/Plain_0.java create mode 100644 tests/neg/repeatable/SecondLevel_0.java create mode 100644 tests/neg/repeatable/Test_1.scala create mode 100644 tests/run/repeatable.check create mode 100644 tests/run/repeatable/FirstLevel_0.java create mode 100644 tests/run/repeatable/Plain_0.java create mode 100644 tests/run/repeatable/SecondLevel_0.java create mode 100644 tests/run/repeatable/Test_1.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index e54725f48ff4..4818fca07d0b 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -127,7 +127,8 @@ class Compiler { new RestoreScopes, // Repair scopes rendered invalid by moving definitions in prior phases of the group new SelectStatic, // get rid of selects that would be compiled into GetStatic new sjs.JUnitBootstrappers, // Generate JUnit-specific bootstrapper classes for Scala.js (not enabled by default) - new CollectSuperCalls) :: // Find classes that are called with super + new CollectSuperCalls, // Find classes that are called with super + new RepeatableAnnotations) :: // Aggregate repeatable annotations Nil /** Generate the output of the compilation */ diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 25af385ea153..517da5ece073 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -923,6 +923,8 @@ class Definitions { @tu lazy val TargetNameAnnot: ClassSymbol = requiredClass("scala.annotation.targetName") @tu lazy val VarargsAnnot: ClassSymbol = requiredClass("scala.annotation.varargs") + @tu lazy val JavaRepeatableAnnot: ClassSymbol = requiredClass("java.lang.annotation.Repeatable") + // A list of meta-annotations that are relevant for fields and accessors @tu lazy val FieldAccessorMetaAnnots: Set[Symbol] = Set(FieldMetaAnnot, GetterMetaAnnot, ParamMetaAnnot, SetterMetaAnnot) diff --git a/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala b/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala new file mode 100644 index 000000000000..17347dabbf77 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala @@ -0,0 +1,48 @@ +package dotty.tools.dotc +package transform + +import core._ +import ast.tpd._ +import Contexts._ +import MegaPhase._ +import Annotations._ +import Symbols.defn +import Constants._ +import Types._ +import Decorators._ + +class RepeatableAnnotations extends MiniPhase: + override def phaseName = "repeatableAnnotations" + + override def transformTypeDef(tree: TypeDef)(using Context): Tree = transformDef(tree) + override def transformValDef(tree: ValDef)(using Context): Tree = transformDef(tree) + override def transformDefDef(tree: DefDef)(using Context): Tree = transformDef(tree) + + private def transformDef(tree: DefTree)(using Context) = + val annotations = tree.symbol.annotations + if (!annotations.isEmpty) then + tree.symbol.annotations = aggregateAnnotations(tree.symbol.annotations) + tree + + private def aggregateAnnotations(annotations: Seq[Annotation])(using Context): List[Annotation] = + val annsByType = annotations.groupBy(_.symbol) + annsByType.flatMap { + case (_, a :: Nil) => Some(a) + case (sym, anns) => + sym.annotations.find(_ matches defn.JavaRepeatableAnnot).flatMap(_.argumentConstant(0)) match + case Some(Constant(containerTpe: Type)) => + val clashingAnns = annsByType.getOrElse(containerTpe.classSymbol, Nil) + if !clashingAnns.isEmpty then + // this is the same error javac would raise in this case + val pos = clashingAnns.map(_.tree.srcPos).minBy(_.line) + report.error("Container must not be present at the same time as the element it contains", pos) + None + else + val aggregated = JavaSeqLiteral(anns.map(_.tree).toList, TypeTree(sym.typeRef)) + Some(Annotation(containerTpe, NamedArg("value".toTermName, aggregated))) + case _ => + val pos = anns.map(_.tree.srcPos).sortBy(_.line).apply(1) + report.error("Not repeatable annotation repeated", pos) + None + }.toList + diff --git a/tests/neg/repeatable/FirstLevel_0.java b/tests/neg/repeatable/FirstLevel_0.java new file mode 100644 index 000000000000..5932d32ed58d --- /dev/null +++ b/tests/neg/repeatable/FirstLevel_0.java @@ -0,0 +1,9 @@ +package repeatable; + +import java.lang.annotation.*; + +@Repeatable(SecondLevel_0.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface FirstLevel_0 { + Plain_0[] value(); +} \ No newline at end of file diff --git a/tests/neg/repeatable/Plain_0.java b/tests/neg/repeatable/Plain_0.java new file mode 100644 index 000000000000..f79048f1c52a --- /dev/null +++ b/tests/neg/repeatable/Plain_0.java @@ -0,0 +1,9 @@ +package repeatable; + +import java.lang.annotation.*; + +@Repeatable(FirstLevel_0.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface Plain_0 { + public int value(); +} \ No newline at end of file diff --git a/tests/neg/repeatable/SecondLevel_0.java b/tests/neg/repeatable/SecondLevel_0.java new file mode 100644 index 000000000000..6073302c4e4b --- /dev/null +++ b/tests/neg/repeatable/SecondLevel_0.java @@ -0,0 +1,8 @@ +package repeatable; + +import java.lang.annotation.*; + +@Retention(RetentionPolicy.RUNTIME) +public @interface SecondLevel_0 { + FirstLevel_0[] value(); +} diff --git a/tests/neg/repeatable/Test_1.scala b/tests/neg/repeatable/Test_1.scala new file mode 100644 index 000000000000..3779b6ffa4a8 --- /dev/null +++ b/tests/neg/repeatable/Test_1.scala @@ -0,0 +1,16 @@ +import repeatable._ + +@Plain_0(1) +@Plain_0(2) +@Plain_0(3) +@FirstLevel_0(Array()) // error +trait U + +@FirstLevel_0(Array(Plain_0(4), Plain_0(5))) +@FirstLevel_0(Array(Plain_0(6), Plain_0(7))) +@SecondLevel_0(Array()) // error +trait T + +@SecondLevel_0(Array()) +@SecondLevel_0(Array()) // error +trait S \ No newline at end of file diff --git a/tests/run/repeatable.check b/tests/run/repeatable.check new file mode 100644 index 000000000000..cdef0e44bea4 --- /dev/null +++ b/tests/run/repeatable.check @@ -0,0 +1,6 @@ +1 +2 +3 + +List(4, 5) +List(6, 7) diff --git a/tests/run/repeatable/FirstLevel_0.java b/tests/run/repeatable/FirstLevel_0.java new file mode 100644 index 000000000000..5932d32ed58d --- /dev/null +++ b/tests/run/repeatable/FirstLevel_0.java @@ -0,0 +1,9 @@ +package repeatable; + +import java.lang.annotation.*; + +@Repeatable(SecondLevel_0.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface FirstLevel_0 { + Plain_0[] value(); +} \ No newline at end of file diff --git a/tests/run/repeatable/Plain_0.java b/tests/run/repeatable/Plain_0.java new file mode 100644 index 000000000000..f79048f1c52a --- /dev/null +++ b/tests/run/repeatable/Plain_0.java @@ -0,0 +1,9 @@ +package repeatable; + +import java.lang.annotation.*; + +@Repeatable(FirstLevel_0.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface Plain_0 { + public int value(); +} \ No newline at end of file diff --git a/tests/run/repeatable/SecondLevel_0.java b/tests/run/repeatable/SecondLevel_0.java new file mode 100644 index 000000000000..6073302c4e4b --- /dev/null +++ b/tests/run/repeatable/SecondLevel_0.java @@ -0,0 +1,8 @@ +package repeatable; + +import java.lang.annotation.*; + +@Retention(RetentionPolicy.RUNTIME) +public @interface SecondLevel_0 { + FirstLevel_0[] value(); +} diff --git a/tests/run/repeatable/Test_1.scala b/tests/run/repeatable/Test_1.scala new file mode 100644 index 000000000000..2a7fccc7b4fa --- /dev/null +++ b/tests/run/repeatable/Test_1.scala @@ -0,0 +1,20 @@ +import repeatable._ + +@Plain_0(1) +@Plain_0(2) +@Plain_0(3) +trait U + +@FirstLevel_0(Array(Plain_0(4), Plain_0(5))) +@FirstLevel_0(Array(Plain_0(6), Plain_0(7))) +trait T + +object Test: + def main(args: Array[String]) = + val annValuesU = classOf[U].getAnnotation(classOf[FirstLevel_0]).value.toList.map(_.value).sorted + annValuesU.foreach(println) + + println() + + val annValuesT = classOf[T].getAnnotation(classOf[SecondLevel_0]).value.toList.map(_.value.toList.map(_.value).sorted).sorted + annValuesT.foreach(println) From 3901cb01b9222dc69e0a0eae0e9dc127f356c7e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Fri, 11 Dec 2020 10:56:25 +0100 Subject: [PATCH 2/3] Fix overzealous filtering of annotations --- .../tools/dotc/transform/RepeatableAnnotations.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala b/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala index 17347dabbf77..5cbfdcb1b9a6 100644 --- a/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala +++ b/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala @@ -27,8 +27,8 @@ class RepeatableAnnotations extends MiniPhase: private def aggregateAnnotations(annotations: Seq[Annotation])(using Context): List[Annotation] = val annsByType = annotations.groupBy(_.symbol) annsByType.flatMap { - case (_, a :: Nil) => Some(a) - case (sym, anns) => + case (_, a :: Nil) => a :: Nil + case (sym, anns) if sym.derivesFrom(defn.ClassfileAnnotationClass) => sym.annotations.find(_ matches defn.JavaRepeatableAnnot).flatMap(_.argumentConstant(0)) match case Some(Constant(containerTpe: Type)) => val clashingAnns = annsByType.getOrElse(containerTpe.classSymbol, Nil) @@ -36,13 +36,14 @@ class RepeatableAnnotations extends MiniPhase: // this is the same error javac would raise in this case val pos = clashingAnns.map(_.tree.srcPos).minBy(_.line) report.error("Container must not be present at the same time as the element it contains", pos) - None + Nil else val aggregated = JavaSeqLiteral(anns.map(_.tree).toList, TypeTree(sym.typeRef)) - Some(Annotation(containerTpe, NamedArg("value".toTermName, aggregated))) + Annotation(containerTpe, NamedArg("value".toTermName, aggregated)) :: Nil case _ => val pos = anns.map(_.tree.srcPos).sortBy(_.line).apply(1) report.error("Not repeatable annotation repeated", pos) - None + Nil + case (_, anns) => anns }.toList From 7406c30443aaf671dd1d1576558f5aa3be7cb20e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Tue, 16 Mar 2021 12:04:42 +0100 Subject: [PATCH 3/3] Apply suggestions from code review - now only the first clashing annotation is reported Co-authored-by: Fengyun Liu --- .../tools/dotc/transform/RepeatableAnnotations.scala | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala b/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala index 5cbfdcb1b9a6..ecf2d3e2b96f 100644 --- a/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala +++ b/compiler/src/dotty/tools/dotc/transform/RepeatableAnnotations.scala @@ -29,21 +29,20 @@ class RepeatableAnnotations extends MiniPhase: annsByType.flatMap { case (_, a :: Nil) => a :: Nil case (sym, anns) if sym.derivesFrom(defn.ClassfileAnnotationClass) => - sym.annotations.find(_ matches defn.JavaRepeatableAnnot).flatMap(_.argumentConstant(0)) match + sym.getAnnotation(defn.JavaRepeatableAnnot).flatMap(_.argumentConstant(0)) match case Some(Constant(containerTpe: Type)) => val clashingAnns = annsByType.getOrElse(containerTpe.classSymbol, Nil) - if !clashingAnns.isEmpty then + if clashingAnns.nonEmpty then // this is the same error javac would raise in this case - val pos = clashingAnns.map(_.tree.srcPos).minBy(_.line) + val pos = clashingAnns.head.tree.srcPos report.error("Container must not be present at the same time as the element it contains", pos) Nil else val aggregated = JavaSeqLiteral(anns.map(_.tree).toList, TypeTree(sym.typeRef)) Annotation(containerTpe, NamedArg("value".toTermName, aggregated)) :: Nil case _ => - val pos = anns.map(_.tree.srcPos).sortBy(_.line).apply(1) + val pos = anns.head.tree.srcPos report.error("Not repeatable annotation repeated", pos) Nil case (_, anns) => anns }.toList -