Imported From: https://issues.scala-lang.org/browse/SI-8926?orig=1
Reporter: Antti Lampinen (arlampin)
Affected Versions: 2.11.3

@soc said:
Aaaargh, Looks like the existing separate compilation tests were not sufficient. :-/

@lrytz said:
@soc I just started investigating the issue, I'll let you know if I have questions, OK? No need to duplicate the work.

@soc said:
Roger, Lukas, let me know when you have questions!

@adriaanm said:
Hi Antti, thanks for reporting!

Would compiling the java sources with javac and not with scalac be a feasible workaround for you?

@adriaanm said:
Looks related to scala/scala#4026

@lrytz said (edited on Oct 20, 2014 9:22:58 AM UTC):
Regressed in scala/scala#4026.

@soc: this is totally not your fault: right now the Java source parser skips over all annotations, def annotations doesn't return anything.. Therefore the ClassSymbol for SomeAnnotation doesn't have a @Retention annotation in case the class is read from source (instead of the classfile when the annotation class was previously compiled with javac).

Extending the Java parser is out of scope for 2.11.4, see here what it takes.

My proposal: How about we change the default visibility (in case there is no @Retention annotation on the annotation class' ClassSymbol) to RUNTIME? This means that we emit a too large visibility in some cases, like we did in the past, which would be safe. What happens currently (too small visibility in some cases) is not safe, because some expected annotations are missing. For 2.11.5 we should fix the java parser.

The other option is to revert the change and put the complete change in 2.11.5. What do you think?

@soc said:
Mhhh, I think changing the default visibility could work, as it could be an incremental improvement. At least it is better than before, even if not perfect.

I have a hard time coming up with examples which "worked" before, but would be broken after this change, but the conservative choice of revert could be an option if we want to get 2.11.4 out of the door with the most minimal risk.

@soc said:
Regarding the necessary fix: I guess the hardest part is to figure out how to make sure that things like Retention, RetentionPolicy and its members actually refer to java.lang.annotation and not some random other type. (Very unlikely, but easy to provide with an obvious example if matching is implemented "by name only".)

@lrytz said:
The following shows the problem:

lucmac:sandbox luc$ rm -f *.class && scalac-hash v2.11.2 Test.scala SomeAnnotation.java && javac SomeAnnotation.java && scala-hash v2.11.2 Test
Annotation is present

lucmac:sandbox luc$ rm -f *.class && scalac-hash v2.11.3 Test.scala SomeAnnotation.java && javac SomeAnnotation.java && scala-hash v2.11.3 Test
Annotation is NOT present

lucmac:sandbox luc$ cat SomeAnnotation.java 
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface SomeAnnotation { }

lucmac:sandbox luc$ cat Test.scala 
@SomeAnnotation class A

object Test {
  def main(args: Array[String]): Unit = {
    if (classOf[A].isAnnotationPresent(classOf[SomeAnnotation])) {
      println("Annotation is present")
    }
    else {
      println("Annotation is NOT present")
    }
  }
}

@lrytz said:
For the real fix, i think we should go all the way and parse & type-check annotation expressions. Resolution of the annotation type (Retention) will be done symbolically in the namer / typer.

@gkossakowski said:
I'm joining the discussion because I'd like to get 2.11.4 out ASAP and this issue is blocking the release.

Did you guys agree whether to revert #4026 or change the default value for retention policy?

@lrytz said:
Yes: scala/scala#4067