SI-8597 Improved pattern unchecked warnings

The spec says that `case _: List[Int]` should be always issue
an unchecked warning:

> Types which are not of one of the forms described above are
> also accepted as type patterns. However, such type patterns
> will be translated to their erasure (§3.7). The Scala compiler
> will issue an “unchecked” warning for these patterns to flag
> the possible loss of type-safety.

But the implementation goes a little further to omit warnings
based on the static type of the scrutinee. As a trivial example:

   def foo(s: Seq[Int]) = s match { case _: List[Int] => }

need not issue this warning.

These discriminating unchecked warnings are domain of
`CheckabilityChecker`.

Let's deconstruct the reported bug:

  def nowarn[T] = (null: Any) match { case _: Some[T] => }

We used to determine that if the first case matched, the scrutinee
type would be `Some[Any]` (`Some` is covariant). If this statically
matches `Some[T]` in a pattern context, we don't need to issue an
unchecked warning. But, our blanket use of `existentialAbstraction`
in `matchesPattern` loosened the pattern type to `Some[Any]`, and
the scrutinee type was deemed compatible.

I've added a new method, `scrutConformsToPatternType` which replaces
pattern type variables by wildcards, but leaves other abstract
types intact in the pattern type. We have to use this inside
`CheckabilityChecker` only. If we were to make `matchesPattern`
stricter in the same way, tests like `pos/t2486.scala` would fail.

I have introduced a new symbol test to (try to) identify pattern
type variables introduced by `typedBind`. Its not pretty, and it
might be cleaner to reserve a new flag for these.

I've also included a test variation exercising with nested matches.
The pattern type of the inner case can't, syntactically, refer to the
pattern type variable of the enclosing case. If it could, we would
have to be more selective in our wildcarding in `ptMatchesPatternType`
by restricting ourselves to type variables associated with the closest
enclosing `CaseDef`.

As some further validation of the correctness of this patch,
four stray warnings have been teased out of
neg/unchecked-abstract.scala

I also had to changes `typeArgsInTopLevelType` to extract the type
arguments of `Array[T]` if `T` is an abstract type. This avoids the
"Checkability checker says 'Uncheckable', but uncheckable type
cannot be found" warning and consequent overly lenient analysis.
Without this change, the warning was suppressed for:

    def warnArray[T] = (null: Any) match { case _: Array[T] => }

9 files changed, 115 insertions, 5 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
index 3a77cab919..fcc3f6901c 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
@@ -100,7 +100,7 @@ trait Checkable {
   private def typeArgsInTopLevelType(tp: Type): List[Type] = {
     val tps = tp match {
       case RefinedType(parents, _)              => parents flatMap typeArgsInTopLevelType
-      case TypeRef(_, ArrayClass, arg :: Nil)   => typeArgsInTopLevelType(arg)
+      case TypeRef(_, ArrayClass, arg :: Nil)   => if (arg.typeSymbol.isAbstractType) arg :: Nil else typeArgsInTopLevelType(arg)
       case TypeRef(pre, sym, args)              => typeArgsInTopLevelType(pre) ++ args
       case ExistentialType(tparams, underlying) => tparams.map(_.tpe) ++ typeArgsInTopLevelType(underlying)
       case _                                    => Nil
@@ -108,14 +108,31 @@ trait Checkable {
     tps filterNot isUnwarnableTypeArg
   }
 
+  private def scrutConformsToPatternType(scrut: Type, pattTp: Type): Boolean = {
+    def typeVarToWildcard(tp: Type) = {
+      // The need for typeSymbolDirect is demonstrated in neg/t8597b.scala
+      if (tp.typeSymbolDirect.isPatternTypeVariable) WildcardType else tp
+    }
+    val pattTpWild = pattTp.map(typeVarToWildcard)
+    scrut <:< pattTpWild
+  }
+
   private class CheckabilityChecker(val X: Type, val P: Type) {
     def Xsym = X.typeSymbol
     def Psym = P.typeSymbol
-    def XR   = if (Xsym == AnyClass) classExistentialType(Psym) else propagateKnownTypes(X, Psym)
+    def PErased = {
+      P match {
+        case erasure.GenericArray(n, core) => existentialAbstraction(core.typeSymbol :: Nil, P)
+        case _ => existentialAbstraction(Psym.typeParams, Psym.tpe_*)
+      }
+    }
+    def XR   = if (Xsym == AnyClass) PErased else propagateKnownTypes(X, Psym)
+
+
     // sadly the spec says (new java.lang.Boolean(true)).isInstanceOf[scala.Boolean]
-    def P1   = X matchesPattern P
+    def P1   = scrutConformsToPatternType(X, P)
     def P2   = !Psym.isPrimitiveValueClass && isNeverSubType(X, P)
-    def P3   = isNonRefinementClassType(P) && (XR matchesPattern P)
+    def P3   = isNonRefinementClassType(P) && scrutConformsToPatternType(XR, P)
     def P4   = !(P1 || P2 || P3)
 
     def summaryString = f"""
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 51f06b1d6d..b2f176e894 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -792,6 +792,10 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     final def isDefinedInPackage  = effectiveOwner.isPackageClass
     final def needsFlatClasses    = phase.flatClasses && rawowner != NoSymbol && !rawowner.isPackageClass
 
+    // TODO introduce a flag for these?
+    final def isPatternTypeVariable: Boolean =
+      isAbstractType && !isExistential && !isTypeParameterOrSkolem && isLocalToBlock
+
     /** change name by appending $$<fully-qualified-name-of-class `base`>
      *  Do the same for any accessed symbols or setters/getters.
      *  Implementation in TermSymbol.
diff --git a/test/files/neg/t8597.check b/test/files/neg/t8597.check
new file mode 100644
index 0000000000..bc945f9191
--- /dev/null
+++ b/test/files/neg/t8597.check
@@ -0,0 +1,21 @@
+t8597.scala:2: warning: abstract type T in type pattern Some[T] is unchecked since it is eliminated by erasure
+  def nowarn[T] = (null: Any) match { case _: Some[T]      => } // warn (did not warn due to SI-8597)
+                                              ^
+t8597.scala:5: warning: abstract type pattern T is unchecked since it is eliminated by erasure
+  def warn1[T]  = (null: Any) match { case _: T            => } // warn
+                                              ^
+t8597.scala:6: warning: non-variable type argument String in type pattern Some[String] is unchecked since it is eliminated by erasure
+  def warn2     = (null: Any) match { case _: Some[String] => } // warn
+                                              ^
+t8597.scala:7: warning: non-variable type argument Unchecked.this.C in type pattern Some[Unchecked.this.C] is unchecked since it is eliminated by erasure
+                  (null: Any) match { case _: Some[C]      => } // warn
+                                              ^
+t8597.scala:18: warning: abstract type T in type pattern Array[T] is unchecked since it is eliminated by erasure
+  def warnArray[T] = (null: Any) match { case _: Array[T] => } // warn (did not warn due to SI-8597)
+                                                 ^
+t8597.scala:26: warning: non-variable type argument String in type pattern Array[Array[List[String]]] is unchecked since it is eliminated by erasure
+  def warnArrayErasure2   = (null: Any) match {case Some(_: Array[Array[List[String]]]) => } // warn
+                                                            ^
+error: No warnings can be incurred under -Xfatal-warnings.
+6 warnings found
+one error found
diff --git a/test/files/neg/t8597.flags b/test/files/neg/t8597.flags
new file mode 100644
index 0000000000..85d8eb2ba2
--- /dev/null
+++ b/test/files/neg/t8597.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
diff --git a/test/files/neg/t8597.scala b/test/files/neg/t8597.scala
new file mode 100644
index 0000000000..068e87d91a
--- /dev/null
+++ b/test/files/neg/t8597.scala
@@ -0,0 +1,27 @@
+class Unchecked[C] {
+  def nowarn[T] = (null: Any) match { case _: Some[T]      => } // warn (did not warn due to SI-8597)
+
+  // These warned before.
+  def warn1[T]  = (null: Any) match { case _: T            => } // warn
+  def warn2     = (null: Any) match { case _: Some[String] => } // warn
+                  (null: Any) match { case _: Some[C]      => } // warn
+
+  // These must remain without warnings. These are excerpts from
+  // related tests that are more exhauative.
+  class C; class D extends C
+  def okay      = (List(new D) : Seq[D]) match { case _: List[C] => case _ => } // nowarn
+  class B2[A, B]
+  class A2[X] extends B2[X, String]
+  def okay2(x: A2[Int]) = x match { case _: B2[Int, _] => true } // nowarn
+  def okay3(x: A2[Int]) = x match { case _: B2[Int, typeVar] => true } // nowarn
+
+  def warnArray[T] = (null: Any) match { case _: Array[T] => } // warn (did not warn due to SI-8597)
+  def nowarnArrayC   = (null: Any) match { case _: Array[C] => } // nowarn
+
+  def nowarnArrayTypeVar[T] = (null: Any) match { case _: Array[t] => } // nowarn
+
+  def noWarnArrayErasure1 = (null: Any) match {case Some(_: Array[String]) => } // nowarn
+  def noWarnArrayErasure2 = (null: Any) match {case Some(_: Array[List[_]]) => } // nowarn
+  def noWarnArrayErasure3 = (null: Any) match {case Some(_: Array[Array[List[_]]]) => } // nowarn
+  def warnArrayErasure2   = (null: Any) match {case Some(_: Array[Array[List[String]]]) => } // warn
+}
diff --git a/test/files/neg/t8597b.check b/test/files/neg/t8597b.check
new file mode 100644
index 0000000000..3c45a31337
--- /dev/null
+++ b/test/files/neg/t8597b.check
@@ -0,0 +1,6 @@
+t8597b.scala:18: warning: non-variable type argument T in type pattern Some[T] is unchecked since it is eliminated by erasure
+        case _: Some[T] => // warn
+                ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/t8597b.flags b/test/files/neg/t8597b.flags
new file mode 100644
index 0000000000..85d8eb2ba2
--- /dev/null
+++ b/test/files/neg/t8597b.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
diff --git a/test/files/neg/t8597b.scala b/test/files/neg/t8597b.scala
new file mode 100644
index 0000000000..b29d591cb1
--- /dev/null
+++ b/test/files/neg/t8597b.scala
@@ -0,0 +1,21 @@
+object Unchecked {
+  (null: Any) match {
+    case _: Some[t] =>
+
+      // t is a fresh pattern type variable, despite our attempts to
+      // backtick our way to the enclosing `t`. Under this interpretation,
+      // the absense of an unchecked warning is expected.
+      (null: Any) match {
+        case _: Some[t] => // no warn
+      }
+      (null: Any) match {
+        case _: Some[`t`] => // no warn
+      }
+
+      // here we correctly issue an unchecked warning
+      type T = t
+      (null: Any) match {
+        case _: Some[T] => // warn
+      }
+  }
+}
diff --git a/test/files/neg/unchecked-abstract.check b/test/files/neg/unchecked-abstract.check
index 72019082ac..703929dca8 100644
--- a/test/files/neg/unchecked-abstract.check
+++ b/test/files/neg/unchecked-abstract.check
@@ -4,6 +4,9 @@ unchecked-abstract.scala:16: warning: abstract type H in type Contravariant[M.th
 unchecked-abstract.scala:21: warning: abstract type H in type Contravariant[M.this.H] is unchecked since it is eliminated by erasure
     /*   warn */ println(x.isInstanceOf[Contravariant[H]])
                                        ^
+unchecked-abstract.scala:22: warning: abstract type T in type Contravariant[M.this.T] is unchecked since it is eliminated by erasure
+    /*   warn */ println(x.isInstanceOf[Contravariant[T]])
+                                       ^
 unchecked-abstract.scala:27: warning: abstract type T in type Invariant[M.this.T] is unchecked since it is eliminated by erasure
     /*   warn */ println(x.isInstanceOf[Invariant[T]])
                                        ^
@@ -22,6 +25,15 @@ unchecked-abstract.scala:36: warning: abstract type H in type Invariant[M.this.H
 unchecked-abstract.scala:37: warning: abstract type T in type Invariant[M.this.T] is unchecked since it is eliminated by erasure
     /*   warn */ println(x.isInstanceOf[Invariant[T]])
                                        ^
+unchecked-abstract.scala:42: warning: abstract type T in type Covariant[M.this.T] is unchecked since it is eliminated by erasure
+    /*   warn */ println(x.isInstanceOf[Covariant[T]])
+                                       ^
+unchecked-abstract.scala:43: warning: abstract type L in type Covariant[M.this.L] is unchecked since it is eliminated by erasure
+    /*   warn */ println(x.isInstanceOf[Covariant[L]])
+                                       ^
+unchecked-abstract.scala:48: warning: abstract type L in type Covariant[M.this.L] is unchecked since it is eliminated by erasure
+    /*   warn */ println(x.isInstanceOf[Covariant[L]])
+                                       ^
 error: No warnings can be incurred under -Xfatal-warnings.
-8 warnings found
+12 warnings found
 one error found