Merge pull request #3747 from dotty-staging/fix-#3248-1

Fix #3248: Handle repeated arguments in results of unapply selectors

Author: odersky

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -77,7 +77,10 @@ class Definitions {
         val cls = denot.asClass.classSymbol
         val paramDecls = newScope
         val typeParam = enterSyntheticTypeParam(cls, paramFlags, paramDecls)
-        val parents = parentConstrs.toList
+        def instantiate(tpe: Type) =
+          if (tpe.typeParams.nonEmpty) tpe.appliedTo(typeParam.typeRef)
+          else tpe
+        val parents = parentConstrs.toList map instantiate
         denot.info = ClassInfo(ScalaPackageClass.thisType, cls, parents, paramDecls)
       }
     }

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -1366,6 +1366,15 @@ object Types {
      */
     def signature(implicit ctx: Context): Signature = Signature.NotAMethod
 
+    def annotatedToRepeated(implicit ctx: Context): Type = this match {
+      case tp @ ExprType(tp1) => tp.derivedExprType(tp1.annotatedToRepeated)
+      case AnnotatedType(tp, annot) if annot matches defn.RepeatedAnnot =>
+        val typeSym = tp.typeSymbol.asClass
+        assert(typeSym == defn.SeqClass || typeSym == defn.ArrayClass)
+        tp.translateParameterized(typeSym, defn.RepeatedParamClass)
+      case _ => this
+    }
+
     /** Convert to text */
     def toText(printer: Printer): Text = printer.toText(this)
 
@@ -2863,21 +2872,12 @@ object Types {
      *   - add @inlineParam to inline call-by-value parameters
      */
     def fromSymbols(params: List[Symbol], resultType: Type)(implicit ctx: Context) = {
-      def translateRepeated(tp: Type): Type = tp match {
-        case tp @ ExprType(tp1) => tp.derivedExprType(translateRepeated(tp1))
-        case AnnotatedType(tp, annot) if annot matches defn.RepeatedAnnot =>
-          val typeSym = tp.typeSymbol.asClass
-          assert(typeSym == defn.SeqClass || typeSym == defn.ArrayClass)
-          tp.translateParameterized(typeSym, defn.RepeatedParamClass)
-        case tp =>
-          tp
-      }
       def translateInline(tp: Type): Type = tp match {
         case _: ExprType => tp
         case _ => AnnotatedType(tp, Annotation(defn.InlineParamAnnot))
       }
       def paramInfo(param: Symbol) = {
-        val paramType = translateRepeated(param.info)
+        val paramType = param.info.annotatedToRepeated
         if (param.is(Inline)) translateInline(paramType) else paramType
       }
 

compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala

@@ -351,6 +351,8 @@ object PatternMatcher {
           }
         case WildcardPattern() =>
           onSuccess
+        case SeqLiteral(pats, _) =>
+          matchElemsPlan(scrutinee, pats, exact = true, onSuccess)
         case _ =>
           TestPlan(EqualTest(tree), scrutinee, tree.pos, onSuccess, onFailure)
       }

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

@@ -395,6 +395,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
       Typ(pat.tpe.stripAnnots, false)
     case Alternative(trees) => Or(trees.map(project(_)))
     case Bind(_, pat) => project(pat)
+    case SeqLiteral(pats, _) => projectSeq(pats)
     case UnApply(fun, _, pats) =>
       if (fun.symbol.name == nme.unapplySeq)
         if (fun.symbol.owner == scalaSeqFactoryClass)
@@ -496,7 +497,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
 
     debug.println(s"signature of ${unappSym.showFullName} ----> ${sig.map(_.show).mkString(", ")}")
 
-    sig
+    sig.map(_.annotatedToRepeated)
   }
 
   /** Decompose a type into subspaces -- assume the type can be decomposed */

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -47,7 +47,7 @@ object Applications {
     val ref = extractorMember(tp, name)
     if (ref.isOverloaded)
       errorType(i"Overloaded reference to $ref is not allowed in extractor", errorPos)
-    ref.info.widenExpr.dealias
+    ref.info.widenExpr.annotatedToRepeated.dealias
   }
 
   /** Does `tp` fit the "product match" conditions as an unapply result type
@@ -93,7 +93,10 @@ object Applications {
     def getTp = extractorMemberType(unapplyResult, nme.get, pos)
 
     def fail = {
-      ctx.error(i"$unapplyResult is not a valid result type of an $unapplyName method of an extractor", pos)
+      val addendum =
+        if (ctx.scala2Mode && unapplyName == nme.unapplySeq)
+          "\n You might want to try to rewrite the extractor to use `unapply` instead."
+      ctx.error(em"$unapplyResult is not a valid result type of an $unapplyName method of an extractor$addendum", pos)
       Nil
     }
 
@@ -959,13 +962,19 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
 
         var argTypes = unapplyArgs(unapplyApp.tpe, unapplyFn, args, tree.pos)
         for (argType <- argTypes) assert(!argType.isInstanceOf[TypeBounds], unapplyApp.tpe.show)
-        val bunchedArgs = argTypes match {
-          case argType :: Nil =>
-            if (argType.isRepeatedParam) untpd.SeqLiteral(args, untpd.TypeTree()) :: Nil
-            else if (args.lengthCompare(1) > 0 && ctx.canAutoTuple) untpd.Tuple(args) :: Nil
-            else args
-          case _ => args
-        }
+        val bunchedArgs =
+          if (argTypes.nonEmpty && argTypes.last.isRepeatedParam)
+            args.lastOption match {
+              case Some(arg @ Typed(argSeq, _)) if untpd.isWildcardStarArg(arg) =>
+                args.init :+ argSeq
+              case _ =>
+                val (regularArgs, varArgs) = args.splitAt(argTypes.length - 1)
+                regularArgs :+ untpd.SeqLiteral(varArgs, untpd.TypeTree())
+            }
+          else if (argTypes.lengthCompare(1) == 0 && args.lengthCompare(1) > 0 && ctx.canAutoTuple)
+            untpd.Tuple(args) :: Nil
+          else
+            args
         if (argTypes.length != bunchedArgs.length) {
           ctx.error(UnapplyInvalidNumberOfArguments(qual, argTypes), tree.pos)
           argTypes = argTypes.take(args.length) ++

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -1289,7 +1289,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
       case _ =>
         if (tree.name == nme.WILDCARD) body1
         else {
-          val sym = newPatternBoundSym(tree.name, body1.tpe, tree.pos)
+          val sym = newPatternBoundSym(tree.name, body1.tpe.underlyingIfRepeated(isJava = false), tree.pos)
           if (ctx.mode.is(Mode.InPatternAlternative))
             ctx.error(i"Illegal variable ${sym.name} in pattern alternative", tree.pos)
           assignType(cpy.Bind(tree)(tree.name, body1), sym)

tests/neg/i3248.scala

@@ -0,0 +1,10 @@
+class Test {
+  class Foo(val name: String, val children: Int *)
+  object Foo {
+    def unapplySeq(f: Foo) = Some((f.name, f.children))
+  }
+
+  def foo(f: Foo) = f match {
+    case Foo(name, ns: _*) => 1 // error: not a valid unapply result type
+  }
+}

tests/patmat/t8178.scala

@@ -4,7 +4,7 @@ case class FailsChild2(a: Seq[String]) extends Fails
 object FailsTest {
   def matchOnVarArgsFirstFails(f: Fails) = {
     f match {
-      case VarArgs1(_) => ???
+      case VarArgs1(_: _*) => ???
       // BUG: Without this line we should get a non-exhaustive match compiler error.
       //case FailsChild2(_) => ???
     }

tests/pos/i3248.scala

@@ -0,0 +1,13 @@
+object Test {
+  class Foo(val name: String, val children: Int *)
+  object Foo {
+    def unapply(f: Foo) = Some((f.name, f.children))
+  }
+
+  def foo(f: Foo) = f match {
+    case Foo(name, cs : _*) => name :: cs.reverse.toList.map(_.toString)
+  }
+  def main(args: Array[String]) = {
+    println(foo(new Foo("hi", 1, 2, 3)).mkString(" "))
+  }
+}

tests/run/i3248.scala

@@ -0,0 +1,23 @@
+object Test extends App {
+  class Foo(val name: String, val children: Int *)
+  object Foo {
+    def unapply(f: Foo) = Some((f.name, f.children))
+  }
+
+  def foo(f: Foo) = (f: Any) match {
+    case Foo(name, ns: _*) => ns.length
+    case List(ns: _*) => ns.length
+  }
+
+  case class Bar(val children: Int*)
+
+  def bar(f: Any) = f match {
+    case Bar(1, 2, 3) => 0
+    case Bar(a, b) => a + b
+    case Bar(ns: _*) => ns.length
+  }
+
+  assert(bar(new Bar(1, 2, 3)) == 0)
+  assert(bar(new Bar(3, 2, 1)) == 3)
+  assert(foo(new Foo("name", 1, 2, 3)) == 3)
+}