From f26026a1a976066886c9e028a476f5d80eb1b56e Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Thu, 27 May 2021 21:08:56 +0200 Subject: [PATCH] forbid given patterns in val definitions. Suggest that they use an alias given instead. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 43 ++++++++++++++---- tests/neg/given-pattern.scala | 10 ++--- tests/neg/i11897.check | 45 +++++++++++++++++++ tests/neg/i11897.scala | 20 +++++++++ tests/run/i10178.scala | 39 ++++++++++++++++ 5 files changed, 143 insertions(+), 14 deletions(-) create mode 100644 tests/neg/i11897.check create mode 100644 tests/neg/i11897.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index af7a3b235717..870af91e083c 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1093,6 +1093,16 @@ object desugar { case IdPattern(named, tpt) => derivedValDef(original, named, tpt, rhs, mods) case _ => + + def filterWildcardGivenBinding(givenPat: Bind): Boolean = + givenPat.name != nme.WILDCARD + + def errorOnGivenBinding(bind: Bind)(using Context): Boolean = + report.error( + em"""${hl("given")} patterns are not allowed in a ${hl("val")} definition, + |please bind to an identifier and use an alias given.""".stripMargin, bind) + false + def isTuplePattern(arity: Int): Boolean = pat match { case Tuple(pats) if pats.size == arity => pats.forall(isVarPattern) @@ -1108,13 +1118,23 @@ object desugar { // - `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)` val tupleOptimizable = forallResults(rhs, isMatchingTuple) + val inAliasGenerator = original match + case _: GenAlias => true + case _ => false + val vars = if (tupleOptimizable) // include `_` - pat match { - case Tuple(pats) => - pats.map { case id: Ident => id -> TypeTree() } - } - else getVariables(pat) // no `_` + pat match + case Tuple(pats) => pats.map { case id: Ident => id -> TypeTree() } + else + getVariables( + tree = pat, + shouldAddGiven = + if inAliasGenerator then + filterWildcardGivenBinding + else + errorOnGivenBinding + ) // no `_` val ids = for ((named, _) <- vars) yield Ident(named.name) val caseDef = CaseDef(pat, EmptyTree, makeTuple(ids)) @@ -1800,16 +1820,21 @@ object desugar { /** Returns list of all pattern variables, possibly with their types, * without duplicates */ - private def getVariables(tree: Tree)(using Context): List[VarInfo] = { + private def getVariables(tree: Tree, shouldAddGiven: Context ?=> Bind => Boolean)(using Context): List[VarInfo] = { val buf = ListBuffer[VarInfo]() def seenName(name: Name) = buf exists (_._1.name == name) def add(named: NameTree, t: Tree): Unit = if (!seenName(named.name) && named.name.isTermName) buf += ((named, t)) def collect(tree: Tree): Unit = tree match { - case Bind(nme.WILDCARD, tree1) => + case tree @ Bind(nme.WILDCARD, tree1) => + if tree.mods.is(Given) then + val Typed(_, tpt) = tree1: @unchecked + if shouldAddGiven(tree) then + add(tree, tpt) collect(tree1) case tree @ Bind(_, Typed(tree1, tpt)) => - add(tree, tpt) + if !(tree.mods.is(Given) && !shouldAddGiven(tree)) then + add(tree, tpt) collect(tree1) case tree @ Bind(_, tree1) => add(tree, TypeTree()) @@ -1827,7 +1852,7 @@ object desugar { case SeqLiteral(elems, _) => elems foreach collect case Alternative(trees) => - for (tree <- trees; (vble, _) <- getVariables(tree)) + for (tree <- trees; (vble, _) <- getVariables(tree, shouldAddGiven)) report.error(IllegalVariableInPatternAlternative(), vble.srcPos) case Annotated(arg, _) => collect(arg) diff --git a/tests/neg/given-pattern.scala b/tests/neg/given-pattern.scala index 72f5213c9f06..68e322a2201c 100644 --- a/tests/neg/given-pattern.scala +++ b/tests/neg/given-pattern.scala @@ -4,11 +4,11 @@ class Test { import scala.collection.immutable.{TreeSet, HashSet} def f2[T](x: Ordering[T]) = { - val (given Ordering[T]) = x - new TreeSet[T] // error: no implicit ordering defined for T + val (given Ordering[T]) = x // error: given Ordering[T] not allowed here + new TreeSet[T] // error: no implicit ordering defined for T } def f3[T](x: Ordering[T]) = { - val given Ordering[T] = x - new TreeSet[T] // error: no implicit ordering defined for T + val given Ordering[T] = x // error: given Ordering[T] not allowed here + new TreeSet[T] // error: no implicit ordering defined for T } -} \ No newline at end of file +} diff --git a/tests/neg/i11897.check b/tests/neg/i11897.check new file mode 100644 index 000000000000..10191735122a --- /dev/null +++ b/tests/neg/i11897.check @@ -0,0 +1,45 @@ +-- Error: tests/neg/i11897.scala:11:10 --------------------------------------------------------------------------------- +11 | val (x, given A) = (1, A(23)) // error + | ^^^^^^^ + | given patterns are not allowed in a val definition, + | please bind to an identifier and use an alias given. +-- Error: tests/neg/i11897.scala:12:10 --------------------------------------------------------------------------------- +12 | val (_, given B) = (true, B(false)) // error + | ^^^^^^^ + | given patterns are not allowed in a val definition, + | please bind to an identifier and use an alias given. +-- Error: tests/neg/i11897.scala:13:8 ---------------------------------------------------------------------------------- +13 | val D(given C) = D(C("c")) // error + | ^^^^^^^ + | given patterns are not allowed in a val definition, + | please bind to an identifier and use an alias given. +-- Error: tests/neg/i11897.scala:14:11 --------------------------------------------------------------------------------- +14 | val F(y, given E) = F(47, E(93)) // error + | ^^^^^^^ + | given patterns are not allowed in a val definition, + | please bind to an identifier and use an alias given. +-- Error: tests/neg/i11897.scala:15:11 --------------------------------------------------------------------------------- +15 | val H(z, q @ given G) = H(47, G(101)) // error + | ^^^^^^^^^^^ + | given patterns are not allowed in a val definition, + | please bind to an identifier and use an alias given. +-- Error: tests/neg/i11897.scala:16:18 --------------------------------------------------------------------------------- +16 | assert(summon[A] == A(23)) // error + | ^ + | no implicit argument of type A was found for parameter x of method summon in object Predef +-- Error: tests/neg/i11897.scala:17:18 --------------------------------------------------------------------------------- +17 | assert(summon[B] == B(false)) // error + | ^ + | no implicit argument of type B was found for parameter x of method summon in object Predef +-- Error: tests/neg/i11897.scala:18:18 --------------------------------------------------------------------------------- +18 | assert(summon[C] == C("c")) // error + | ^ + | no implicit argument of type C was found for parameter x of method summon in object Predef +-- Error: tests/neg/i11897.scala:19:18 --------------------------------------------------------------------------------- +19 | assert(summon[E] == E(93)) // error + | ^ + | no implicit argument of type E was found for parameter x of method summon in object Predef +-- Error: tests/neg/i11897.scala:20:18 --------------------------------------------------------------------------------- +20 | assert(summon[G] == G(101)) // error + | ^ + | no implicit argument of type G was found for parameter x of method summon in object Predef diff --git a/tests/neg/i11897.scala b/tests/neg/i11897.scala new file mode 100644 index 000000000000..897a0026eecd --- /dev/null +++ b/tests/neg/i11897.scala @@ -0,0 +1,20 @@ +case class A(i: Int) +case class B(b: Boolean) +case class C(s: String) +case class D(c: C) +case class E(i: Int) +case class F(i: Int, e: E) +case class G(i: Int) +case class H(i: Int, e: G) + +def Test = + val (x, given A) = (1, A(23)) // error + val (_, given B) = (true, B(false)) // error + val D(given C) = D(C("c")) // error + val F(y, given E) = F(47, E(93)) // error + val H(z, q @ given G) = H(47, G(101)) // error + assert(summon[A] == A(23)) // error + assert(summon[B] == B(false)) // error + assert(summon[C] == C("c")) // error + assert(summon[E] == E(93)) // error + assert(summon[G] == G(101)) // error diff --git a/tests/run/i10178.scala b/tests/run/i10178.scala index 2225a154fcd0..335de0b0d986 100644 --- a/tests/run/i10178.scala +++ b/tests/run/i10178.scala @@ -3,3 +3,42 @@ x <- Option(23) given Int = x do assert(summon[Int] == 23) + + for + y <- Option("ok") + q @ given String = y + do assert(summon[String] == "ok") + + for + z <- Option("key" -> true) + (q @ given String, u @ given Boolean) = z + do + assert(summon[String] == "key") + assert(summon[Boolean] == true) + + for + w <- Option("no" -> false) + (given String, given Boolean) = w + do + assert(summon[String] == "no") + assert(summon[Boolean] == false) + + for + given Int <- Option(23) + do assert(summon[Int] == 23) + + for + q @ given String <- Option("ok") + do assert(summon[String] == "ok") + + for + (q @ given String, u @ given Boolean) <- Option("key" -> true) + do + assert(summon[String] == "key") + assert(summon[Boolean] == true) + + for + (given String, given Boolean) <- Option("no" -> false) + do + assert(summon[String] == "no") + assert(summon[Boolean] == false)