annotations cleanup/fix for slitting between fields and getter annotation targets

Author: adriaanm

src/compiler/scala/tools/nsc/transform/Fields.scala

@@ -275,6 +275,9 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
             val clonedAccessor = (accessor cloneSymbol clazz) setPos clazz.pos
             setMixedinAccessorFlags(accessor, clonedAccessor)
 
+            if (clonedAccessor.isGetter)
+              clonedAccessor setAnnotations (clonedAccessor.annotations filter AnnotationInfo.mkFilter(GetterTargetClass, defaultRetention = false))
+
             // if we don't cloneInfo, method argument symbols are shared between trait and subclasses --> lambalift proxy crash
             // TODO: use derive symbol variant?
             //            println(s"cloning accessor $accessor to $clazz / $clonedInfo -> $relativeInfo")

src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala

@@ -306,44 +306,42 @@ trait MethodSynthesis {
 
         result
       }
+
       final def derive(initial: List[AnnotationInfo]): Tree = {
         validate()
 
-        // Which meta-annotation is associated with this kind of entity.
-        // Presently one of: field, getter, setter, beanGetter, beanSetter, param.
-        // For traits, getter must play role of field as there isn't one until Constructors (we'll triage there)
-        val categories = this match {
-          case _: BaseGetter    => if (owner.isTrait) List(FieldTargetClass, GetterTargetClass) else List(GetterTargetClass)
-          case _: Setter        => List(SetterTargetClass)
-          case _: BeanSetter    => List(BeanSetterTargetClass)
-          case _: AnyBeanGetter => List(BeanGetterTargetClass)
-          case _: Param         => List(ParamTargetClass)
-          case _: Field         => List(FieldTargetClass)
-        }
-
-        // whether annotations whose definitions are not meta-annotated should be kept.
-        val defaultRetention = this match {
-          case _: Param => true
-          // By default annotations go to the field, except if the field is
-          // generated for a class parameter (PARAMACCESSOR).
-          case _: Field                       => !mods.isParamAccessor
-          case _: BaseGetter if owner.isTrait => true // like, Field, but cannot be a param accessor in a trait
-          case _                              => false
+        // see scala.annotation.meta's package class for more info
+        // Annotations on ValDefs can be targeted towards the following: field, getter, setter, beanGetter, beanSetter, param.
+        // The defaults are:
+        //   - (`val`-, `var`- or plain) constructor parameter annotations end up on the parameter, not on any other entity.
+        //   - val/var member annotations solely end up on the underlying field, except in traits (@since 2.12),
+        //     where there is no field, and the getter thus holds annotations targetting both getter & field.
+        //     As soon as there is a field/getter (in subclasses mixing in the trait), we triage the annotations.
+        //
+        // TODO: these defaults can be surprising for annotations not meant for accessors/fields -- should we revisit?
+        // (In order to have `@foo val X` result in the X getter being annotated with `@foo`, foo needs to be meta-annotated with @getter)
+        val annotFilter: AnnotationInfo => Boolean = this match {
+          case _: Param                       => annotationFilter(ParamTargetClass,      defaultRetention = true)
+          // By default annotations go to the field, except if the field is generated for a class parameter (PARAMACCESSOR).
+          case _: Field                       => annotationFilter(FieldTargetClass,      defaultRetention = !mods.isParamAccessor)
+          case _: BaseGetter if owner.isTrait => annotationFilter(List(FieldTargetClass, GetterTargetClass), defaultRetention = true)
+          case _: BaseGetter                  => annotationFilter(GetterTargetClass,     defaultRetention = false)
+          case _: Setter                      => annotationFilter(SetterTargetClass,     defaultRetention = false)
+          case _: BeanSetter                  => annotationFilter(BeanSetterTargetClass, defaultRetention = false)
+          case _: AnyBeanGetter               => annotationFilter(BeanGetterTargetClass, defaultRetention = false)
         }
 
-        val derivedAnnotations = initial filter annotationFilter(categories, defaultRetention)
-        // println(s"annots for $derivedSym: $derivedAnnotations (out of $initial)")
         // The annotations amongst those found on the original symbol which
         // should be propagated to this kind of accessor.
-        derivedSym setAnnotations derivedAnnotations
+        derivedSym setAnnotations (initial filter annotFilter)
 
         if (derivedSym.isSetter && owner.isTrait && !isDeferred)
           derivedSym addAnnotation TraitSetterAnnotationClass
 
         logDerived(derivedTree)
       }
-
     }
+
     sealed trait DerivedGetter extends DerivedFromValDef {
       def needsSetter = mods.isMutable
     }

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -461,7 +461,8 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
             overrideError("cannot be used here - only term macros can override term macros")
           } else {
             checkOverrideTypes()
-            checkOverrideDeprecated()
+            if (!other.hasFlag(SYNTHESIZE_IMPL_IN_SUBCLASS))
+              checkOverrideDeprecated()
             if (settings.warnNullaryOverride) {
               if (other.paramss.isEmpty && !member.paramss.isEmpty) {
                 reporter.warning(member.pos, "non-nullary method overrides nullary method")
@@ -1450,7 +1451,20 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       tree match {
         case m: MemberDef =>
           val sym = m.symbol
-          applyChecks(sym.annotations)
+          val allAnnots = sym.annotations
+
+          // drop field-targeting annotations from getters
+          // (in traits, getters must also hold annotations that target the underlying field,
+          //  because the latter won't be created until the trait is mixed into a class)
+          val annots =
+            if (sym.isGetter && sym.owner.isTrait) {
+              val annots = allAnnots filter AnnotationInfo.mkFilter(GetterTargetClass, defaultRetention = false)
+//              if (annots != allAnnots) println(s"filtering for $sym: $allAnnots to $annots")
+              sym setAnnotations annots
+              annots
+            } else allAnnots
+
+          applyChecks(annots)
 
           def messageWarning(name: String)(warn: String) =
             reporter.warning(tree.pos, f"Invalid $name message for ${sym}%s${sym.locationString}%s:%n$warn")

src/library/scala/deprecatedInheritance.scala

@@ -8,6 +8,8 @@
 
 package scala
 
+import scala.annotation.meta._
+
 /** An annotation that designates that inheriting from a class is deprecated.
  *
  *  This is usually done to warn about a non-final class being made final in a future version.
@@ -19,5 +21,6 @@ package scala
  *  @since  2.10
  *  @see    [[scala.deprecatedOverriding]]
  */
+@getter @setter @beanGetter @beanSetter
 private[scala] // for now, this needs to be generalized to communicate other modifier deltas
 class deprecatedInheritance(message: String = "", since: String = "") extends scala.annotation.StaticAnnotation

src/library/scala/deprecatedOverriding.scala

@@ -8,6 +8,8 @@
 
 package scala
 
+import scala.annotation.meta._
+
 /** An annotation that designates that overriding a member is deprecated.
  *
  *  Overriding such a member in  a sub-class then generates a warning.
@@ -17,5 +19,6 @@ package scala
  *  @since  2.10
  *  @see    [[scala.deprecatedInheritance]]
  */
+@getter @setter @beanGetter @beanSetter
 private[scala] // for the same reasons as deprecatedInheritance
 class deprecatedOverriding(message: String = "", since: String = "") extends scala.annotation.StaticAnnotation