Deduplicate JavaDeprecatedAttr in ClassfileParser

[som-snytt]

Fixes scala/bug#12799

[SethTisue]

👀

!!    1 - run/t9644.scala                           [output differs]
% diff /home/jenkins/workspace/scala-2.13.x-validate-main/test/files/run/t9644.check /home/jenkins/workspace/scala-2.13.x-validate-main/test/files/run/t9644-run.log--- empty
+++ t9644-run.log
@@ -1,0 +1,42 @@
+scala.MatchError: new java.lang.annotation.Retention(value = RUNTIME) (of class scala.reflect.internal.Trees$Apply)
+	at Test$.delayedEndpoint$Test$1(t9644.scala:14)
+	at Test$delayedInit$body.apply(t9644.scala:5)
+	at scala.Function0.apply$mcV$sp(Function0.scala:42)

[som-snytt]

This implements the old hrhino suggestion to maintain annotation order in sym.addAnnotation, which changes the order for annotations other than runtime attrs read by class file parser.

Still need to check whether there was a general assumption that annotations from source were built in reverse.

[som-snytt]

My first edit retained last added annotation, which I changed when I fixed the failing test. It makes sense to prefer the DeprecatedAttr to its runtime twin; probably they are identical. Scala annotations maybe be multiple, such as throws.

[som-snytt]

Reminder to self to check how deprecation is emitted.

[som-snytt]

This now implements the lrytz suggestion to confine the special handling to class file parser.

[som-snytt]

Of course the since test must be constrained to javaVersion: 9+, as Lukas had mentioned on an old ticket somewhere. So the neg test is for that check, and the run test always runs (so however CI is set up, it must undergo a jdk 8 test).

[som-snytt]

Thanks, I was so consumed with the previous edit that I did not drill down to answer your question.

[lrytz]

Are we sure that the DeprecatedATTR is visited before the anntoation, or should we safe-guard there too? https://github.com/scala/scala/blob/v2.13.11/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala#L827-L831

Also, for the attribute we also add an annotation to the synthetic companion module symbol, but we don't do so for the annotation. This means the "since" message doesn't show when accessing a static member:

scala> new A
           ^
       warning: class A is deprecated (since you like it)
val res0: A = A@2c26ba07

scala> A.foo
       ^
       warning: class A is deprecated

[som-snytt]

I intended not to assume DeprecatedATTR is seen before the runtime attr, which is why the previous edit touched addAnnotation, but I neglected to update that spot to use the new method on the last refactor.

I'll tackle Nr 2 after coffee.

[som-snytt]

I noticed on a recent dotty ticket about @BeanProperty that Scala treats @Deprecated as an ordinary annotation only. But @deprecated distributes deprecation attribute to all var accessors. I haven't checked for the Scala annotation yet.

Edit: @deprecated winds up on everything, including both mutators if @BeanProperty. Actually the meta annots are listed in the Scaladoc.

bean.scala:34: warning: variable y in class Bean is deprecated (since 0.1): Bad var
  def reset() = b.y = 0
                  ^
bean.scala:35: warning: method setY in class Bean is deprecated (since 0.1): Bad var
  def kill() = b.setY(0)
                 ^

[som-snytt]

Last commit addresses both review comments: There is no reason to assume attribute ordering, so don't add plain deprecated if it already has it. There is no test. When adding the annotation, also check the static module, verified by test.

Did not squash! (My tweaking should be squashed.)

[lrytz]

LGTM otherwise. I think this can be a single commit.

[lrytz]

Thank you !