scala3
scala3 copied to clipboard
Failed compile when Java annotation used as parameter of another Java annotation
Compiler version
Scala 3.3 and 3.4
Minimized code
//> using scala 3.4.0
//> using dep "jakarta.persistence:jakarta.persistence-api:3.1.0"
import jakarta.persistence.*
@Table(indexes=Array(Index(columnList="email")))
class User
Output
$ scala-cli run test.scala
Compiling project (Scala 3.4.0, JVM (20))
[error] ./test.scala:6:22
[error] object Index in package jakarta.persistence does not take parameters
[error] @Table(indexes=Array(Index(columnList="email")))
[error] ^^^^^
Error compiling project (Scala 3.4.0, JVM (20))
Compilation failed
Expectation
The code compiles with Scala <= 3.2, but does not compile with higher version of Scala. Expected to be working with Scala 3.4
Ok, so this is weird... So far I didn't manage to minimise it, despite the code from jakarta.persistance being relatively simple, so it may be something specific to the jakarta build.
We'd perhaps need a self-contained minimisation to tell more. (without the jakarta dependency).
The example indeed compiles on Scala 3.2.0, but doesn't on LTS nor on 3.4.0.
However, I wonder if it's a valid bug in the first place. The following code compiles on 3.4.0:
//> using scala 3.4.0
//> using dep "jakarta.persistence:jakarta.persistence-api:3.1.0"
import jakarta.persistence.Index
import jakarta.persistence.Table
@Table(indexes=Array(new Index(columnList="email")))
class User
So the new keyword seems to be necessary to call the constructor here.
Should it be necessary, however?
@sjrd Would you consider this a valid regression?
Should the new keyword be required when passing an anotation parameter like this?
You're not supposed to need a new in Scala 3, whether you're in annotations or not. So IMO this is a real regression.
Yes. Adding a new keyword fixed my problem. Thanks
using local java sources, 3.2.2 also fails to compile.
package trial;
import java.lang.annotation.*;
@Target({})
@Retention(RetentionPolicy.RUNTIME)
public @interface Index {
String name() default "";
String columnList();
}
package trial;
import java.lang.annotation.*;
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface Table {
String name() default "";
Index[] indexes() default {};
}
using
ThisBuild / compileOrder := CompileOrder.JavaThenScala
makes it compile, which is obviously a bug or limitation.
Nothing works on 3.3.3 or 3.4 except new.
Outside of annotation context, 3.2.2 crashes:
java.util.NoSuchElementException: class String while running genBCode
but 3.3 emits normal error.
That edit of Scala 2 spec on annotations isn't ported yet, but then perhaps "Host-platform Annotations" could be updated. Currently it says it should just work, unless it doesn't.
additional restrictions on the expressions which are valid as annotation arguments
probably doesn't intend to say that you have to use new. Java requires @.
Not sure how incremental compilation faked out javac here:
[success] Total time: 0 s, completed Mar 18, 2024, 9:28:07 PM
[info] compiling 1 Scala source and 1 Java source to .../i19959/target/scala-3.3.3/classes ...
[error] .../i19959/src/main/java/trial/Used.java:5:1: cannot find symbol
[error] symbol: class Table
[error] Table
[error] .../i19959/src/main/java/trial/Used.java:5:1: cannot find symbol
[error] symbol: class Index
[error] Index
[error] (Compile / compileIncremental) javac returned non-zero exit code
[error] Total time: 0 s, completed Mar 18, 2024, 9:28:07 PM
You're not supposed to need a new in Scala 3, whether you're in annotations or not. So IMO this is a real regression.
This was actually an intended change, quoting from https://github.com/scala/scala3/pull/16260/commits/3125665da2e9a15d88b1a6006d60e6b14e1bd34a :
This change is not fully backwards source compatible: this is illustrated by the diffs in tests/run/repeatable/Test_1.scala:
-@FirstLevel_0(Array(Plain_0(4), Plain_0(5))) +@FirstLevel_0(Array(new Plain_0(4), new Plain_0(5)))
Here, FirstLevel_0 takes an array of
Plain_0annotations as arguments, and in previous releases of Scala 3 we could putPlain_0(4)in this array withoutnew. This is because the compiler generates a "constructor proxy" apply method for classes, but this no longer works sincePlain_0is now a trait. While we could potentially tweak the constructor proxy logic to handle this case, it seems simpler to require anewhere, both because Scala 2 does it too and because it ensures that user code that inspects the annotation tree does not have to deal with constructor proxies.
Given that the current encoding of constructors with annotations is already causing us headaches (https://github.com/scala/scala3/issues/19951#issuecomment-2083782502), it's probably wiser to not make things more complicated by adding constructor proxies in the mix, at least not until #19951 has been fixed.
Okay, so this is blocked by #19951.