Fixes #4008 validates annotation arguments

Niklas Vest · Niklas Vest · commit 800febf55149 · 2020-08-07T18:26:29.000+02:00
Both, annotated function arguments and annotated classes / traits,
are now checked for invalid type variable references and warn at
declaration site.
diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
@@ -164,7 +164,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     UnexpectedPatternForSummonFromID,
     AnonymousInstanceCannotBeEmptyID,
     TypeSpliceInValPatternID,
-    ModifierNotAllowedForDefinitionID
+    ModifierNotAllowedForDefinitionID,
+    InvalidReferenceInImplicitNotFoundAnnotationID
 
   def errorNumber = ordinal - 2
 }
diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -2390,3 +2390,11 @@ import ast.tpd
     def msg = s"Modifier `${flag.flagsString}` is not allowed for this definition"
     def explain = ""
   }
+
+  class InvalidReferenceInImplicitNotFoundAnnotation(typeVar: String, owner: String)(using Context)
+    extends ReferenceMsg(InvalidReferenceInImplicitNotFoundAnnotationID) {
+    def msg = em"""|Invalid reference to a type variable "${hl(typeVar)}" found in the annotation argument.
+                   |The variable does not occur in the signature of ${hl(owner)}.
+                   |""".stripMargin
+    def explain = ""
+  }
diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -4,7 +4,7 @@ package typer
 import transform._
 import core._
 import Symbols._, Types._, Contexts._, Flags._, Names._, NameOps._
-import StdNames._, Denotations._, SymUtils._, Phases._
+import StdNames._, Denotations._, SymUtils._, Phases._, SymDenotations._
 import NameKinds.DefaultGetterName
 import Annotations._
 import util.Spans._
@@ -19,9 +19,11 @@ import Decorators._
 import typer.ErrorReporting._
 import config.Feature.warnOnMigration
 import reporting._
+import scala.util.matching.Regex._
+import Constants.Constant
 
 object RefChecks {
-  import tpd.{Tree, MemberDef}
+  import tpd.{Tree, MemberDef, Literal, Template, DefDef}
 
   val name: String = "refchecks"
 
@@ -926,6 +928,74 @@ object RefChecks {
   }
 
   val NoLevelInfo: RefChecks.OptLevelInfo = new OptLevelInfo()
+
+  /** Verify that references in the user-defined `@implicitNotFound` message are valid.
+   *  (i.e. they refer to a type variable that really occurs in the signature of the annotated symbol.)
+   */
+  private object checkImplicitNotFoundAnnotation:
+
+    /** Warns if the class or trait has an @implicitNotFound annotation
+     *  with invalid type variable references.
+     */
+    def template(sd: SymDenotation)(using Context): Unit =
+      for
+        annotation <- sd.getAnnotation(defn.ImplicitNotFoundAnnot)
+        l@Literal(c: Constant) <- annotation.argument(0)
+      do forEachTypeVariableReferenceIn(c.stringValue) { case (ref, start) =>
+        if !sd.typeParams.exists(_.denot.name.show == ref) then
+          reportInvalidReferences(l, ref, start, sd)
+      }
+
+    /** Warns if the def has parameters with an `@implicitNotFound` annotation
+     *  with invalid type variable references.
+     */
+    def defDef(sd: SymDenotation)(using Context): Unit =
+      for
+        paramSymss <- sd.paramSymss
+        param <- paramSymss
+      do
+        for
+          annotation <- param.getAnnotation(defn.ImplicitNotFoundAnnot)
+          l@Literal(c: Constant) <- annotation.argument(0)
+        do forEachTypeVariableReferenceIn(c.stringValue) { case (ref, start) =>
+          if !sd.paramSymss.flatten.exists(_.name.show == ref) then
+            reportInvalidReferences(l, ref, start, sd)
+        }
+
+    /** Reports an invalid reference to a type variable `typeRef` that was found in `l` */
+    private def reportInvalidReferences(
+      l: Literal,
+      typeRef: String,
+      offsetInLiteral: Int,
+      sd: SymDenotation
+    )(using Context) =
+      val msg = InvalidReferenceInImplicitNotFoundAnnotation(
+        typeRef, if (sd.isConstructor) "the constructor" else sd.name.show)
+      val span = l.span.shift(offsetInLiteral + 1) // +1 because of 0-based index
+      val pos = ctx.source.atSpan(span.startPos)
+      report.warning(msg, pos)
+
+    /** Calls the supplied function for each quoted reference to a type variable in <pre>s</pre>.
+     *  The input
+     *
+     *  ```scala
+     *     "This is a ${T}ype re${F}erence"
+     *  //  ^0          ^12       ^22
+     *  ```
+     *
+     *  will lead to two invocations of `f`, once with `(T, 12)` and once with `(F, 22)` as argument.
+     *
+     * @param s The string to query for type variable references.
+     * @param f A function to apply to every pair of (\<type variable>, \<position in string>).
+     */
+    private def forEachTypeVariableReferenceIn(s: String)(f: (String, Int) => Unit) =
+      // matches quoted references such as "${(A)}", "${(Abc)}", etc.
+      val reference = """(?<=\$\{)[a-zA-Z]+(?=\})""".r
+      val matches = reference.findAllIn(s)
+      for m <- matches do f(m, matches.start)
+
+  end checkImplicitNotFoundAnnotation
+
 }
 import RefChecks._
 
@@ -1002,6 +1072,7 @@ class RefChecks extends MiniPhase { thisPhase =>
   override def transformDefDef(tree: DefDef)(using Context): DefDef = {
     checkNoPrivateOverrides(tree)
     checkDeprecatedOvers(tree)
+    checkImplicitNotFoundAnnotation.defDef(tree.symbol.denot)
     tree
   }
 
@@ -1012,6 +1083,7 @@ class RefChecks extends MiniPhase { thisPhase =>
     if (cls.is(Trait)) tree.parents.foreach(checkParentPrefix(cls, _))
     checkCompanionNameClashes(cls)
     checkAllOverrides(cls)
+    checkImplicitNotFoundAnnotation.template(cls.classDenot)
     tree
   }
   catch {
diff --git a/tests/neg-custom-args/fatal-warnings/i4008.check b/tests/neg-custom-args/fatal-warnings/i4008.check
@@ -0,0 +1,40 @@
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:5:56 ---------------------------------------
+5 |@annotation.implicitNotFound("An implicit ShouldWarn1[${B}] is not in scope") // error
+  |                                                        ^
+  |                                        Invalid reference to a type variable "B" found in the annotation argument.
+  |                                        The variable does not occur in the signature of ShouldWarn1.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:9:56 ---------------------------------------
+9 |@annotation.implicitNotFound("An implicit ShouldWarn2[${A}] is not in scope") // error
+  |                                                        ^
+  |                                        Invalid reference to a type variable "A" found in the annotation argument.
+  |                                        The variable does not occur in the signature of ShouldWarn2.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:13:56 --------------------------------------
+13 |@annotation.implicitNotFound("An implicit ShouldWarn3[${A},${B}] is not in scope") // error
+   |                                                        ^
+   |                                      Invalid reference to a type variable "A" found in the annotation argument.
+   |                                      The variable does not occur in the signature of ShouldWarn3.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:17:56 --------------------------------------
+17 |@annotation.implicitNotFound("An implicit ShouldWarn4[${A},${B}] is not in scope") // error // error
+   |                                                        ^
+   |                                      Invalid reference to a type variable "A" found in the annotation argument.
+   |                                      The variable does not occur in the signature of ShouldWarn4.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:17:61 --------------------------------------
+17 |@annotation.implicitNotFound("An implicit ShouldWarn4[${A},${B}] is not in scope") // error // error
+   |                                                             ^
+   |                                      Invalid reference to a type variable "B" found in the annotation argument.
+   |                                      The variable does not occur in the signature of ShouldWarn4.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:21:61 --------------------------------------
+21 |@annotation.implicitNotFound("An implicit ShouldWarn5[${C},${Abc}] is not in scope") // error
+   |                                                             ^
+   |                                    Invalid reference to a type variable "Abc" found in the annotation argument.
+   |                                    The variable does not occur in the signature of ShouldWarn5.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:44:54 --------------------------------------
+44 |class C[A](using @annotation.implicitNotFound("No C[${B}] found") c: Class[A]) // error
+   |                                                      ^
+   |                                      Invalid reference to a type variable "B" found in the annotation argument.
+   |                                      The variable does not occur in the signature of the constructor.
+-- [E157] Reference Error: tests/neg-custom-args/fatal-warnings/i4008.scala:46:62 --------------------------------------
+46 |def someMethod1[A](using @annotation.implicitNotFound("No C[${B}] found") sc: C[A]) = 0 // error
+   |                                                              ^
+   |                                      Invalid reference to a type variable "B" found in the annotation argument.
+   |                                      The variable does not occur in the signature of someMethod1.
diff --git a/tests/neg-custom-args/fatal-warnings/i4008.scala b/tests/neg-custom-args/fatal-warnings/i4008.scala
@@ -0,0 +1,48 @@
+// ===== Template annotations =====
+
+
+// class, 1TP, invalid ref
+@annotation.implicitNotFound("An implicit ShouldWarn1[${B}] is not in scope") // error
+class ShouldWarn1[A]
+
+// trait, 1TP, invalid ref
+@annotation.implicitNotFound("An implicit ShouldWarn2[${A}] is not in scope") // error
+trait ShouldWarn2[B]
+
+// trait, 2TP, 1 invalid ref
+@annotation.implicitNotFound("An implicit ShouldWarn3[${A},${B}] is not in scope") // error
+trait ShouldWarn3[B, C]
+
+// class, 2TP, 2 invalid refs
+@annotation.implicitNotFound("An implicit ShouldWarn4[${A},${B}] is not in scope") // error // error
+class ShouldWarn4[C, D]
+
+// class, 2TP, 1 invalid multi-char refs
+@annotation.implicitNotFound("An implicit ShouldWarn5[${C},${Abc}] is not in scope") // error
+class ShouldWarn5[C, D]
+
+// trait, 1TP, valid ref
+@annotation.implicitNotFound("An implicit ShouldntWarn1[${A}] is not in scope")
+trait ShouldntWarn1[A]
+
+// class, 2TP, only one ref but that one is valid
+@annotation.implicitNotFound("An implicit ShouldntWarn2[${A}, ...] is not in scope")
+class ShouldntWarn2[A, B]
+
+// trait, 2TP, 2 valid refs
+@annotation.implicitNotFound("An implicit ShouldntWarn3[${A}, ${B}] is not in scope")
+trait ShouldntWarn3[A, B]
+
+// class, 2TP, 2 valid refs
+@annotation.implicitNotFound("An implicit ShouldntWarn4[${Hello},${World}] is not in scope")
+class ShouldntWarn4[Hello, World]
+
+// ===== DefDef param annotations =====
+
+
+@annotation.implicitNotFound("Hopefully you don't see this!")
+class C[A](using @annotation.implicitNotFound("No C[${B}] found") c: Class[A]) // error
+
+def someMethod1[A](using @annotation.implicitNotFound("No C[${B}] found") sc: C[A]) = 0 // error
+
+def someMethod2[A](using @annotation.implicitNotFound("No C[${A}] found") sc: C[A]) = ""

Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {`
`164`	`164`	`UnexpectedPatternForSummonFromID,`
`165`	`165`	`AnonymousInstanceCannotBeEmptyID,`
`166`	`166`	`TypeSpliceInValPatternID,`
`167`		`- ModifierNotAllowedForDefinitionID`
	`167`	`+ ModifierNotAllowedForDefinitionID,`
	`168`	`+ InvalidReferenceInImplicitNotFoundAnnotationID`
`168`	`169`
`169`	`170`	`def errorNumber = ordinal - 2`
`170`	`171`	`}`