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

IncompatibleClassChangeError when implementing Java annotation #14199

Closed
cchantep opened this issue Jan 2, 2022 · 3 comments
Closed

IncompatibleClassChangeError when implementing Java annotation #14199

cchantep opened this issue Jan 2, 2022 · 3 comments

Comments

@cchantep
Copy link
Contributor

cchantep commented Jan 2, 2022

See that the previous issue is back: scala/bug#8778

Compiler version

3.1.2-RC1-bin-20211222-c94b333-NIGHTLY

Minimized code

import javax.inject.Qualifier;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

@Qualifier
@Retention(RetentionPolicy.RUNTIME)
public @interface NamedDatabase {
    String value();
}
import java.io.Serializable

final class NamedDatabaseImpl(val value: String)
    extends NamedDatabase(value)
    with Serializable {

  assert(value != null)

  override def hashCode: Int = (127 * "value".hashCode()) ^ value.hashCode()

  override def equals(that: Any): Boolean = that match {
    case other: NamedDatabase =>
      other.value == this.value

    case _ =>
      false
  }

  override def toString: String =
    s"@${classOf[NamedDatabase].getName}(value=${value})"

  def annotationType = classOf[NamedDatabase]
}

Output

When try to use it with Guice, fails with a IncompatibleClassChangeError

[error]  java.lang.IncompatibleClassChangeError: play/modules/reactivemongo/NamedDatabaseImpl (ReactiveMongoModule.scala:48
[error] play.modules.reactivemongo.ReactiveMongoModule$.key(ReactiveMongoModule.scala:48)
[error] play.modules.reactivemongo.ReactiveMongoModule.apiBindings$$anonfun$1(ReactiveMongoModule.scala:36)
[error] play.modules.reactivemongo.ReactiveMongoModule$$Lambda$12039/1390106650.apply(Unknown Source)
[error] play.modules.reactivemongo.ReactiveMongoModule.apiBindings(ReactiveMongoModule.scala:41)
[error] play.modules.reactivemongo.ReactiveMongoModule.bindings(ReactiveMongoModule.scala:24)
[error] play.modules.reactivemongo.ReactiveMongoModule.bindings(ReactiveMongoModule.scala:17)
[error] play.api.inject.guice.GuiceableModuleConversions.guice(GuiceInjectorBuilder.scala:365)
[error] play.api.inject.guice.GuiceableModuleConversions.guice$(GuiceInjectorBuilder.scala:364)
[error] play.api.inject.guice.GuiceableModule$.guice(GuiceInjectorBuilder.scala:297)

Expectation

@smarter
Copy link
Member

smarter commented Jan 2, 2022

Same root cause as #12840 (comment), so no easy fix.

@cchantep
Copy link
Contributor Author

cchantep commented Jan 2, 2022

That's quite a regression considering the fix applied about that in 2.13, no easy workaround.

@smarter
Copy link
Member

smarter commented Jan 2, 2022

Yep, that's what happens when two compilers are developed in parallel.

smarter added a commit to dotty-staging/dotty that referenced this issue Oct 28, 2022
Previously we treated Java annotations as if they were classes, like Scala
annotations. For example, given

    @interface Ann { int foo(); }

we pretended it was defined as:

    abstract class Ann(foo: Int) extends java.lang.annotation.Annotation { def foo(): Int }

We take advantage of this to type annotation trees as if they were
new calls, for example `@Ann(1)` is typed as `new Ann(1)`.

Pretending that annotations are classes is fine most of the time and matches
what Scala 2.12 did, but it's problematic because the JVM treats annotations as
interfaces. In practice this was only an issue with code trying to extend
Java annotations, which would either be rejected at compile-time or miscompiled
before this commit.

This commit switches our representation of annotations to be trait-based
instead:

    trait Ann(foo: Int) extends java.lang.annotation.Annotation { def foo(): Int }

Classes are then free to extend annotations using the same pattern as in Scala 2.13:

    class Foo extends Ann {val annotationType = classOf[Retention]; def foo(): Int = 1}

Notice that we still pretend these traits have constructors, this lets us type
annotation trees in much the same way as before, and crucially it means that
macros that depended on the exact tree shape of annotation trees can continue to
work, as demonstrated by the annot-java-tree test extracted from wartremover.
To prevent miscompilation issues, we disallow passing arguments to the
annotation constructor in `extends` clause.

The treatment of default arguments to annotations stays unchanged from 85cd1cf.

Fixes scala#5690. Fixes scala#12840. Fixes scala#14199.
little-inferno pushed a commit to little-inferno/dotty that referenced this issue Jan 25, 2023
Previously we treated Java annotations as if they were classes, like Scala
annotations. For example, given

    @interface Ann { int foo(); }

we pretended it was defined as:

    abstract class Ann(foo: Int) extends java.lang.annotation.Annotation { def foo(): Int }

We take advantage of this to type annotation trees as if they were
new calls, for example `@Ann(1)` is typed as `new Ann(1)`.

Pretending that annotations are classes is fine most of the time and matches
what Scala 2.12 did, but it's problematic because the JVM treats annotations as
interfaces. In practice this was only an issue with code trying to extend
Java annotations, which would either be rejected at compile-time or miscompiled
before this commit.

This commit switches our representation of annotations to be trait-based
instead:

    trait Ann(foo: Int) extends java.lang.annotation.Annotation { def foo(): Int }

Classes are then free to extend annotations using the same pattern as in Scala 2.13:

    class Foo extends Ann {val annotationType = classOf[Retention]; def foo(): Int = 1}

Notice that we still pretend these traits have constructors, this lets us type
annotation trees in much the same way as before, and crucially it means that
macros that depended on the exact tree shape of annotation trees can continue to
work, as demonstrated by the annot-java-tree test extracted from wartremover.
To prevent miscompilation issues, we disallow passing arguments to the
annotation constructor in `extends` clause.

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_0` annotations as arguments, and in
previous releases of Scala 3 we could put `Plain_0(4)` in this array without
`new`. This is because the compiler generates a "constructor proxy" apply method
for classes, but this no longer works since `Plain_0` is now a trait. While we
could potentially tweak the constructor proxy logic to handle this case, it
seems simpler to require a `new` here, 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.

The treatment of default arguments to annotations stays unchanged from 85cd1cf.

Fixes scala#5690. Fixes scala#12840. Fixes scala#14199.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants