Merge pull request scala#5622 from edmundnoble/extra-errs

Improved error messages for identically named, differently prefixed types

Author: adriaanm

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

@@ -62,7 +62,7 @@ trait Contexts { self: Analyzer =>
 
   def warnUnusedImports(unit: CompilationUnit) = if (!unit.isJava) {
     for (imps <- allImportInfos.remove(unit)) {
-      for (imp <- imps.reverse.distinct) {
+      for (imp <- imps.distinct.reverse) {
         val used = allUsedSelectors(imp)
 
         imp.tree.selectors filterNot (s => isMaskImport(s) || used(s)) foreach { sel =>

src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala

@@ -112,7 +112,7 @@ abstract class TreeCheckers extends Analyzer {
       else if (prevTrees exists (t => (t eq tree) || (t.symbol == sym)))
         ()
       else {
-        val s1 = (prevTrees map wholetreestr).sorted.distinct
+        val s1 = (prevTrees map wholetreestr).distinct.sorted
         val s2 = wholetreestr(tree)
         if (s1 contains s2) ()
         else movedMsgs += ("\n** %s moved:\n** Previously:\n%s\n** Currently:\n%s".format(ownerstr(sym), s1 mkString ", ", s2))

src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala

@@ -11,6 +11,7 @@ import scala.collection.mutable.ListBuffer
 import scala.util.control.Exception.ultimately
 import symtab.Flags._
 import PartialFunction._
+import scala.annotation.tailrec
 
 /** An interface to enable higher configurability of diagnostic messages
  *  regarding type errors.  This is barely a beginning as error messages are
@@ -274,19 +275,54 @@ trait TypeDiagnostics {
     if (AnyRefTpe <:< req) notAnyRefMessage(found) else ""
   }
 
+  def finalOwners(tpe: Type): Boolean = (tpe.prefix == NoPrefix) || recursivelyFinal(tpe)
+
+  @tailrec
+  final def recursivelyFinal(tpe: Type): Boolean = {
+    val prefix = tpe.prefix
+    if (prefix != NoPrefix) {
+      if (prefix.typeSymbol.isFinal) {
+        recursivelyFinal(prefix)
+      } else {
+        false
+      }
+    } else {
+      true
+    }
+  }
+
   // TODO - figure out how to avoid doing any work at all
   // when the message will never be seen.  I though context.reportErrors
   // being false would do that, but if I return "<suppressed>" under
   // that condition, I see it.
   def foundReqMsg(found: Type, req: Type): String = {
-    def baseMessage = (
-      ";\n found   : " + found.toLongString + existentialContext(found) + explainAlias(found) +
-       "\n required: " + req + existentialContext(req) + explainAlias(req)
-    )
-    (   withDisambiguation(Nil, found, req)(baseMessage)
-      + explainVariance(found, req)
-      + explainAnyVsAnyRef(found, req)
-    )
+    val foundWiden = found.widen
+    val reqWiden = req.widen
+    val sameNamesDifferentPrefixes =
+      foundWiden.typeSymbol.name == reqWiden.typeSymbol.name &&
+        foundWiden.prefix.typeSymbol != reqWiden.prefix.typeSymbol
+    val easilyMistakable =
+      sameNamesDifferentPrefixes &&
+      !req.typeSymbol.isConstant &&
+      finalOwners(foundWiden) && finalOwners(reqWiden) &&
+      !found.typeSymbol.isTypeParameterOrSkolem && !req.typeSymbol.isTypeParameterOrSkolem
+
+    if (easilyMistakable) {
+      val longestNameLength = foundWiden.nameAndArgsString.length max reqWiden.nameAndArgsString.length
+      val paddedFoundName = foundWiden.nameAndArgsString.padTo(longestNameLength, ' ')
+      val paddedReqName = reqWiden.nameAndArgsString.padTo(longestNameLength, ' ')
+      ";\n found   : " + (paddedFoundName + s" (in ${found.prefix.typeSymbol.fullNameString}) ") + explainAlias(found) +
+       "\n required: " + (paddedReqName + s" (in ${req.prefix.typeSymbol.fullNameString}) ") + explainAlias(req)
+    } else {
+      def baseMessage = {
+        ";\n found   : " + found.toLongString + existentialContext(found) + explainAlias(found) +
+         "\n required: " + req + existentialContext(req) + explainAlias(req)
+      }
+      (withDisambiguation(Nil, found, req)(baseMessage)
+        + explainVariance(found, req)
+        + explainAnyVsAnyRef(found, req)
+        )
+    }
   }
 
   def typePatternAdvice(sym: Symbol, ptSym: Symbol) = {
@@ -315,14 +351,6 @@ trait TypeDiagnostics {
     def restoreName()     = sym.name = savedName
     def modifyName(f: String => String) = sym setName newTypeName(f(sym.name.toString))
 
-    /** Prepend java.lang, scala., or Predef. if this type originated
-     *  in one of those.
-     */
-    def qualifyDefaultNamespaces() = {
-      val intersect = Set(trueOwner, aliasOwner) intersect UnqualifiedOwners
-      if (intersect.nonEmpty && tp.typeSymbolDirect.name == tp.typeSymbol.name) preQualify()
-    }
-
     // functions to manipulate the name
     def preQualify()   = modifyName(trueOwner.fullName + "." + _)
     def postQualify()  = if (!(postQualifiedWith contains trueOwner)) { postQualifiedWith ::= trueOwner; modifyName(_ + "(in " + trueOwner + ")") }
@@ -414,12 +442,6 @@ trait TypeDiagnostics {
         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 (_.qualifyDefaultNamespaces())
-
         // 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.

src/reflect/scala/reflect/internal/Types.scala

@@ -967,6 +967,8 @@ trait Types
      */
     def directObjectString = safeToString
 
+    def nameAndArgsString = typeSymbol.name.toString
+
     /** A test whether a type contains any unification type variables.
      *  Overridden with custom logic except where trivially true.
      */
@@ -2321,6 +2323,8 @@ trait Types
     private def preString  = if (needsPreString) pre.prefixString else ""
     private def argsString = if (args.isEmpty) "" else args.mkString("[", ",", "]")
 
+    override def nameAndArgsString = typeSymbol.name.toString + argsString
+
     private def refinementDecls = fullyInitializeScope(decls) filter (sym => sym.isPossibleInRefinement && sym.isPublic)
     private def refinementString = (
       if (sym.isStructuralRefinement)
@@ -2728,6 +2732,19 @@ trait Types
         arg.toString
     }
 
+    override def nameAndArgsString: String = underlying match {
+      case TypeRef(_, sym, args) if !settings.debug && isRepresentableWithWildcards =>
+        sym.name + wildcardArgsString(quantified.toSet, args).mkString("[", ",", "]")
+      case TypeRef(_, sym, args) =>
+        sym.name + args.mkString("[", ",", "]") + existentialClauses
+      case _ => underlying.typeSymbol.name + existentialClauses
+    }
+
+    private def existentialClauses = {
+      val str = quantified map (_.existentialToString) mkString (" forSome { ", "; ", " }")
+      if (settings.explaintypes) "(" + str + ")" else str
+    }
+
     /** An existential can only be printed with wildcards if:
      *   - the underlying type is a typeref
      *   - every quantified variable appears at most once as a type argument and
@@ -2746,7 +2763,7 @@ trait Types
             tpe.typeSymbol.isRefinementClass && (tpe.parents exists isQuantified)
           }
           val (wildcardArgs, otherArgs) = args partition (arg => qset contains arg.typeSymbol)
-          wildcardArgs.distinct == wildcardArgs &&
+          wildcardArgs.toSet.size == wildcardArgs.size &&
           !(otherArgs exists (arg => isQuantified(arg))) &&
           !(wildcardArgs exists (arg => isQuantified(arg.typeSymbol.info.bounds))) &&
           !(qset contains sym) &&
@@ -2756,17 +2773,13 @@ trait Types
     }
 
     override def safeToString: String = {
-      def clauses = {
-        val str = quantified map (_.existentialToString) mkString (" forSome { ", "; ", " }")
-        if (settings.explaintypes) "(" + str + ")" else str
-      }
       underlying match {
         case TypeRef(pre, sym, args) if !settings.debug && isRepresentableWithWildcards =>
           "" + TypeRef(pre, sym, Nil) + wildcardArgsString(quantified.toSet, args).mkString("[", ", ", "]")
         case MethodType(_, _) | NullaryMethodType(_) | PolyType(_, _) =>
-          "(" + underlying + ")" + clauses
+          "(" + underlying + ")" + existentialClauses
         case _ =>
-          "" + underlying + clauses
+          "" + underlying + existentialClauses
       }
     }
 

test/files/neg/no-predef.check

@@ -1,11 +1,11 @@
 no-predef.scala:2: error: type mismatch;
- found   : scala.Long(5L)
- required: java.lang.Long
+ found   : Long (in scala)
+ required: Long (in java.lang)
   def f1 = 5L: java.lang.Long
            ^
 no-predef.scala:3: error: type mismatch;
- found   : java.lang.Long
- required: scala.Long
+ found   : Long (in java.lang)
+ required: Long (in scala)
   def f2 = new java.lang.Long(5) : Long
            ^
 no-predef.scala:4: error: value map is not a member of String

test/files/neg/t2102.check

@@ -1,6 +1,6 @@
 t2102.scala:2: error: type mismatch;
- found   : java.util.Iterator[Int]
- required: scala.collection.Iterator[_]
+ found   : Iterator[Int] (in java.util)
+ required: Iterator[_]   (in scala.collection)
   val x: Iterator[_] = new java.util.ArrayList[Int]().iterator
                                                       ^
 one error found

test/files/neg/type-diagnostics.check

@@ -1,6 +1,6 @@
 type-diagnostics.scala:4: error: type mismatch;
- found   : scala.collection.Set[String]
- required: scala.collection.immutable.Set[String]
+ found   : Set[String] (in scala.collection)
+ required: Set[String] (in scala.collection.immutable)
   def f = Calculator("Hello", binding.keySet: collection.Set[String])
                                             ^
 type-diagnostics.scala:13: error: type mismatch;