From 61f70e48cecddf27fba162c165dfaf712c84278c Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Mon, 14 Jan 2013 14:30:57 -0800 Subject: SI-6375, warn on lost annotation. Annotations on abstract vals which are not meta-annotated were silently discarded. Still discarded, only less silently. I warned on as many "lost annotation" situations as I was reasonably able to cover without false positives. --- .../tools/nsc/typechecker/MethodSynthesis.scala | 35 ++++++++++- test/files/neg/t6375.check | 27 +++++++++ test/files/neg/t6375.flags | 1 + test/files/neg/t6375.scala | 67 ++++++++++++++++++++++ 4 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/t6375.check create mode 100644 test/files/neg/t6375.flags create mode 100644 test/files/neg/t6375.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala index d74d5ecfbe..438c783810 100644 --- a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala +++ b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala @@ -126,7 +126,7 @@ trait MethodSynthesis { /** There are two key methods in here. * - * 1) Enter methods such as enterGetterSetterare called + * 1) Enter methods such as enterGetterSetter are called * from Namer with a tree which may generate further trees such as accessors or * implicit wrappers. Some setup is performed. In general this creates symbols * and enters them into the scope of the owner. @@ -171,14 +171,45 @@ trait MethodSynthesis { enterBeans(tree) } + /** This is called for those ValDefs which addDerivedTrees ignores, but + * which might have a warnable annotation situation. + */ + private def warnForDroppedAnnotations(tree: Tree) { + val annotations = tree.symbol.initialize.annotations + val targetClass = defaultAnnotationTarget(tree) + val retained = deriveAnnotations(annotations, targetClass, keepClean = true) + + annotations filterNot (retained contains _) foreach (ann => issueAnnotationWarning(ann, targetClass)) + } + private def issueAnnotationWarning(ann: AnnotationInfo, defaultTarget: Symbol) { + global.reporter.warning(ann.pos, + s"Annotation is unused - it can be retained with a meta-annotation such as @($ann @${defaultTarget.name})") + } + def addDerivedTrees(typer: Typer, stat: Tree): List[Tree] = stat match { case vd @ ValDef(mods, name, tpt, rhs) if !noFinishGetterSetter(vd) => // If we don't save the annotations, they seem to wander off. val annotations = stat.symbol.initialize.annotations - ( allValDefDerived(vd) + val trees = ( + allValDefDerived(vd) map (acc => atPos(vd.pos.focus)(acc derive annotations)) filterNot (_ eq EmptyTree) ) + // Verify each annotation landed safely somewhere, else warn. + // Filtering when isParamAccessor is a necessary simplification + // because there's a bunch of unwritten annotation code involving + // the propagation of annotations - constructor parameter annotations + // may need to make their way to parameters of the constructor as + // well as fields of the class, etc. + if (!mods.isParamAccessor) annotations foreach (ann => + if (!trees.exists(_.symbol hasAnnotation ann.symbol)) + issueAnnotationWarning(ann, GetterTargetClass) + ) + + trees + case vd: ValDef => + warnForDroppedAnnotations(vd) + vd :: Nil case cd @ ClassDef(mods, _, _, _) if mods.isImplicit => val annotations = stat.symbol.initialize.annotations // TODO: need to shuffle annotations between wrapper and class. diff --git a/test/files/neg/t6375.check b/test/files/neg/t6375.check new file mode 100644 index 0000000000..b94a067cbb --- /dev/null +++ b/test/files/neg/t6375.check @@ -0,0 +1,27 @@ +t6375.scala:6: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @getter) + @Bippy val x1: Int // warn + ^ +t6375.scala:7: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.field @getter) + @(Bippy @field) val x2: Int // warn + ^ +t6375.scala:9: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.setter @getter) + @(Bippy @setter) val x4: Int // warn + ^ +t6375.scala:10: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.param @getter) + @(Bippy @param) val x5: Int // warn + ^ +t6375.scala:20: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.getter @field) + @(Bippy @getter) private[this] val q1: Int = 1 // warn + ^ +t6375.scala:40: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.getter @param) + @(Bippy @getter) p2: Int, // warn + ^ +t6375.scala:41: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.setter @param) + @(Bippy @setter) p3: Int, // warn + ^ +t6375.scala:42: warning: Annotation is unused - it can be retained with a meta-annotation such as @(Bippy @scala.annotation.meta.field @param) + @(Bippy @field) p4: Int // warn + ^ +error: No warnings can be incurred under -Xfatal-warnings. +8 warnings found +one error found diff --git a/test/files/neg/t6375.flags b/test/files/neg/t6375.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/t6375.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t6375.scala b/test/files/neg/t6375.scala new file mode 100644 index 0000000000..21634df688 --- /dev/null +++ b/test/files/neg/t6375.scala @@ -0,0 +1,67 @@ +import scala.annotation.meta._ + +class Bippy extends scala.annotation.StaticAnnotation + +abstract class Foo { + @Bippy val x1: Int // warn + @(Bippy @field) val x2: Int // warn + @(Bippy @getter) val x3: Int // no warn + @(Bippy @setter) val x4: Int // warn + @(Bippy @param) val x5: Int // warn +} + +object Bar extends Foo { + val x1 = 1 + val x2 = 2 + val x3 = 3 + val x4 = 4 + val x5 = 5 + + @(Bippy @getter) private[this] val q1: Int = 1 // warn + @(Bippy @getter) private val q2: Int = 1 // no warn + + def f1(@(Bippy @param) x: Int): Int = 0 // no warn + def f2(@(Bippy @getter) x: Int): Int = 0 // warn - todo + def f3(@(Bippy @setter) x: Int): Int = 0 // warn - todo + def f4(@(Bippy @field) x: Int): Int = 0 // warn - todo + def f5(@Bippy x: Int): Int = 0 // no warn + + @(Bippy @companionClass) def g1(x: Int): Int = 0 // warn - todo + @(Bippy @companionObject) def g2(x: Int): Int = 0 // warn - todo + @(Bippy @companionMethod) def g3(x: Int): Int = 0 // no warn + @Bippy def g4(x: Int): Int = 0 // no warn + + @(Bippy @companionObject @companionMethod) def g5(x: Int): Int = 0 // no warn +} + +class Dingo( + @Bippy p0: Int, // no warn + @(Bippy @param) p1: Int, // no warn + @(Bippy @getter) p2: Int, // warn + @(Bippy @setter) p3: Int, // warn + @(Bippy @field) p4: Int // warn +) + +class ValDingo( + @Bippy val p0: Int, // no warn + @(Bippy @param) val p1: Int, // no warn + @(Bippy @getter) val p2: Int, // no warn + @(Bippy @setter) val p3: Int, // warn - todo + @(Bippy @field) val p4: Int // no warn +) + +class VarDingo( + @Bippy var p0: Int, // no warn + @(Bippy @param) var p1: Int, // no warn + @(Bippy @getter) var p2: Int, // no warn + @(Bippy @setter) var p3: Int, // no warn + @(Bippy @field) var p4: Int // no warn +) + +case class CaseDingo( + @Bippy p0: Int, // no warn + @(Bippy @param) p1: Int, // no warn + @(Bippy @getter) p2: Int, // no warn + @(Bippy @setter) p3: Int, // warn - todo + @(Bippy @field) p4: Int // no warn +) -- cgit v1.2.3