Merge pull request #10422 from som-snytt/issue/12799-inherited-annots

Deduplicate JavaDeprecatedAttr in ClassfileParser

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -20,18 +20,17 @@ import java.lang.Integer.toHexString
 import java.net.URLClassLoader
 import java.util.UUID
 
-import scala.collection.{immutable, mutable}
-import scala.collection.mutable.{ArrayBuffer, ListBuffer}
 import scala.annotation.switch
+import scala.collection.{immutable, mutable}, mutable.{ArrayBuffer, ListBuffer}
 import scala.reflect.internal.JavaAccFlags
 import scala.reflect.internal.pickling.ByteCodecs
 import scala.reflect.internal.util.ReusableInstance
-import scala.tools.nsc.Reporting.WarningCategory
 import scala.reflect.io.{NoAbstractFile, PlainFile, ZipArchive}
-import scala.tools.nsc.util.ClassPath
+import scala.tools.nsc.Reporting.WarningCategory
 import scala.tools.nsc.io.AbstractFile
-import scala.tools.tasty.{TastyHeaderUnpickler, TastyReader}
+import scala.tools.nsc.util.ClassPath
 import scala.tools.nsc.tasty.{TastyUniverse, TastyUnpickler}
+import scala.tools.tasty.{TastyHeaderUnpickler, TastyReader}
 import scala.util.control.NonFatal
 
 /** This abstract class implements a class file parser.
@@ -820,8 +819,9 @@ abstract class ClassfileParser(reader: ReusableInstance[ReusableDataReader]) {
 
         case tpnme.DeprecatedATTR =>
           in.skip(attrLen)
-          sym.addAnnotation(JavaDeprecatedAttr)
-          if (sym == clazz)
+          if (!sym.hasAnnotation(JavaDeprecatedAttr))
+            sym.addAnnotation(JavaDeprecatedAttr)
+          if (sym == clazz && !staticModule.hasAnnotation(JavaDeprecatedAttr))
             staticModule.addAnnotation(JavaDeprecatedAttr)
 
         case tpnme.ConstantValueATTR =>
@@ -851,16 +851,9 @@ abstract class ClassfileParser(reader: ReusableInstance[ReusableDataReader]) {
 
         case tpnme.RuntimeAnnotationATTR =>
           val numAnnots = u2()
-          val annots = new ListBuffer[AnnotationInfo]
           numAnnots times {
-            parseAnnotation(u2()).foreach(annots.addOne)
+            parseAnnotation(u2()).foreach(addUniqueAnnotation(sym, _))
           }
-          /* `sym.withAnnotations(annots)`, like `sym.addAnnotation(annot)`, prepends,
-           * so if we parsed in classfile order we would wind up with the annotations
-           * in reverse order in `sym.annotations`. Instead we just read them out the
-           * other way around, for now. TODO: sym.addAnnotation add to the end?
-           */
-          sym.setAnnotations(sym.annotations ::: annots.toList)
 
         // TODO 1: parse runtime visible annotations on parameters
         // case tpnme.RuntimeParamAnnotationATTR
@@ -1510,6 +1503,25 @@ abstract class ClassfileParser(reader: ReusableInstance[ReusableDataReader]) {
 
   protected def getScope(flags: JavaAccFlags): Scope =
     if (flags.isStatic) staticScope else instanceScope
+
+  // Append annotation. For Java deprecation, prefer an annotation with values (since, etc).
+  private def addUniqueAnnotation(symbol: Symbol, annot: AnnotationInfo): symbol.type =
+    if (annot.atp.typeSymbol == JavaDeprecatedAttr) {
+      def ensureDepr(sym: Symbol): sym.type = {
+        if (sym.hasAnnotation(JavaDeprecatedAttr))
+          if (List(0, 1).exists(annot.constantAtIndex(_).isDefined))
+            sym.setAnnotations {
+              def drop(cur: AnnotationInfo): Boolean = cur.atp.typeSymbol == JavaDeprecatedAttr
+              sym.annotations.foldRight(annot :: Nil)((a, all) => if (drop(a)) all else a :: all)
+            }
+          else sym
+        else sym.addAnnotation(annot)
+      }
+      if (symbol == clazz)
+        ensureDepr(staticModule)
+      ensureDepr(symbol)
+    }
+    else symbol.addAnnotation(annot)
 }
 object ClassfileParser {
   private implicit class GoodTimes(private val n: Int) extends AnyVal {

src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala

@@ -304,7 +304,7 @@ abstract class Pickler extends SubComponent {
 
             putChildren(sym, children.toList sortBy (_.sealedSortName))
           }
-          for (annot <- (sym.annotations filter (ann => ann.isStatic && !ann.isErroneous)).reverse)
+          for (annot <- sym.annotations.filter(ann => ann.isStatic && !ann.isErroneous))
             putAnnotation(sym, annot)
         }
         else if (sym != NoSymbol) {

src/reflect/scala/reflect/internal/Symbols.scala

@@ -902,7 +902,10 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def isSerializable         = info.baseClasses.exists(_ == SerializableClass)
     def isDeprecated           = hasAnnotation(DeprecatedAttr) || (isJava && hasAnnotation(JavaDeprecatedAttr))
     def deprecationMessage     = getAnnotation(DeprecatedAttr) flatMap (_ stringArg 0)
-    def deprecationVersion     = getAnnotation(DeprecatedAttr) flatMap (_ stringArg 1)
+    def deprecationVersion     = getAnnotation(DeprecatedAttr).flatMap(_.stringArg(1)) match {
+                                   case v @ Some(_) => v
+                                   case _ => getAnnotation(JavaDeprecatedAttr).flatMap(_.stringArg(0))
+                                 }
     def deprecatedParamName    = getAnnotation(DeprecatedNameAttr) flatMap (ann => ann.symbolArg(0).orElse(ann.stringArg(0).map(newTermName)).orElse(Some(nme.NO_NAME)))
     def deprecatedParamVersion = getAnnotation(DeprecatedNameAttr) flatMap (_ stringArg 1)
     def hasDeprecatedInheritanceAnnotation
@@ -1913,8 +1916,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def filterAnnotations(p: AnnotationInfo => Boolean): this.type =
       setAnnotations(annotations filter p)
 
-    def addAnnotation(annot: AnnotationInfo): this.type =
-      setAnnotations(annot :: annotations)
+    def addAnnotation(annot: AnnotationInfo): this.type = setAnnotations(annotations.appended(annot))
 
     // Convenience for the overwhelmingly common cases, and avoid varags and listbuilders
     final def addAnnotation(sym: Symbol): this.type = {

test/files/neg/t12799.check

@@ -0,0 +1,9 @@
+Test_2.scala:8: warning: method answer in class C is deprecated (since beginning)
+  println(C.answer())
+            ^
+Test_2.scala:8: warning: class C in package example is deprecated (since you like it)
+  println(C.answer())
+          ^
+error: No warnings can be incurred under -Werror.
+2 warnings
+1 error

test/files/neg/t12799/C.java

@@ -0,0 +1,11 @@
+
+package example;
+
+@Deprecated(since="you like it")
+public class C {
+
+    @Deprecated(since="beginning")
+    public static int answer() {
+        return 42;
+    }
+}

test/files/neg/t12799/Test_2.scala

@@ -0,0 +1,9 @@
+
+// scalac: -Werror -Xlint
+// javaVersion: 9+
+
+import example._
+
+object Test extends App {
+  println(C.answer())
+}

test/files/run/t12799.check

@@ -0,0 +1,10 @@
+Test_2.scala:7: warning: class B in package example is deprecated (since 1.0): beeless
+  val deprecatedMethod = classOf[B].getMethod("a")
+                                ^
+Test_2.scala:9: warning: method a in trait A is deprecated
+  println(new B().a)
+                  ^
+Test_2.scala:9: warning: class B in package example is deprecated (since 1.0): beeless
+  println(new B().a)
+              ^
+a

test/files/run/t12799/A.java

@@ -0,0 +1,9 @@
+
+package example;
+
+interface A {
+    @Deprecated
+    default String a() {
+        return "a";
+    }
+}

test/files/run/t12799/B_1.scala

@@ -0,0 +1,5 @@
+
+package example
+
+@deprecated("beeless", since="1.0")
+class B extends A

test/files/run/t12799/Test_2.scala

@@ -0,0 +1,11 @@
+
+// scalac: -Xlint
+
+import example._
+
+object Test extends App {
+  val deprecatedMethod = classOf[B].getMethod("a")
+  assert(deprecatedMethod.isAnnotationPresent(classOf[Deprecated]))
+  println(new B().a)
+}
+//java.lang.annotation.AnnotationFormatError: Duplicate annotation for class: interface java.lang.Deprecated: @java.lang.Deprecated(forRemoval=false, since="")

test/files/run/t9644.scala

@@ -19,7 +19,7 @@ object Test extends App {
   val RetentionPolicy_RUNTIME = symbolOf[RetentionPolicy].companion.info.decl(TermName("RUNTIME"))
   r.tree match {
     case Apply(Select(New(tpt), _), List(NamedArg(Ident(TermName("value")), Literal(Constant(RetentionPolicy_RUNTIME))))) =>
-      assert (tpt.tpe.typeSymbol == symbolOf[Retention], tpt.tpe.typeSymbol)
+      assert(tpt.tpe.typeSymbol == symbolOf[Retention], tpt.tpe.typeSymbol)
   }
 
 }

test/files/run/t9644b/Foo.scala

@@ -0,0 +1,4 @@
+
+import java.lang.annotation._
+
+@Deprecated @Retention(RetentionPolicy.RUNTIME) class Foo

test/files/run/t9644b/Test_2.scala

@@ -0,0 +1,24 @@
+
+import java.lang.annotation._
+
+object Test extends App {
+  classOf[Foo].getAnnotation(classOf[Deprecated])
+
+  assert(classOf[Foo].getAnnotation(classOf[Retention]).value() == RetentionPolicy.RUNTIME)
+
+  import reflect.runtime.universe._
+
+  val List(d, r) = symbolOf[Foo].annotations
+
+  d.tree match {
+    case Apply(Select(New(tpt), _), Nil) =>
+      assert (tpt.tpe.typeSymbol == symbolOf[Deprecated], tpt.tpe.typeSymbol)
+  }
+
+  val RetentionPolicy_RUNTIME = symbolOf[RetentionPolicy].companion.info.decl(TermName("RUNTIME"))
+  r.tree match {
+    case Apply(Select(New(tpt), _), List(NamedArg(Ident(TermName("value")), Literal(Constant(RetentionPolicy_RUNTIME))))) =>
+      assert(tpt.tpe.typeSymbol == symbolOf[Retention], tpt.tpe.typeSymbol)
+  }
+
+}