From 03fc304e5f177ac79b740190702f930fea49623e Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Mon, 19 Aug 2024 20:41:52 +0200 Subject: [PATCH 1/2] Add migration rewrite for non-named arguments in Java annotations --- .../tools/dotc/config/MigrationVersion.scala | 2 ++ .../dotty/tools/dotc/reporting/messages.scala | 2 ++ .../src/dotty/tools/dotc/typer/Checking.scala | 16 ++++++++++++++-- .../test/dotty/tools/dotc/CompilationTests.scala | 1 + tests/neg/i20554-a.check | 2 ++ tests/neg/i20554-b.check | 1 + .../MyAnnotation.java | 8 ++++++++ .../annotation-named-pararamters/test.check | 6 ++++++ .../annotation-named-pararamters/test.scala | 6 ++++++ 9 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/rewrites/annotation-named-pararamters/MyAnnotation.java create mode 100644 tests/rewrites/annotation-named-pararamters/test.check create mode 100644 tests/rewrites/annotation-named-pararamters/test.scala diff --git a/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala b/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala index 4dd9d065395b..4a16111e76a5 100644 --- a/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala +++ b/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala @@ -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) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 38b49e63c685..91642ca51bc5 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -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 @@ -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. diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index aeda38cc7646..efcdad2b427f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -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, untpd.cpy.NamedArg(param)(paramName, param).show) annot case _ => annot end checkNamedArgumentForJavaAnnotation diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 2b9ebd2c69d1..dd722403723a 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -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() } diff --git a/tests/neg/i20554-a.check b/tests/neg/i20554-a.check index 5cfa4e3faaad..b223cba32f77 100644 --- a/tests/neg/i20554-a.check +++ b/tests/neg/i20554-a.check @@ -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`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -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`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/neg/i20554-b.check b/tests/neg/i20554-b.check index 2395554a7485..5e5119e043fe 100644 --- a/tests/neg/i20554-b.check +++ b/tests/neg/i20554-b.check @@ -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`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/rewrites/annotation-named-pararamters/MyAnnotation.java b/tests/rewrites/annotation-named-pararamters/MyAnnotation.java new file mode 100644 index 000000000000..a7e247c8083c --- /dev/null +++ b/tests/rewrites/annotation-named-pararamters/MyAnnotation.java @@ -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 ""; +} \ No newline at end of file diff --git a/tests/rewrites/annotation-named-pararamters/test.check b/tests/rewrites/annotation-named-pararamters/test.check new file mode 100644 index 000000000000..186a4fbf7974 --- /dev/null +++ b/tests/rewrites/annotation-named-pararamters/test.check @@ -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 diff --git a/tests/rewrites/annotation-named-pararamters/test.scala b/tests/rewrites/annotation-named-pararamters/test.scala new file mode 100644 index 000000000000..85cf34ab976b --- /dev/null +++ b/tests/rewrites/annotation-named-pararamters/test.scala @@ -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 \ No newline at end of file From 90eedc5dbb0059839a61ba1208d07ec90f84d608 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 20 Aug 2024 12:54:11 +0200 Subject: [PATCH 2/2] Replace usage of printer to create patch with a simple string prefix Co-authored-by: Hamza Remmal --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index efcdad2b427f..7f5ac955fa12 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -910,7 +910,7 @@ object Checking { report.errorOrMigrationWarning(NonNamedArgumentInJavaAnnotation(), param, MigrationVersion.NonNamedArgumentInJavaAnnotation) if MigrationVersion.NonNamedArgumentInJavaAnnotation.needsPatch then annotationFieldNamesByIdx.get(paramIdx).foreach: paramName => - patch(param.span, untpd.cpy.NamedArg(param)(paramName, param).show) + patch(param.span.startPos, s"$paramName = ") annot case _ => annot end checkNamedArgumentForJavaAnnotation