Some refinement of the error messages when the ...

Some refinement of the error messages when the found and required types
have the same simple names. No longer must we watch people scratch their
heads at such messages as:

  found   : scala.collection.Set[String]
  required: Set[String]

Now so clear you could enjoy a movie through it:

  found   : scala.collection.Set[String]
  required: scala.collection.immutable.Set[String]

No review.

10 files changed, 138 insertions, 61 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index ef80177464..167a067485 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -150,6 +150,7 @@ trait Definitions extends reflect.generic.StandardDefinitions {
 
     // fundamental modules
     lazy val PredefModule: Symbol = getModule("scala.Predef")
+    lazy val PredefModuleClass = PredefModule.tpe.typeSymbol
       def Predef_classOf = getMember(PredefModule, nme.classOf)
       def Predef_error    = getMember(PredefModule, nme.error)
       def Predef_identity = getMember(PredefModule, nme.identity)
diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
index 76359ed46c..081be78c8b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
@@ -198,64 +198,107 @@ trait TypeDiagnostics {
        "\n required: " + req + existentialContext(req)
     }
 
-  /** If two given types contain different type variables with the same name
-   *  differentiate the names by including owner information.  Also, if the
-   *  type error is because of a conflict between two identically named
-   *  classes and one is in package scala, fully qualify the name so one
-   *  need not deduce why "java.util.Iterator" and "Iterator" don't match.
-   *  Another disambiguation performed is to address the confusion present
-   *  in the following snippet:
-   *    def f[Int](x: Int) = x + 5.
-   */
-  def withDisambiguation[T](types: Type*)(op: => T): T = {
-    object SymExtractor {
-      def unapply(x: Any) = x match {
-        case t @ TypeRef(_, sym, _)   => Some(t -> sym)
-        case t @ ConstantType(value)  => Some(t -> t.underlying.typeSymbol)
-        case _                        => None
-      }
-    }
-    val typerefs =
-      for (tp <- types.toList ; SymExtractor(t, sym) <- tp) yield
-        t -> sym
-
-    val savedNames    = typerefs map { case (_, sym) => sym -> sym.name } toMap
-    def restoreNames  = savedNames foreach { case (sym, name) => sym.name = name }
-
-    def isAlreadyAltered(sym: Symbol) = sym.name != savedNames(sym)
-
-    def modifyName(sym: Symbol)(f: String => String): Unit =
+  case class TypeDiag(tp: Type, sym: Symbol) extends Ordered[TypeDiag] {
+    // save the name because it will be mutated until it has been
+    // distinguished from the other types in the same error message
+    private val savedName = sym.name
+    def restoreName()     = sym.name = savedName
+    def isAltered         = sym.name != savedName
+    def modifyName(f: String => String) =
       sym.name = newTypeName(f(sym.name.toString))
 
-    def scalaQualify(sym: Symbol) =
-      if (sym.owner.isScalaPackageClass)
-        modifyName(sym)("scala." + _)
+    // functions to manipulate the name
+    def preQualify()   = modifyName(trueOwner.fullName + "." + _)
+    def postQualify()  = modifyName(_ + "(in " + trueOwner + ")")
+    def scalaQualify() = if (isInScalaOrPredef) preQualify()
+    def typeQualify()  = if (sym.isTypeParameterOrSkolem) postQualify()
+    def nameQualify()  = if (trueOwner.isPackageClass) preQualify() else postQualify()
 
-    def explainName(sym: Symbol) = {
-      scalaQualify(sym)
+    def trueOwner  = tp.typeSymbol.ownerSkipPackageObject
+    def aliasOwner = tp.typeSymbolDirect.ownerSkipPackageObject
+    def owners     = List(trueOwner, aliasOwner)
 
-      if (!isAlreadyAltered(sym))
-        modifyName(sym)(_ + "(in " + sym.owner + ")")
+    def isInScalaOrPredef = owners exists {
+      case ScalaPackageClass | PredefModuleClass => true
+      case _                                     => false
+    }
+
+    def sym_==(other: TypeDiag)     = tp.typeSymbol == other.tp.typeSymbol
+    def owner_==(other: TypeDiag)   = trueOwner == other.trueOwner
+    def string_==(other: TypeDiag)  = tp.toString == other.tp.toString
+    def name_==(other: TypeDiag)    = sym.name == other.sym.name
+
+    def compare(other: TypeDiag) =
+      if (this == other) 0
+      else if (sym isLess other.sym) -1
+      else 1
+
+    override def toString = {
+      """
+      |tp = %s
+      |tp.typeSymbol = %s
+      |tp.typeSymbol.owner = %s
+      |tp.typeSymbolDirect = %s
+      |tp.typeSymbolDirect.owner = %s
+      |isInScalaOrPredef = %s
+      """.stripMargin.format(
+        tp, tp.typeSymbol, tp.typeSymbol.owner, tp.typeSymbolDirect, tp.typeSymbolDirect.owner, isInScalaOrPredef
+      )
+    }
+  }
+  private def typeDiags(types: Type*): List[TypeDiag] = {
+    object SymExtractor {
+      def unapply(x: Any) = x match {
+        case t @ ConstantType(_)    => Some(t -> t.underlying.typeSymbol)
+        case t @ TypeRef(_, sym, _) => Some(t -> sym)
+        case _                      => None
+      }
     }
 
-    ultimately(restoreNames) {
-      for ((t1, sym1) <- typerefs ; (t2, sym2) <- typerefs ; if sym1 != sym2 && (sym1 isLess sym2)) {
+    for (tp <- types.toList ; SymExtractor(t, sym) <- tp) yield
+      TypeDiag(t, sym)
+  }
 
-        if (t1.toString == t2.toString) {   // type variable collisions
-          List(sym1, sym2) foreach explainName
-          if (sym1.owner == sym2.owner)
-            sym2.name = newTypeName("(some other)"+sym2.name)
-        }
-        else if (sym1.name == sym2.name) {  // symbol name collisions
-          List(sym1, sym2) foreach { x =>
-            if (x.owner.isScalaPackageClass)
-              modifyName(x)("scala." + _)
-            else if (x.isTypeParameterOrSkolem)
-              explainName(x)
-          }
+  /** The distinct pairs from an ordered list. */
+  private def pairs[T <: Ordered[T]](xs: Seq[T]): Seq[(T, T)] = {
+    for (el1 <- xs ; el2 <- xs ; if el1 < el2) yield
+      ((el1, el2))
+  }
+
+  /** Given any number of types, alters the name information in the symbols
+   *  until they can be distinguished from one another: then executes the given
+   *  code.  The names are restored and the result is returned.
+   */
+  def withDisambiguation[T](types: Type*)(op: => T): T = {
+    val typeRefs = typeDiags(types: _*)
+    val toCheck  = pairs(typeRefs) filterNot { case (td1, td2) => td1 sym_== td2 }
+
+    ultimately(typeRefs foreach (_.restoreName())) {
+      for ((td1, td2) <- toCheck) {
+        val tds = List(td1, td2)
+
+        // If the types print identically, qualify them:
+        //   a) If the dealiased owner is a package, the full path
+        //   b) Otherwise, append (in <owner>)
+        if (td1 string_== td2)
+          tds foreach (_.nameQualify())
+
+        // If they have the same simple name, and either of them is in the
+        // scala package or predef, qualify with scala so it is not confusing why
+        // e.g. java.util.Iterator and Iterator are different types.
+        if (td1 name_== td2)
+          tds foreach (_.scalaQualify())
+
+        // If they still print identically:
+        //   a) If they are type parameters with different owners, append (in <owner>)
+        //   b) Failing that, the best we can do is append "(some other)" to the latter.
+        if (td1 string_== td2) {
+          if (td1 owner_== td2)
+            td2.modifyName("(some other)" + _)
+          else
+            tds foreach (_.typeQualify())
         }
       }
-
       // performing the actual operation
       op
     }
diff --git a/test/files/neg/bug2102.check b/test/files/neg/bug2102.check
index 075aa85e48..7478fcfcea 100644
--- a/test/files/neg/bug2102.check
+++ b/test/files/neg/bug2102.check
@@ -1,6 +1,6 @@
 bug2102.scala:2: error: type mismatch;
  found   : java.util.Iterator[Int]
- required: scala.Iterator[_]
+ required: scala.collection.Iterator[_]
   val x: Iterator[_] = new java.util.ArrayList[Int]().iterator
                                                       ^
 one error found
diff --git a/test/files/neg/bug910.check b/test/files/neg/bug910.check
index 2bc2d986fa..1a845db9b9 100644
--- a/test/files/neg/bug910.check
+++ b/test/files/neg/bug910.check
@@ -1,6 +1,6 @@
 bug910.scala:4: error: type mismatch;
  found   : Seq[Char]
- required: scala.Seq[Int]
+ required: Seq[Int]
     val y: Seq[Int] = rest
                       ^
 one error found
diff --git a/test/files/neg/implicits.check b/test/files/neg/implicits.check
index d975ccfa84..f7923131e1 100644
--- a/test/files/neg/implicits.check
+++ b/test/files/neg/implicits.check
@@ -5,7 +5,7 @@ implicits.scala:38: error: type mismatch;
       ^
 implicits.scala:46: error: type mismatch;
  found   : List[Any]
- required: scala.List[Mxml]
+ required: List[Mxml]
         children.toList.flatMap ( e => {
                                 ^
 two errors found
diff --git a/test/files/neg/names-defaults-neg.check b/test/files/neg/names-defaults-neg.check
index 743496e0e1..4882c01e4c 100644
--- a/test/files/neg/names-defaults-neg.check
+++ b/test/files/neg/names-defaults-neg.check
@@ -89,13 +89,13 @@ names-defaults-neg.scala:76: error: no type parameters for method test4: (x: T[T
  --- because ---
 argument expression's type is not compatible with formal parameter type;
  found   : List[Int]
- required: ?T[ ?T[ scala.List[?T[ X forSome { type X } ]] ] ]
+ required: ?T[ ?T[ List[?T[ X forSome { type X } ]] ] ]
 Error occurred in an application involving default arguments.
   test4()
   ^
 names-defaults-neg.scala:79: error: type mismatch;
  found   : List[Int]
- required: scala.List[scala.List[?]]
+ required: List[List[?]]
   def test6[T](x: List[List[T]] = List(1,2)) = x
                                       ^
 names-defaults-neg.scala:82: error: type mismatch;
diff --git a/test/files/neg/t2180.check b/test/files/neg/t2180.check
index 58eb05b6b6..addc4cfbb8 100644
--- a/test/files/neg/t2180.check
+++ b/test/files/neg/t2180.check
@@ -1,6 +1,6 @@
 t2180.scala:3: error: type mismatch;
  found   : List[Any]
- required: scala.List[Mxml]
+ required: List[Mxml]
     children.toList.flatMap ( e => {
                             ^
 one error found
diff --git a/test/files/neg/type-diagnostics.check b/test/files/neg/type-diagnostics.check
new file mode 100644
index 0000000000..33e07f3816
--- /dev/null
+++ b/test/files/neg/type-diagnostics.check
@@ -0,0 +1,16 @@
+type-diagnostics.scala:4: error: type mismatch;
+ found   : scala.collection.Set[String]
+ required: scala.collection.immutable.Set[String]
+  def f = Calculator("Hello",binding.keySet)
+                                     ^
+type-diagnostics.scala:13: error: type mismatch;
+ found   : List[a(in method f2)]
+ required: List[a(in method f1)]
+      y match { case y1: List[a] => f3(x, y1) }
+                                          ^
+type-diagnostics.scala:17: error: type mismatch;
+ found   : String(in method f2)
+ required: java.lang.String
+  def f2[String](s: String) = strings(List(s))
+                                           ^
+three errors found
diff --git a/test/files/neg/type-diagnostics.scala b/test/files/neg/type-diagnostics.scala
new file mode 100644
index 0000000000..fdc0978138
--- /dev/null
+++ b/test/files/neg/type-diagnostics.scala
@@ -0,0 +1,18 @@
+object SetVsSet {
+  case class Calculator[+T](name: String, parameters: Set[String])
+  val binding = Map.empty[String, String]
+  def f = Calculator("Hello",binding.keySet)
+}
+
+object TParamConfusion {
+  def strings(xs: List[String]) = xs
+
+  def f1[a <% Ordered[a]](x: List[a]) = {
+    def f2[b >: List[a] <% Ordered[b]](x: List[a], y: b): Int = {
+      def f3(xs: List[a], ys: List[a]) = -1
+      y match { case y1: List[a] => f3(x, y1) }
+    }
+  }
+
+  def f2[String](s: String) = strings(List(s))
+}
diff --git a/test/pending/run/instanceOfAndTypeMatching.scala b/test/pending/run/instanceOfAndTypeMatching.scala
index 9ab2d6c3c4..60b11ef0c1 100644
--- a/test/pending/run/instanceOfAndTypeMatching.scala
+++ b/test/pending/run/instanceOfAndTypeMatching.scala
@@ -87,8 +87,7 @@ class Outer {
   }
 }
 
-object Test
-{
+object Test {
   val outer1 = new Outer
   val outer2 = new Outer
   val inner1 = new outer1.Inner
@@ -118,8 +117,8 @@ object Test
     List("These should be true under any scenario: ",
       inner1.isInstanceOf[outer1.Inner] ,
       inner1.isInstanceOf[Outer#Inner] ,
-      inner1 match { case _: Outer#Inner => true ; case _ => false } ,
-      inner1 match { case _: outer1.Inner => true ; case _ => false } ,
+      (inner1: Any) match { case _: Outer#Inner => true ; case _ => false } ,
+      (inner1: Any) match { case _: outer1.Inner => true ; case _ => false } ,
       inner1.compareSharpWithTypeMatch(inner2) ,
       inner1.compareSharpWithInstanceOf(inner2)
     ) foreach println
@@ -139,7 +138,7 @@ object Test
     ) foreach println
 
     List("These are doing the wrong thing under current proposal",
-      inner1 match { case _: outer2.Inner => true ; case _ => false }    // should be false
+      (inner1: Any) match { case _: outer2.Inner => true ; case _ => false }    // should be false
     ) foreach println
   }
 
@@ -159,7 +158,7 @@ object Test
     //  required: MethodInner where type MethodInner <: java.lang.Object with ScalaObject{def passOuter(other: Outer): Unit; def passThisType(other: Test.outer1.type): Unit; def passInner(other: Test.outer1.Inner): Unit; def passInner2(other: Test.outer1.Inner): Unit; def passInnerSharp(other: Outer#Inner): Unit; def passMethodInner(other: MethodInner): Unit}
     //     method1.passMethodInner(method1)
     //                             ^
-    // method1.passMethodInner(method1)
+    method1.passMethodInner(method1)
 
     // these should all fail to compile, and do
     //