From 74ab3d125708631c03863eef6818191ad14cecbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Thu, 7 Jan 2021 15:52:29 +0100 Subject: [PATCH 1/4] Fix #10788: Parsing arguments for java annotations * A port of fixes from https://github.com/scala/scala/pull/8781 with improvements. * Some of improvements come from https://github.com/scala/scala/pull/8982. * There are small changes to typer to allow for single elements of any type T be used where array of T is expected inside of java annotations arguments as it is correct in java and is used in 2 files in scala standard library. --- .../tools/dotc/parsing/JavaParsers.scala | 92 ++++++++++++++++--- .../src/dotty/tools/dotc/typer/Typer.scala | 28 +++++- tests/run/java-annot-params.check | 23 +++++ tests/run/java-annot-params/Annots_0.java | 70 ++++++++++++++ tests/run/java-annot-params/Test_1.scala | 5 + tests/run/java-annot-params/Use_0.java | 20 ++++ tests/run/java-annot-params/Use_1.java | 21 +++++ tests/run/java-annot-params/run_1.scala | 17 ++++ 8 files changed, 258 insertions(+), 18 deletions(-) create mode 100644 tests/run/java-annot-params.check create mode 100644 tests/run/java-annot-params/Annots_0.java create mode 100644 tests/run/java-annot-params/Test_1.scala create mode 100644 tests/run/java-annot-params/Use_0.java create mode 100644 tests/run/java-annot-params/Use_1.java create mode 100644 tests/run/java-annot-params/run_1.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala index e54ea0b2b6aa..928b68226ef6 100644 --- a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala @@ -246,7 +246,7 @@ object JavaParsers { def qualId(): RefTree = { var t: RefTree = atSpan(in.offset) { Ident(ident()) } - while (in.token == DOT) { + while (in.token == DOT && in.lookaheadToken == IDENTIFIER) { in.nextToken() t = atSpan(t.span.start, in.offset) { Select(t, ident()) } } @@ -346,20 +346,86 @@ object JavaParsers { /** Annotation ::= TypeName [`(` AnnotationArgument {`,` AnnotationArgument} `)`] */ def annotation(): Option[Tree] = { - val id = convertToTypeId(qualId()) - // only parse annotations without arguments - if (in.token == LPAREN && in.lookaheadToken != RPAREN) { - skipAhead() - accept(RPAREN) - None - } - else { - if (in.token == LPAREN) { + object LiteralT: + def unapply(token: Token) = Option(token match { + case TRUE => true + case FALSE => false + case CHARLIT => in.name(0) + case INTLIT => in.intVal(false).toInt + case LONGLIT => in.intVal(false) + case FLOATLIT => in.floatVal(false).toFloat + case DOUBLELIT => in.floatVal(false) + case STRINGLIT => in.name.toString + case _ => null + }).map(Constant(_)) + + def classOrId(): Tree = + val id = qualId() + if in.lookaheadToken == CLASS then in.nextToken() - accept(RPAREN) + accept(CLASS) + TypeApply( + Select( + scalaDot(nme.Predef), + nme.classOf), + convertToTypeId(id) :: Nil + ) + else id + + def array(): Tree = + accept(LBRACE) + val buffer = ListBuffer[Tree]() + while (in.token != RBRACE) { + buffer += argValue() + if (in.token == COMMA) in.nextToken() // using this instead of repsep allows us to handle trailing commas } - Some(ensureApplied(Select(New(id), nme.CONSTRUCTOR))) - } + val ok = !buffer.contains(EmptyTree) + in.token match { + case RBRACE if ok => + accept(RBRACE) + Apply(scalaDot(nme.Array), buffer.toList) + case _ => + skipTo(RBRACE) + EmptyTree + } + + def argValue(): Tree = + in.token match { + case LiteralT(c) => + val tree = atSpan(in.offset)(Literal(c)) + in.nextToken() + tree + case AT => + in.nextToken() + annotation().get + case IDENTIFIER => classOrId() + case LBRACE => array() + case _ => EmptyTree + } + + def annArg(): Tree = + if (in.token == IDENTIFIER && in.lookaheadToken == EQUALS) + val name = ident() + accept(EQUALS) + val argv = argValue() + NamedArg(name, argv) + + else + NamedArg(nme.value, argValue()) + + + val id = convertToTypeId(qualId()) + val args = if in.token == LPAREN then + in.nextToken() + val args = repsep(annArg, COMMA) + accept(RPAREN) + args + else Nil + + Some(Apply( + Select(New(id), nme.CONSTRUCTOR), + args + )) } def modifiers(inInterface: Boolean): Modifiers = { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index b9eff97f1a5c..3d3aaadea5d1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -81,6 +81,15 @@ object Typer { */ private val DroppedEmptyArgs = new Property.Key[Unit] + + /** Marker context property that indicates that typer is resolving types for arguments of + * an annotation defined in Java. This means that value of any type T can appear in positions where + * Array[T] is expected. + * For example, both `@Annot(5)` and `@Annot({5, 6}) are viable calls of the constructor + * of annotation defined as `@interface Annot { int[] value() }` + */ + private[typer] val JavaAnnotationArg = Property.Key[Unit]() + /** An attachment that indicates a failed conversion or extension method * search was tried on a tree. This will in some cases be reported in error messages */ @@ -855,7 +864,14 @@ class Typer extends Namer } def typedNamedArg(tree: untpd.NamedArg, pt: Type)(using Context): NamedArg = { - val arg1 = typed(tree.arg, pt) + val arg1 = if (ctx.property(JavaAnnotationArg).isDefined) { + pt match { + case AppliedType(a, typ :: Nil) if (a.isRef(defn.ArrayClass)) => + tryAlternatively { typed(tree.arg, pt) } { typed(untpd.JavaSeqLiteral(tree.arg :: Nil, TypeTree(typ)), pt) } + case _ => typed(tree.arg, pt) + } + } else typed(tree.arg, pt) + assignType(cpy.NamedArg(tree)(tree.name, arg1), arg1) } @@ -1977,11 +1993,13 @@ class Typer extends Namer */ def annotContext(mdef: untpd.Tree, sym: Symbol)(using Context): Context = { def isInner(owner: Symbol) = owner == sym || sym.is(Param) && owner == sym.owner - val c = ctx.outersIterator.dropWhile(c => isInner(c.owner)).next() - c.property(ExprOwner) match { - case Some(exprOwner) if c.owner.isClass => c.exprContext(mdef, exprOwner) - case _ => c + val outer = ctx.outersIterator.dropWhile(c => isInner(c.owner)).next() + val c = outer.property(ExprOwner) match { + case Some(exprOwner) if outer.owner.isClass => outer.exprContext(mdef, exprOwner) + case _ => outer } + if (c.isJava) c.fresh.setProperty(JavaAnnotationArg, ()) + else c } def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(using Context): Unit = { diff --git a/tests/run/java-annot-params.check b/tests/run/java-annot-params.check new file mode 100644 index 000000000000..273067c04996 --- /dev/null +++ b/tests/run/java-annot-params.check @@ -0,0 +1,23 @@ +class annots.A +class annots.A +SOME STRING +VALUE OF CONST +false +13.7 +VALUE +List(a, b, c) +List() +List(SINGLE) +List(ABC) + +class annots.A +class annots.A +SOME STRING +VALUE OF CONST +false +13.7 +VALUE +List(a, b, c) +List() +List(SINGLE) +List(ABC) diff --git a/tests/run/java-annot-params/Annots_0.java b/tests/run/java-annot-params/Annots_0.java new file mode 100644 index 000000000000..58ca781e6e29 --- /dev/null +++ b/tests/run/java-annot-params/Annots_0.java @@ -0,0 +1,70 @@ +package annots; + +import java.lang.annotation.*; + +@Retention(RetentionPolicy.RUNTIME) +@interface WithClass { + Class arg(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithClassDefaultName { + Class value(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithString { + String arg(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithReference { + String arg(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithBoolean { + boolean arg(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithFloat { + float arg(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithNested { + Nested arg(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface Nested { + String value(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithArray { + String[] value(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithEmptyArray { + String[] value(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithSingleElement { + String[] value(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface WithMultipleArgs { + int[] ints(); + float floatVal(); + Nested[] annots(); + Class clazz(); + String[] value(); +} +class A { + static final String CONST = "VALUE OF CONST"; +} diff --git a/tests/run/java-annot-params/Test_1.scala b/tests/run/java-annot-params/Test_1.scala new file mode 100644 index 000000000000..00ee08b3c623 --- /dev/null +++ b/tests/run/java-annot-params/Test_1.scala @@ -0,0 +1,5 @@ +object Test: + def main(args: Array[String]): Unit = + annots.runTest(classOf[annots.Use_0]) + println() + annots.runTest(classOf[annots.Use_1]) \ No newline at end of file diff --git a/tests/run/java-annot-params/Use_0.java b/tests/run/java-annot-params/Use_0.java new file mode 100644 index 000000000000..ff55cbd8cd12 --- /dev/null +++ b/tests/run/java-annot-params/Use_0.java @@ -0,0 +1,20 @@ +package annots; + +@WithClass(arg = A.class) +@WithClassDefaultName(A.class) +@WithString(arg = "SOME STRING") +@WithReference(arg = A.CONST) +@WithBoolean(arg = false) +@WithFloat(arg = 13.7f) +@WithNested(arg = @Nested("VALUE")) +@WithArray({ "a", "b", "c" }) +@WithEmptyArray({}) +@WithSingleElement("SINGLE") +@WithMultipleArgs( + ints = {1, 2, 3, }, + annots = { @Nested("Value"), @Nested(A.CONST) }, + floatVal = 13.7f, + value = "ABC", + clazz = A.class +) +public class Use_0 {} \ No newline at end of file diff --git a/tests/run/java-annot-params/Use_1.java b/tests/run/java-annot-params/Use_1.java new file mode 100644 index 000000000000..cec0aa69ea3f --- /dev/null +++ b/tests/run/java-annot-params/Use_1.java @@ -0,0 +1,21 @@ +package annots; + +@WithClass(arg = A.class) +@WithClassDefaultName(A.class) +@WithString(arg = "SOME STRING") +@WithReference(arg = A.CONST) +@WithBoolean(arg = false) +@WithFloat(arg = 13.7f) +@WithNested(arg = @Nested("VALUE")) +@WithArray({"a", "b", "c"}) +@WithEmptyArray({}) +@WithSingleElement("SINGLE") +@WithMultipleArgs( + ints = { 1, 2, 3, }, + annots = { @Nested("Value"), + @Nested(A.CONST) }, + floatVal = 13.7f, + value = "ABC", + clazz = A.class +) +public class Use_1 {} \ No newline at end of file diff --git a/tests/run/java-annot-params/run_1.scala b/tests/run/java-annot-params/run_1.scala new file mode 100644 index 000000000000..0fe66f00a2e6 --- /dev/null +++ b/tests/run/java-annot-params/run_1.scala @@ -0,0 +1,17 @@ +package annots + +def runTest(cls: Class[_]): Unit = + val params = + Option(cls.getAnnotation(classOf[WithClass])).map(_.arg) :: + Option(cls.getAnnotation(classOf[WithClassDefaultName])).map(_.value) :: + Option(cls.getAnnotation(classOf[WithString])).map(_.arg) :: + Option(cls.getAnnotation(classOf[WithReference])).map(_.arg) :: + Option(cls.getAnnotation(classOf[WithBoolean])).map(_.arg) :: + Option(cls.getAnnotation(classOf[WithFloat])).map(_.arg) :: + Option(cls.getAnnotation(classOf[WithNested])).map(_.arg.value) :: + Option(cls.getAnnotation(classOf[WithArray])).map(_.value.toList) :: + Option(cls.getAnnotation(classOf[WithEmptyArray])).map(_.value.toList) :: + Option(cls.getAnnotation(classOf[WithSingleElement])).map(_.value.toList) :: + Option(cls.getAnnotation(classOf[WithMultipleArgs])).map(_.value.toList) :: + Nil + params.flatten.foreach(println) \ No newline at end of file From d3f46fc655db7e2078c3e0caf2181558b04d3141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Wed, 13 Jan 2021 22:05:22 +0100 Subject: [PATCH 2/4] Skip java annotations with complex args in parser --- .../tools/dotc/parsing/JavaParsers.scala | 87 +++++++++++-------- tests/run/java-annot-params/Annots_0.java | 12 +++ tests/run/java-annot-params/Use_0.java | 2 + 3 files changed, 66 insertions(+), 35 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala index 928b68226ef6..50f8f319c55e 100644 --- a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala @@ -343,7 +343,20 @@ object JavaParsers { annots.toList } - /** Annotation ::= TypeName [`(` AnnotationArgument {`,` AnnotationArgument} `)`] + /** Annotation ::= TypeName [`(` [AnnotationArgument {`,` AnnotationArgument}] `)`] + * AnnotationArgument ::= ElementValuePair | ELementValue + * ElementValuePair ::= Identifier `=` ElementValue + * ElementValue ::= ConstExpressionSubset + * | ElementValueArrayInitializer + * | Annotation + * ElementValueArrayInitializer ::= `{` [ElementValue {`,` ElementValue}] [`,`] `}` + * ConstExpressionSubset ::= Literal + * | QualifiedName + * | ClassLiteral + * + * We support only subset of const expressions expected in this context by java. + * If we encounter expression that we cannot parse, we do not raise parsing error, + * but instead we skip entire annotation silently. */ def annotation(): Option[Tree] = { object LiteralT: @@ -372,60 +385,64 @@ object JavaParsers { ) else id - def array(): Tree = + def array(): Option[Tree] = accept(LBRACE) - val buffer = ListBuffer[Tree]() - while (in.token != RBRACE) { + val buffer = ListBuffer[Option[Tree]]() + while in.token != RBRACE do buffer += argValue() - if (in.token == COMMA) in.nextToken() // using this instead of repsep allows us to handle trailing commas - } - val ok = !buffer.contains(EmptyTree) - in.token match { - case RBRACE if ok => - accept(RBRACE) - Apply(scalaDot(nme.Array), buffer.toList) - case _ => - skipTo(RBRACE) - EmptyTree + if in.token == COMMA then + in.nextToken() // using this instead of repsep allows us to handle trailing commas + accept(RBRACE) + Option.unless(buffer contains None) { + Apply(scalaDot(nme.Array), buffer.flatten.toList) } - def argValue(): Tree = - in.token match { + def argValue(): Option[Tree] = + val tree = in.token match { case LiteralT(c) => val tree = atSpan(in.offset)(Literal(c)) in.nextToken() - tree + Some(tree) case AT => in.nextToken() - annotation().get - case IDENTIFIER => classOrId() + annotation() + case IDENTIFIER => Some(classOrId()) case LBRACE => array() - case _ => EmptyTree + case _ => None } + if in.token == COMMA || in.token == RBRACE || in.token == RPAREN then + tree + else + skipTo(COMMA, RBRACE, RPAREN) + None - def annArg(): Tree = - if (in.token == IDENTIFIER && in.lookaheadToken == EQUALS) - val name = ident() + def annArg(): Option[Tree] = + val name = if (in.token == IDENTIFIER && in.lookaheadToken == EQUALS) + val n = ident() accept(EQUALS) - val argv = argValue() - NamedArg(name, argv) - + n else - NamedArg(nme.value, argValue()) + nme.value + argValue().map(NamedArg(name, _)) val id = convertToTypeId(qualId()) - val args = if in.token == LPAREN then + val args = ListBuffer[Option[Tree]]() + if in.token == LPAREN then in.nextToken() - val args = repsep(annArg, COMMA) + if in.token != RPAREN then + args += annArg() + while in.token == COMMA do + in.nextToken() + args += annArg() accept(RPAREN) - args - else Nil - Some(Apply( - Select(New(id), nme.CONSTRUCTOR), - args - )) + Option.unless(args contains None) { + Apply( + Select(New(id), nme.CONSTRUCTOR), + args.flatten.toList + ) + } } def modifiers(inInterface: Boolean): Modifiers = { diff --git a/tests/run/java-annot-params/Annots_0.java b/tests/run/java-annot-params/Annots_0.java index 58ca781e6e29..48b1e8a0dc08 100644 --- a/tests/run/java-annot-params/Annots_0.java +++ b/tests/run/java-annot-params/Annots_0.java @@ -65,6 +65,18 @@ Class clazz(); String[] value(); } + +@Retention(RetentionPolicy.RUNTIME) +@interface ShouldNotCrash { + String value(); +} + +@Retention(RetentionPolicy.RUNTIME) +@interface ShouldAlsoNotCrash { + String value(); + int[] ints(); +} + class A { static final String CONST = "VALUE OF CONST"; } diff --git a/tests/run/java-annot-params/Use_0.java b/tests/run/java-annot-params/Use_0.java index ff55cbd8cd12..5ddcb542c418 100644 --- a/tests/run/java-annot-params/Use_0.java +++ b/tests/run/java-annot-params/Use_0.java @@ -17,4 +17,6 @@ value = "ABC", clazz = A.class ) +@ShouldNotCrash(false ? "A" + A.CONST : "B") +@ShouldAlsoNotCrash(value = "C", ints = { 1, 2, 3, 5 - 1 }) public class Use_0 {} \ No newline at end of file From d1d582664c55a83a09b5cfa553a1e92f1c46e8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Wed, 3 Feb 2021 15:00:22 +0100 Subject: [PATCH 3/4] =?UTF-8?q?Remove=20JavaAnnotationArg=20context=20prop?= =?UTF-8?q?erty=C2=A7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/dotty/tools/dotc/typer/Typer.scala | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 3d3aaadea5d1..9e1ad0f9b939 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -81,15 +81,6 @@ object Typer { */ private val DroppedEmptyArgs = new Property.Key[Unit] - - /** Marker context property that indicates that typer is resolving types for arguments of - * an annotation defined in Java. This means that value of any type T can appear in positions where - * Array[T] is expected. - * For example, both `@Annot(5)` and `@Annot({5, 6}) are viable calls of the constructor - * of annotation defined as `@interface Annot { int[] value() }` - */ - private[typer] val JavaAnnotationArg = Property.Key[Unit]() - /** An attachment that indicates a failed conversion or extension method * search was tried on a tree. This will in some cases be reported in error messages */ @@ -863,14 +854,20 @@ class Typer extends Namer case _ => tree } + def typedNamedArg(tree: untpd.NamedArg, pt: Type)(using Context): NamedArg = { - val arg1 = if (ctx.property(JavaAnnotationArg).isDefined) { - pt match { - case AppliedType(a, typ :: Nil) if (a.isRef(defn.ArrayClass)) => - tryAlternatively { typed(tree.arg, pt) } { typed(untpd.JavaSeqLiteral(tree.arg :: Nil, TypeTree(typ)), pt) } - case _ => typed(tree.arg, pt) - } - } else typed(tree.arg, pt) + /* Special case for resolving types for arguments of an annotation defined in Java. + * It allows that value of any type T can appear in positions where Array[T] is expected. + * For example, both `@Annot(5)` and `@Annot({5, 6}) are viable calls of the constructor + * of annotation defined as `@interface Annot { int[] value() }` + * We assume that calling `typedNamedArg` in context of Java implies that we are dealing + * with annotation contructor, as named arguments are not allowed anywhere else in Java. + */ + val arg1 = pt match { + case AppliedType(a, typ :: Nil) if ctx.isJava && a.isRef(defn.ArrayClass) => + tryAlternatively { typed(tree.arg, pt) } { typed(untpd.JavaSeqLiteral(tree.arg :: Nil, TypeTree(typ)), pt) } + case _ => typed(tree.arg, pt) + } assignType(cpy.NamedArg(tree)(tree.name, arg1), arg1) } @@ -1994,12 +1991,10 @@ class Typer extends Namer def annotContext(mdef: untpd.Tree, sym: Symbol)(using Context): Context = { def isInner(owner: Symbol) = owner == sym || sym.is(Param) && owner == sym.owner val outer = ctx.outersIterator.dropWhile(c => isInner(c.owner)).next() - val c = outer.property(ExprOwner) match { + outer.property(ExprOwner) match { case Some(exprOwner) if outer.owner.isClass => outer.exprContext(mdef, exprOwner) case _ => outer } - if (c.isJava) c.fresh.setProperty(JavaAnnotationArg, ()) - else c } def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(using Context): Unit = { From e057a3e00b28b3f3f44dc6e21eeef7bd2d34a25a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Wed, 3 Feb 2021 16:42:41 +0100 Subject: [PATCH 4/4] Wrap typed tree in special case for java annotation args Co-authored-by: Fengyun Liu --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9e1ad0f9b939..944861628785 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -865,7 +865,10 @@ class Typer extends Namer */ val arg1 = pt match { case AppliedType(a, typ :: Nil) if ctx.isJava && a.isRef(defn.ArrayClass) => - tryAlternatively { typed(tree.arg, pt) } { typed(untpd.JavaSeqLiteral(tree.arg :: Nil, TypeTree(typ)), pt) } + tryAlternatively { typed(tree.arg, pt) } { + val elemTp = untpd.TypedSplice(TypeTree(typ)) + typed(untpd.JavaSeqLiteral(tree.arg :: Nil, elemTp), pt) + } case _ => typed(tree.arg, pt) }