Fix #1828: add warning when patternmatching on generics

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

@@ -357,6 +357,9 @@ class Definitions {
       List(AnyClass.typeRef), EmptyScope)
   def SingletonType = SingletonClass.typeRef
 
+  lazy val ListType: TypeRef = ctx.requiredClassRef("scala.collection.immutable.List")
+  def ListClass(implicit ctx: Context) = ListType.symbol.asClass
+
   lazy val SeqType: TypeRef = ctx.requiredClassRef("scala.collection.Seq")
   def SeqClass(implicit ctx: Context) = SeqType.symbol.asClass
 

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

@@ -56,6 +56,7 @@ public enum ErrorMessageID {
     CyclicReferenceInvolvingID,
     CyclicReferenceInvolvingImplicitID,
     SuperQualMustBeParentID,
+    ErasedTypeID,
     ;
 
     public int errorNumber() {

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -1221,4 +1221,11 @@ object messages {
            |Attempting to define a field in a method signature after a varargs field is an error.
            |""".stripMargin
   }
+
+  case class ErasedType(val explanation: String = "")(implicit ctx: Context)
+  extends Message(ErasedTypeID) {
+    val kind = "Erased Type"
+    val msg =
+      i"abstract type pattern is unchecked since it is eliminated by erasure"
+  }
 }

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

@@ -5,7 +5,8 @@ import dotty.tools.dotc.util.Positions._
 import TreeTransforms.{MiniPhaseTransform, TransformerInfo}
 import core._
 import Contexts.Context, Types._, Constants._, Decorators._, Symbols._
-import TypeUtils._, TypeErasure._, Flags._
+import TypeUtils._, TypeErasure._, Flags._, TypeApplications._
+import reporting.diagnostic.messages._
 
 /** Implements partial evaluation of `sc.isInstanceOf[Sel]` according to:
  *
@@ -18,6 +19,11 @@ import TypeUtils._, TypeErasure._, Flags._
  *  This is a generalized solution to raising an error on unreachable match
  *  cases and warnings on other statically known results of `isInstanceOf`.
  *
+ *  This phase also warns if the erased type parameter of a parameterized type
+ *  is used in a match where it would be erased to `Object` or if the
+ *  typeparameters are removed. Both of these cases could cause surprising
+ *  behavior for the users.
+ *
  *  Steps taken:
  *
  *  1. `evalTypeApply` will establish the matrix and choose the appropriate
@@ -128,6 +134,67 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer =>
           val selClassNonFinal = selClass && !(selector.typeSymbol is Final)
           val selFinalClass    = selClass && (selector.typeSymbol is Final)
 
+          // Check if the selector's potential type parameters will be erased, and if so warn
+          val selTypeParam = tree.args.head.tpe.widen match {
+            case tp @ AppliedType(_, args @ (arg :: _)) =>
+              // If the type is `Array[X]` where `X` is a primitive value
+              // class. In the future, when we have a solid implementation of
+              // Arrays of value classes, we might be able to relax this check.
+              val anyValArray = tp.isRef(defn.ArrayClass) && arg.typeSymbol.isPrimitiveValueClass
+              // param is: Any | AnyRef | java.lang.Object
+              val topType = defn.ObjectType <:< arg
+              // has @unchecked annotation to suppress warnings
+              val hasUncheckedAnnot = arg.hasAnnotation(defn.UncheckedAnnot)
+
+              // Shouldn't warn when matching on a subclass with underscore
+              // params or type binding
+              val matchingUnderscoresOrTypeBindings = args.forall(_ match {
+                case tr: TypeRef =>
+                  tr.symbol.is(BindDefinedType)
+                case TypeBounds(lo, hi) =>
+                  (lo eq defn.NothingType) && (hi eq defn.AnyType)
+                case _ => false
+              }) && selector <:< scrutinee
+
+              // we don't want to warn when matching on `List` from `Seq` e.g:
+              // (xs: Seq[Int]) match { case xs: List[Int] => ??? }
+              val matchingSeqToList = {
+                val hasSameTypeArgs = s.qualifier.tpe.widen match {
+                  case AppliedType(_, scrutArg :: Nil) =>
+                    (scrutArg eq arg) || arg <:< scrutArg
+                  case _ => false
+                }
+
+                scrutinee.isRef(defn.SeqClass) &&
+                tp.isRef(defn.ListClass) &&
+                hasSameTypeArgs
+              }
+
+              val shouldWarn =
+                !topType && !hasUncheckedAnnot &&
+                !matchingUnderscoresOrTypeBindings && !matchingSeqToList &&
+                !anyValArray
+
+              if (shouldWarn) ctx.uncheckedWarning(
+                ErasedType(hl"""|Since type parameters are erased, you should not match on them in
+                                |${"match"} expressions."""),
+                tree.pos
+              )
+              true
+            case _ =>
+              if (tree.args.head.symbol.is(TypeParam)) {
+                ctx.uncheckedWarning(
+                  ErasedType(
+                    hl"""|`${tree.args.head.tpe}` will be erased to `${selector}`. Which means that the specified
+                         |behavior could be different during runtime."""
+                  ),
+                  tree.pos
+                )
+                true
+              }
+              else false
+          }
+
           // Cases ---------------------------------
           val valueClassesOrAny =
             ValueClasses.isDerivedValueClass(scrutinee.typeSymbol) ||
@@ -148,7 +215,7 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer =>
 
           val inMatch = s.qualifier.symbol is Case
 
-          if (valueClassesOrAny) tree
+          if (selTypeParam || valueClassesOrAny) tree
           else if (knownStatically)
             handleStaticallyKnown(s, scrutinee, selector, inMatch, tree.pos)
           else if (falseIfUnrelated && scrutinee <:< selector)

compiler/test/dotty/tools/dotc/repl/TestREPL.scala

@@ -10,7 +10,7 @@ import org.junit.Test
 
 /** A subclass of REPL used for testing.
  *  It takes a transcript of a REPL session in `script`. The transcript
- *  starts with the first input prompt `scala> ` and ends with `scala> :quit` and a newline.
+ *  starts with the first input prompt `scala> ` and ends with `scala> :quit`.
  *  Invoking `process()` on the `TestREPL` runs all input lines and
  *  collects then interleaved with REPL output in a string writer `out`.
  *  Invoking `check()` checks that the collected output matches the original
@@ -26,6 +26,7 @@ class TestREPL(script: String) extends REPL {
     override def context(ctx: Context) = {
       val fresh = ctx.fresh
       fresh.setSetting(ctx.settings.color, "never")
+      fresh.setSetting(ctx.settings.unchecked, true)
       fresh.setSetting(ctx.settings.classpath, Jars.dottyReplDeps.mkString(":"))
       fresh.initialize()(fresh)
       fresh

tests/repl/erasure.check

@@ -0,0 +1,64 @@
+scala> def unsafeCast[S](a: Any) = a match { case s: S => s; case _ => ??? }
+-- [E048] Erased Type Unchecked Warning: <console> -----------------------------
+4 |def unsafeCast[S](a: Any) = a match { case s: S => s; case _ => ??? }
+  |                                                ^
+  |      abstract type pattern is unchecked since it is eliminated by erasure
+
+longer explanation available when compiling with `-explain`
+def unsafeCast[S](a: Any): [S](a: Any)S
+scala> unsafeCast[String](1)
+java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String
+	at .<init>(<console>:6)
+	at .<clinit>(<console>)
+	at RequestResult$.<init>(<console>:3)
+	at RequestResult$.<clinit>(<console>)
+	at RequestResult$result(<console>)
+	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
+	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.jav...
+scala> class A; def unsafeCast2[S <: A](a: Any) = a match { case s: S => s; case _ => ??? }
+-- [E048] Erased Type Unchecked Warning: <console> -----------------------------
+4 |class A; def unsafeCast2[S <: A](a: Any) = a match { case s: S => s; case _ => ??? }
+  |                                                               ^
+  |      abstract type pattern is unchecked since it is eliminated by erasure
+
+longer explanation available when compiling with `-explain`
+defined class A
+def unsafeCast2[S <: A](a: Any): [S <: A](a: Any)S
+scala> def matchArray1[A](xs: Array[A]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ??? }
+def matchArray1[A](xs: Array[A]): [A](xs: Array[A])Array[Int]
+scala> def matchArray2[A](xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ??? }
+-- [E048] Erased Type Unchecked Warning: <console> -----------------------------
+5 |def matchArray2[A](xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ??? }
+  |                                                                                             ^
+  |      abstract type pattern is unchecked since it is eliminated by erasure
+
+longer explanation available when compiling with `-explain`
+def matchArray2[A](xs: Array[Any]): [A](xs: Array[Any])Array[Int]
+scala> def matchArray3[A](xs: Array[A]) = xs match { case xs: Array[Int] => xs; case xs: Array[AnyRef] => ???; case xs: Array[Any] => ??? }
+def matchArray3[A](xs: Array[A]): [A](xs: Array[A])Array[Int]
+scala> def matchArray4(xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ???; case xs: Array[Any] => ??? }
+-- [E048] Erased Type Unchecked Warning: <console> -----------------------------
+5 |def matchArray4(xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ???; case xs: Array[Any] => ??? }
+  |                                                                                          ^
+  |      abstract type pattern is unchecked since it is eliminated by erasure
+
+longer explanation available when compiling with `-explain`
+def matchArray4(xs: Array[Any]): Array[Int]
+scala> def matchArray5[A](xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A @unchecked] => ??? }
+def matchArray5[A](xs: Array[Any]): [A](xs: Array[Any])Array[Int]
+scala> def matchList1(xs: List[Any]) = xs match { case xs: List[Int @unchecked] => ??? }
+def matchList1(xs: List[Any]): Nothing
+scala> def matchList2(xs: List[Any]) = xs match { case List() => ???; case _ => ??? }
+def matchList2(xs: List[Any]): Nothing
+scala> def matchList3(xs: Seq[_]) = xs match { case List() => ???; case _ => ??? }
+def matchList3(xs: Seq[_]): Nothing
+scala> val xs: Seq[Int] = Seq(1,2,3)
+val xs: scala.collection.Seq[Int] = List(1, 2, 3)
+scala> val xsMatch = xs match { case ss: List[Int] => 1 }
+val xsMatch: Int = 1
+scala> trait Foo[X]; trait Bar[X,Y]
+defined trait Foo
+defined trait Bar
+scala> val underScoreMatch = (Nil: Any) match { case _: Foo[_] => ???; case _: Bar[_,_] => ???; case _ => 0 }
+val underScoreMatch: Int = 0
+scala> :quit