Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add migration rewrite for non-named arguments in Java annotations #21397

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading