Skip to content

Fix #1747: Improve error message for Scala/Java type mismatch #1984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import scala.annotation.{ switch, meta }
import scala.collection.{ mutable, immutable }
import PartialFunction._
import collection.mutable
import util.common.alwaysZero
import scala.reflect.api.{ Universe => ApiUniverse }

object Definitions {
Expand Down Expand Up @@ -152,7 +153,7 @@ class Definitions {
resultTypeFn: PolyType => Type, flags: FlagSet = EmptyFlags) = {
val tparamNames = tpnme.syntheticTypeParamNames(typeParamCount)
val tparamBounds = tparamNames map (_ => TypeBounds.empty)
val ptype = PolyType(tparamNames)(_ => tparamBounds, resultTypeFn)
val ptype = PolyType(tparamNames, tparamNames.map(alwaysZero))(_ => tparamBounds, resultTypeFn)
enterMethod(cls, name, ptype, flags)
}

Expand Down
7 changes: 3 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2603,7 +2603,7 @@ object Types {
case t => mapOver(t)
}
}
PolyType(paramNames ++ that.paramNames)(
PolyType(paramNames ++ that.paramNames, variances ++ that.variances)(
x => this.paramBounds.mapConserve(_.subst(this, x).bounds) ++
that.paramBounds.mapConserve(shift(_).subst(that, x).bounds),
x => shift(that.resultType).subst(that, x).subst(this, x))
Expand Down Expand Up @@ -2634,11 +2634,10 @@ object Types {
}

object PolyType {
def apply(paramNames: List[TypeName], variances: List[Int] = Nil)(
def apply(paramNames: List[TypeName], variances: List[Int])(
paramBoundsExp: PolyType => List[TypeBounds],
resultTypeExp: PolyType => Type)(implicit ctx: Context): PolyType = {
val vs = if (variances.isEmpty) paramNames.map(alwaysZero) else variances
unique(new PolyType(paramNames, vs)(paramBoundsExp, resultTypeExp))
unique(new PolyType(paramNames, variances)(paramBoundsExp, resultTypeExp))
}

def unapply(tl: PolyType): Some[(List[LambdaParam], Type)] =
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ class PlainPrinter(_ctx: Context) extends Printer {
varianceString(variance) ~ name.toString ~ toText(bounds)
changePrec(GlobalPrec) {
"[" ~ Text((tp.variances, tp.paramNames, tp.paramBounds).zipped.map(paramText), ", ") ~
"] => " ~ toTextGlobal(tp.resultType)
"]" ~ (" => " provided !tp.resultType.isInstanceOf[MethodType]) ~
toTextGlobal(tp.resultType)
}
case tp: PolyParam =>
polyParamNameString(tp) ~ polyHash(tp.binder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ trait FullParameterization {
}
val ctparams = if (abstractOverClass) clazz.typeParams else Nil
val ctnames = ctparams.map(_.name.unexpandedName)
val ctvariances = ctparams.map(_.variance)

/** The method result type */
def resultType(mapClassParams: Type => Type) = {
Expand All @@ -122,14 +123,14 @@ trait FullParameterization {

info match {
case info: PolyType =>
PolyType(info.paramNames ++ ctnames)(
PolyType(info.paramNames ++ ctnames, info.variances ++ ctvariances)(
pt =>
(info.paramBounds.map(mapClassParams(_, pt).bounds) ++
mappedClassBounds(pt)).mapConserve(_.subst(info, pt).bounds),
pt => resultType(mapClassParams(_, pt)).subst(info, pt))
case _ =>
if (ctparams.isEmpty) resultType(identity)
else PolyType(ctnames)(mappedClassBounds, pt => resultType(mapClassParams(_, pt)))
else PolyType(ctnames, ctvariances)(mappedClassBounds, pt => resultType(mapClassParams(_, pt)))
}
}

Expand Down
16 changes: 14 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,23 @@ object ErrorReporting {
errorTree(tree, typeMismatchMsg(normalize(tree.tpe, pt), pt, implicitFailure.postscript))

/** A subtype log explaining why `found` does not conform to `expected` */
def whyNoMatchStr(found: Type, expected: Type) =
if (ctx.settings.explaintypes.value)
def whyNoMatchStr(found: Type, expected: Type) = {
def dropJavaMethod(tp: Type): Type = tp match {
case tp: PolyType =>
tp.derivedPolyType(resType = dropJavaMethod(tp.resultType))
case tp: JavaMethodType =>
MethodType(tp.paramNames, tp.paramTypes, dropJavaMethod(tp.resultType))
case tp => tp
}
val found1 = dropJavaMethod(found)
val expected1 = dropJavaMethod(expected)
if ((found1 eq found) != (expected eq expected1) && (found1 <:< expected1))
"\n (Note that Scala's and Java's representation of this type differs)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not be enough for the user to understand the issue. In the example we would get

overriding method toArray in trait Collection of type [T](x$0: Array[T])Array[T];
       method toArray of type [T](a: Array[T])Array[T] has incompatible type
       (Note that Scala's and Java's representation of this type differs)

Would it be possible to hint that to make the representation match one would need to write
def toArray[T](a: Array[T with Object]): Array[T with Object]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but it is really tricky and would require quite a bit of code. In short interoperating with Java using arrays requires a lot of context knowledge. Hard to condense that in an error message.

else if (ctx.settings.explaintypes.value)
"\n" + ctx.typerState.show + "\n" + TypeComparer.explained((found <:< expected)(_))
else
""
}

def typeMismatchMsg(found: Type, expected: Type, postScript: String = "") = {
// replace constrained polyparams and their typevars by their bounds where possible
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ object Implicits {
}

/** The result of an implicit search */
abstract class SearchResult extends Showable {
sealed abstract class SearchResult extends Showable {
def toText(printer: Printer): Text = printer.toText(this)
}

Expand Down
3 changes: 3 additions & 0 deletions tests/neg/i1747.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Coll[E] extends java.util.Collection[E] { // error: needs to be abstract
def toArray[T](a: Array[T]): Array[T] = ??? // error: cannot override
}