Skip to content

Commit

Permalink
Add migration rewrite for non-named arguments in Java annotations (#2…
Browse files Browse the repository at this point in the history
…1397)

Followup to #21329
It might be beneficial for Scala 3.6 migration to allow for an automatic
rewrite of unnamed Java annotation parameters to ease the migration.
We can assume that already compiling code is already using correct
positions for annotation arguments.
Based on that, we can use the order of the annotation decls and their
indexes to assume the names of annotation arguments
  • Loading branch information
hamzaremmal authored Aug 20, 2024
2 parents b64afad + 90eedc5 commit 30317cd
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 2 deletions.
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/config/MigrationVersion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ object MigrationVersion:
val WithOperator = MigrationVersion(`3.4`, future)
val FunctionUnderscore = MigrationVersion(`3.4`, future)

val NonNamedArgumentInJavaAnnotation = MigrationVersion(`3.6`, `3.6`)

val ImportWildcard = MigrationVersion(future, future)
val ImportRename = MigrationVersion(future, future)
val ParameterEnclosedByParenthesis = MigrationVersion(future, future)
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.util.SourcePosition
import scala.jdk.CollectionConverters.*
import dotty.tools.dotc.util.SourceFile
import dotty.tools.dotc.config.SourceVersion
import DidYouMean.*

/** Messages
Expand Down Expand Up @@ -3293,6 +3294,7 @@ class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamed

override protected def msg(using Context): String =
"Named arguments are required for Java defined annotations"
+ Message.rewriteNotice("This", version = SourceVersion.`3.6-migration`)

override protected def explain(using Context): String =
i"""Starting from Scala 3.6.0, named arguments are required for Java defined annotations.
Expand Down
16 changes: 14 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -891,14 +891,26 @@ object Checking {
def annotationHasValueField: Boolean =
sym.info.decls.exists(_.name == nme.value)

lazy val annotationFieldNamesByIdx: Map[Int, TermName] =
sym.info.decls.filter: decl =>
decl.is(Method) && decl.name != nme.CONSTRUCTOR
.map(_.name.toTermName)
.zipWithIndex
.map(_.swap)
.toMap

annot match
case untpd.Apply(fun, List(param)) if !param.isInstanceOf[untpd.NamedArg] && annotationHasValueField =>
untpd.cpy.Apply(annot)(fun, List(untpd.cpy.NamedArg(param)(nme.value, param)))
case untpd.Apply(_, params) =>
for
param <- params
(param, paramIdx) <- params.zipWithIndex
if !param.isInstanceOf[untpd.NamedArg]
do report.error(NonNamedArgumentInJavaAnnotation(), param)
do
report.errorOrMigrationWarning(NonNamedArgumentInJavaAnnotation(), param, MigrationVersion.NonNamedArgumentInJavaAnnotation)
if MigrationVersion.NonNamedArgumentInJavaAnnotation.needsPatch then
annotationFieldNamesByIdx.get(paramIdx).foreach: paramName =>
patch(param.span.startPos, s"$paramName = ")
annot
case _ => annot
end checkNamedArgumentForJavaAnnotation
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class CompilationTests {
compileFile("tests/rewrites/i17187.scala", unindentOptions.and("-rewrite")),
compileFile("tests/rewrites/i17399.scala", unindentOptions.and("-rewrite")),
compileFile("tests/rewrites/i20002.scala", defaultOptions.and("-indent", "-rewrite")),
compileDir("tests/rewrites/annotation-named-pararamters", defaultOptions.and("-rewrite", "-source:3.6-migration")),
).checkRewrites()
}

Expand Down
2 changes: 2 additions & 0 deletions tests/neg/i20554-a.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
3 |@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
| ^
| Named arguments are required for Java defined annotations
| This can be rewritten automatically under -rewrite -source 3.6-migration.
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Expand All @@ -23,6 +24,7 @@
3 |@Annotation(3, 4) // error // error : Java defined annotation should be called with named arguments
| ^
| Named arguments are required for Java defined annotations
| This can be rewritten automatically under -rewrite -source 3.6-migration.
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Expand Down
1 change: 1 addition & 0 deletions tests/neg/i20554-b.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
3 |@SimpleAnnotation(1) // error: the parameters is not named 'value'
| ^
| Named arguments are required for Java defined annotations
| This can be rewritten automatically under -rewrite -source 3.6-migration.
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Expand Down
8 changes: 8 additions & 0 deletions tests/rewrites/annotation-named-pararamters/MyAnnotation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import java.util.concurrent.TimeUnit;

public @interface MyAnnotation {
public TimeUnit D() default TimeUnit.DAYS;
TimeUnit C() default TimeUnit.DAYS;
String A() default "";
public String B() default "";
}
6 changes: 6 additions & 0 deletions tests/rewrites/annotation-named-pararamters/test.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import java.util.concurrent.TimeUnit
@MyAnnotation() class Test1
@MyAnnotation(D = TimeUnit.DAYS) class Test2
@MyAnnotation(D = TimeUnit.DAYS, C = TimeUnit.DAYS) class Test3
@MyAnnotation(D = TimeUnit.DAYS, C = TimeUnit.DAYS, A = "foo") class Test4
@MyAnnotation(D = TimeUnit.DAYS, C = TimeUnit.DAYS, A = "foo", B = "bar") class Test5
6 changes: 6 additions & 0 deletions tests/rewrites/annotation-named-pararamters/test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import java.util.concurrent.TimeUnit
@MyAnnotation() class Test1
@MyAnnotation(TimeUnit.DAYS) class Test2
@MyAnnotation(TimeUnit.DAYS, TimeUnit.DAYS) class Test3
@MyAnnotation(TimeUnit.DAYS, TimeUnit.DAYS, "foo") class Test4
@MyAnnotation(TimeUnit.DAYS, TimeUnit.DAYS, "foo", "bar") class Test5

0 comments on commit 30317cd

Please sign in to comment.