From d1fc98370ad59518de96ae52e035e36b145180e2 Mon Sep 17 00:00:00 2001 From: Edmund Noble Date: Mon, 26 Dec 2016 17:43:04 -0500 Subject: Improved error messages for identically named, differently prefixed types --- .../scala/tools/nsc/typechecker/Contexts.scala | 2 +- .../scala/tools/nsc/typechecker/TreeCheckers.scala | 2 +- .../tools/nsc/typechecker/TypeDiagnostics.scala | 63 ++++++++++++++-------- src/reflect/scala/reflect/internal/Types.scala | 27 +++++++--- test/files/neg/no-predef.check | 8 +-- test/files/neg/t2102.check | 4 +- test/files/neg/type-diagnostics.check | 4 +- 7 files changed, 71 insertions(+), 39 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index fde2f7bb03..d3c1f2b3f3 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/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) def isMask(s: ImportSelector) = s.name != nme.WILDCARD && s.rename == nme.WILDCARD diff --git a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala index 990edcd86d..50743a922a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala +++ b/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)) diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index b66dbf21c0..b628606897 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/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,51 @@ 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 "" 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) { + ";\n found : " + (foundWiden.nameAndArgsString + s" (in ${found.prefix.typeSymbol.fullNameString}) ") + explainAlias(found) + + "\n required: " + (reqWiden.nameAndArgsString + 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 +348,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 +439,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 ) // b) Failing that, the best we can do is append "(some other)" to the latter. diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index ad7e3ffe8f..b88ff4c2a2 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -964,6 +964,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. */ @@ -2309,6 +2311,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) @@ -2698,6 +2702,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 @@ -2716,7 +2733,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) && @@ -2726,17 +2743,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 } } diff --git a/test/files/neg/no-predef.check b/test/files/neg/no-predef.check index a63d8c5ba5..f5c2e82fe1 100644 --- a/test/files/neg/no-predef.check +++ b/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 diff --git a/test/files/neg/t2102.check b/test/files/neg/t2102.check index b4f91a5319..eef6945c2f 100644 --- a/test/files/neg/t2102.check +++ b/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 diff --git a/test/files/neg/type-diagnostics.check b/test/files/neg/type-diagnostics.check index c5e6dec3f8..fd327bcb66 100644 --- a/test/files/neg/type-diagnostics.check +++ b/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; -- cgit v1.2.3 From 466e52ba58604720b2dee35358d6b3545981b5b5 Mon Sep 17 00:00:00 2001 From: Edmund Noble Date: Tue, 7 Feb 2017 16:12:12 -0500 Subject: Match error lengths --- src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala | 7 +++++-- test/files/neg/t2102.check | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index b628606897..36b9a65334 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -308,8 +308,11 @@ trait TypeDiagnostics { !found.typeSymbol.isTypeParameterOrSkolem && !req.typeSymbol.isTypeParameterOrSkolem if (easilyMistakable) { - ";\n found : " + (foundWiden.nameAndArgsString + s" (in ${found.prefix.typeSymbol.fullNameString}) ") + explainAlias(found) + - "\n required: " + (reqWiden.nameAndArgsString + s" (in ${req.prefix.typeSymbol.fullNameString}) ") + explainAlias(req) + 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) + diff --git a/test/files/neg/t2102.check b/test/files/neg/t2102.check index eef6945c2f..6f70839d22 100644 --- a/test/files/neg/t2102.check +++ b/test/files/neg/t2102.check @@ -1,6 +1,6 @@ t2102.scala:2: error: type mismatch; found : Iterator[Int] (in java.util) - required: Iterator[_] (in scala.collection) + required: Iterator[_] (in scala.collection) val x: Iterator[_] = new java.util.ArrayList[Int]().iterator ^ one error found -- cgit v1.2.3