From aa559359bb55729913d34588462542f10c42e147 Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Wed, 21 Sep 2016 10:09:13 +0200 Subject: Refactor explanation interpolator --- src/dotty/tools/dotc/printing/Formatting.scala | 131 +++++++++++++-------- .../tools/dotc/printing/SyntaxHighlighting.scala | 2 +- src/dotty/tools/dotc/reporting/StoreReporter.scala | 1 - .../tools/dotc/reporting/diagnostic/messages.scala | 27 ++++- src/dotty/tools/dotc/typer/Applications.scala | 15 +-- src/dotty/tools/dotc/typer/ErrorReporting.scala | 11 +- src/dotty/tools/dotc/typer/RefChecks.scala | 2 +- test/test/TestREPL.scala | 4 +- tests/repl/errmsgs.check | 61 +++++----- tests/repl/imports.check | 8 +- 10 files changed, 154 insertions(+), 108 deletions(-) diff --git a/src/dotty/tools/dotc/printing/Formatting.scala b/src/dotty/tools/dotc/printing/Formatting.scala index b39d5683e..89f9eb814 100644 --- a/src/dotty/tools/dotc/printing/Formatting.scala +++ b/src/dotty/tools/dotc/printing/Formatting.scala @@ -9,6 +9,9 @@ import Decorators._ import scala.annotation.switch import scala.util.control.NonFatal import reporting.diagnostic.MessageContainer +import util.DiffUtil +import Highlighting.{ highlightToString => _, _ } +import SyntaxHighlighting._ object Formatting { @@ -113,65 +116,99 @@ object Formatting { seen.record(super.polyParamNameString(param), param) } - def explained2(op: Context => String)(implicit ctx: Context): String = { - val seen = new Seen - val explainCtx = ctx.printer match { - case dp: ExplainingPrinter => ctx // re-use outer printer and defer explanation to it - case _ => ctx.fresh.setPrinterFn(ctx => new ExplainingPrinter(seen)(ctx)) + def explanation(entry: Recorded)(implicit ctx: Context): String = { + def boundStr(bound: Type, default: ClassSymbol, cmp: String) = + if (bound.isRef(default)) "" else i"$cmp $bound" + + def boundsStr(bounds: TypeBounds): String = { + val lo = boundStr(bounds.lo, defn.NothingClass, ">:") + val hi = boundStr(bounds.hi, defn.AnyClass, "<:") + if (lo.isEmpty) hi + else if (hi.isEmpty) lo + else s"$lo and $hi" } - def explanation(entry: Recorded): String = { - def boundStr(bound: Type, default: ClassSymbol, cmp: String) = - if (bound.isRef(default)) "" else i"$cmp $bound" + def addendum(cat: String, info: Type): String = info match { + case bounds @ TypeBounds(lo, hi) if bounds ne TypeBounds.empty => + if (lo eq hi) i" which is an alias of $lo" + else i" with $cat ${boundsStr(bounds)}" + case _ => + "" + } - def boundsStr(bounds: TypeBounds): String = { - val lo = boundStr(bounds.lo, defn.NothingClass, ">:") - val hi = boundStr(bounds.hi, defn.AnyClass, "<:") - if (lo.isEmpty) hi - else if (hi.isEmpty) lo - else s"$lo and $hi" - } + entry match { + case param: PolyParam => + s"is a type variable${addendum("constraint", ctx.typeComparer.bounds(param))}" + case sym: Symbol => + s"is a ${ctx.printer.kindString(sym)}${sym.showExtendedLocation}${addendum("bounds", sym.info)}" + } + } - def addendum(cat: String, info: Type)(implicit ctx: Context): String = info match { - case bounds @ TypeBounds(lo, hi) if bounds ne TypeBounds.empty => - if (lo eq hi) i" which is an alias of $lo" - else i" with $cat ${boundsStr(bounds)}" - case _ => - "" - } + private def explanations(seen: Seen)(implicit ctx: Context): String = { + def needsExplanation(entry: Recorded) = entry match { + case param: PolyParam => ctx.typerState.constraint.contains(param) + case _ => false + } - entry match { - case param: PolyParam => - s"is a type variable${addendum("constraint", ctx.typeComparer.bounds(param))}" - case sym: Symbol => - s"is a ${ctx.printer.kindString(sym)}${sym.showExtendedLocation}${addendum("bounds", sym.info)}" + val toExplain: List[(String, Recorded)] = seen.toList.flatMap { + case (str, entry :: Nil) => + if (needsExplanation(entry)) (str, entry) :: Nil else Nil + case (str, entries) => + entries.map(alt => (seen.record(str, alt), alt)) + }.sortBy(_._1) + + def columnar(parts: List[(String, String)]): List[String] = { + lazy val maxLen = parts.map(_._1.length).max + parts.map { + case (leader, trailer) => + val variable = hl"$leader" + s"""$variable${" " * (maxLen - leader.length)} $trailer""" } } - def explanations(seen: Seen)(implicit ctx: Context): String = { - def needsExplanation(entry: Recorded) = entry match { - case param: PolyParam => ctx.typerState.constraint.contains(param) - case _ => false + val explainParts = toExplain.map { case (str, entry) => (str, explanation(entry)) } + val explainLines = columnar(explainParts) + if (explainLines.isEmpty) "" else i"$explainLines%\n%\n" + } + + private def explainCtx(seen: Seen)(implicit ctx: Context): Context = ctx.printer match { + case dp: ExplainingPrinter => + ctx // re-use outer printer and defer explanation to it + case _ => ctx.fresh.setPrinterFn(ctx => new ExplainingPrinter(seen)(ctx)) + } + + def explained2(op: Context => String)(implicit ctx: Context): String = { + val seen = new Seen + op(explainCtx(seen)) ++ explanations(seen) + } + + def disambiguateTypes(args: Type*)(implicit ctx: Context): String = { + val seen = new Seen + object polyparams extends TypeTraverser { + def traverse(tp: Type): Unit = tp match { + case tp: TypeRef => + seen.record(tp.show(explainCtx(seen)), tp.symbol) + traverseChildren(tp) + case tp: TermRef => + seen.record(tp.show(explainCtx(seen)), tp.symbol) + traverseChildren(tp) + case tp: PolyParam => + seen.record(tp.show(explainCtx(seen)), tp) + traverseChildren(tp) + case _ => + traverseChildren(tp) } - val toExplain: List[(String, Recorded)] = seen.toList.flatMap { - case (str, entry :: Nil) => - if (needsExplanation(entry)) (str, entry) :: Nil else Nil - case (str, entries) => - entries.map(alt => (seen.record(str, alt), alt)) - }.sortBy(_._1) - val explainParts = toExplain.map { case (str, entry) => (str, explanation(entry)) } - val explainLines = columnar(explainParts, " ") - if (explainLines.isEmpty) "" else i"\n\nwhere $explainLines%\n %\n" } - - op(explainCtx) ++ explanations(seen) + args.foreach(polyparams.traverse) + explanations(seen) } - def columnar(parts: List[(String, String)], sep: String): List[String] = { - lazy val maxLen = parts.map(_._1.length).max - parts.map { - case (leader, trailer) => - s"$leader${" " * (maxLen - leader.length)}$sep$trailer" + def typeDiff(found: Type, expected: Type)(implicit ctx: Context): (String, String) = { + (found, expected) match { + case (rf1: RefinedType, rf2: RefinedType) => + DiffUtil.mkColoredTypeDiff(rf1.show, rf2.show) + case _ => + (hl"${found.show}", hl"${expected.show}") } } } diff --git a/src/dotty/tools/dotc/printing/SyntaxHighlighting.scala b/src/dotty/tools/dotc/printing/SyntaxHighlighting.scala index 80170dcad..7d4d12394 100644 --- a/src/dotty/tools/dotc/printing/SyntaxHighlighting.scala +++ b/src/dotty/tools/dotc/printing/SyntaxHighlighting.scala @@ -278,7 +278,7 @@ object SyntaxHighlighting { val toAdd = if (shouldHL(str)) highlight(str) - else if (lastToken == "val" || lastToken == "def" || lastToken == "case") + else if (("var" :: "val" :: "def" :: "case" :: Nil).contains(lastToken)) valDef(str) else str val suffix = if (delim(curr)) s"$curr" else "" diff --git a/src/dotty/tools/dotc/reporting/StoreReporter.scala b/src/dotty/tools/dotc/reporting/StoreReporter.scala index 3744a35dd..e85017ed2 100644 --- a/src/dotty/tools/dotc/reporting/StoreReporter.scala +++ b/src/dotty/tools/dotc/reporting/StoreReporter.scala @@ -4,7 +4,6 @@ package reporting import core.Contexts.Context import collection.mutable -import Reporter.{Error, Warning} import config.Printers.typr import diagnostic.MessageContainer import diagnostic.messages._ diff --git a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index cf67e8989..009636593 100644 --- a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -4,12 +4,13 @@ package reporting package diagnostic import dotc.core._ -import Contexts.Context, Decorators._, Symbols._, Names._ +import Contexts.Context, Decorators._, Symbols._, Names._, Types._ import util.{SourceFile, NoSource} import util.{SourcePosition, NoSourcePosition} import config.Settings.Setting import interfaces.Diagnostic.{ERROR, WARNING, INFO} -import dotc.printing.SyntaxHighlighting._ +import printing.SyntaxHighlighting._ +import printing.Formatting object messages { @@ -127,7 +128,7 @@ object messages { } } - class EmptyCatchBlock(tryBody: untpd.Tree)(implicit ctx: Context) + case class EmptyCatchBlock(tryBody: untpd.Tree)(implicit ctx: Context) extends EmptyCatchOrFinallyBlock(tryBody, "E001") { val kind = "Syntax" val msg = @@ -174,7 +175,7 @@ object messages { } /** Type Errors ----------------------------------------------------------- */ - class DuplicateBind(bind: untpd.Bind, tree: untpd.CaseDef)(implicit ctx: Context) + case class DuplicateBind(bind: untpd.Bind, tree: untpd.CaseDef)(implicit ctx: Context) extends Message("E004") { val kind = "Naming" val msg = em"duplicate pattern variable: `${bind.name}`" @@ -201,9 +202,9 @@ object messages { } } - class MissingIdent(tree: untpd.Ident, treeKind: String, name: Name)(implicit ctx: Context) + case class MissingIdent(tree: untpd.Ident, treeKind: String, name: Name)(implicit ctx: Context) extends Message("E005") { - val kind = "Missing identifier" + val kind = "Missing Identifier" val msg = em"not found: $treeKind$name" val explanation = { @@ -211,4 +212,18 @@ object messages { |has either been misspelt or you're forgetting an import""".stripMargin } } + + case class TypeMismatch(found: Type, expected: Type, whyNoMatch: String = "")(implicit ctx: Context) + extends Message("E006") { + val kind = "Type Mismatch" + private val where = Formatting.disambiguateTypes(found, expected) + private val (fnd, exp) = Formatting.typeDiff(found, expected) + val msg = + s"""|found: $fnd + |required: $exp + | + |$where""".stripMargin + whyNoMatch + + val explanation = "" + } } diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index 2c9039db1..56595a637 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -27,6 +27,7 @@ import collection.mutable import config.Printers.{typr, unapp, overload} import TypeApplications._ import language.implicitConversions +import reporting.diagnostic.Message object Applications { import tpd._ @@ -132,10 +133,10 @@ trait Applications extends Compatibility { self: Typer with Dynamic => protected def harmonizeArgs(args: List[TypedArg]): List[TypedArg] /** Signal failure with given message at position of given argument */ - protected def fail(msg: => String, arg: Arg): Unit + protected def fail(msg: => Message, arg: Arg): Unit /** Signal failure with given message at position of the application itself */ - protected def fail(msg: => String): Unit + protected def fail(msg: => Message): Unit protected def appPos: Position @@ -186,7 +187,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic => // it might be healed by an implicit conversion assert(ctx.typerState.constraint eq savedConstraint) else - fail(err.typeMismatchStr(methType.resultType, resultType)) + fail(err.typeMismatchMsg(methType.resultType, resultType)) } // match all arguments with corresponding formal parameters matchArgs(orderedArgs, methType.paramTypes, 0) @@ -388,9 +389,9 @@ trait Applications extends Compatibility { self: Typer with Dynamic => def addArg(arg: TypedArg, formal: Type) = ok = ok & isCompatible(argType(arg, formal), formal) def makeVarArg(n: Int, elemFormal: Type) = {} - def fail(msg: => String, arg: Arg) = + def fail(msg: => Message, arg: Arg) = ok = false - def fail(msg: => String) = + def fail(msg: => Message) = ok = false def appPos = NoPosition lazy val normalizedFun = ref(methRef) @@ -455,12 +456,12 @@ trait Applications extends Compatibility { self: Typer with Dynamic => override def appPos = app.pos - def fail(msg: => String, arg: Trees.Tree[T]) = { + def fail(msg: => Message, arg: Trees.Tree[T]) = { ctx.error(msg, arg.pos) ok = false } - def fail(msg: => String) = { + def fail(msg: => Message) = { ctx.error(msg, app.pos) ok = false } diff --git a/src/dotty/tools/dotc/typer/ErrorReporting.scala b/src/dotty/tools/dotc/typer/ErrorReporting.scala index 8b740c3dc..1fd4fc96e 100644 --- a/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -12,12 +12,13 @@ import printing.{Showable, RefinedPrinter} import scala.collection.mutable import java.util.regex.Matcher.quoteReplacement import reporting.diagnostic.Message +import reporting.diagnostic.messages._ object ErrorReporting { import tpd._ - def errorTree(tree: untpd.Tree, msg: => String)(implicit ctx: Context): tpd.Tree = + def errorTree(tree: untpd.Tree, msg: => Message)(implicit ctx: Context): tpd.Tree = tree withType errorType(msg, tree.pos) def errorType(msg: => Message, pos: Position)(implicit ctx: Context): ErrorType = { @@ -101,7 +102,7 @@ object ErrorReporting { def patternConstrStr(tree: Tree): String = ??? def typeMismatch(tree: Tree, pt: Type, implicitFailure: SearchFailure = NoImplicitMatches): Tree = - errorTree(tree, typeMismatchStr(normalize(tree.tpe, pt), pt) + implicitFailure.postscript) + 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) = @@ -110,7 +111,7 @@ object ErrorReporting { else "" - def typeMismatchStr(found: Type, expected: Type) = { + def typeMismatchMsg(found: Type, expected: Type) = { // replace constrained polyparams and their typevars by their bounds where possible object reported extends TypeMap { def setVariance(v: Int) = variance = v @@ -132,9 +133,7 @@ object ErrorReporting { val found1 = reported(found) reported.setVariance(-1) val expected1 = reported(expected) - ex"""|type mismatch: - |found: $found1 - |required: $expected1""".stripMargin + whyNoMatchStr(found, expected) + TypeMismatch(found1, expected1, whyNoMatchStr(found, expected)) } /** Format `raw` implicitNotFound argument, replacing all diff --git a/src/dotty/tools/dotc/typer/RefChecks.scala b/src/dotty/tools/dotc/typer/RefChecks.scala index 1f150c519..4d82a2d12 100644 --- a/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/src/dotty/tools/dotc/typer/RefChecks.scala @@ -200,7 +200,7 @@ object RefChecks { infoStringWithLocation(other), infoStringWithLocation(member)) else if (ctx.settings.debug.value) - err.typeMismatchStr(memberTp, otherTp) + err.typeMismatchMsg(memberTp, otherTp) else "" "overriding %s;\n %s %s%s".format( diff --git a/test/test/TestREPL.scala b/test/test/TestREPL.scala index 090c774bb..9867cb4ec 100644 --- a/test/test/TestREPL.scala +++ b/test/test/TestREPL.scala @@ -21,9 +21,7 @@ class TestREPL(script: String) extends REPL { override val output = new NewLinePrintWriter(out) override def context(ctx: Context) = - ctx.fresh - .setSetting(ctx.settings.color, "never") - .setSetting(ctx.settings.pageWidth, 80) + ctx.fresh.setSetting(ctx.settings.color, "never") override def input(in: Interpreter)(implicit ctx: Context) = new InteractiveReader { val lines = script.lines.buffered diff --git a/tests/repl/errmsgs.check b/tests/repl/errmsgs.check index d8c1b9215..8f86aac08 100644 --- a/tests/repl/errmsgs.check +++ b/tests/repl/errmsgs.check @@ -1,48 +1,47 @@ scala> class Inv[T](x: T) defined class Inv scala> val x: List[String] = List(1) --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 3:val x: List[String] = List(1) ^ - type mismatch: found: Int(1) required: String + scala> val y: List[List[String]] = List(List(1)) --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 3:val y: List[List[String]] = List(List(1)) ^ - type mismatch: found: Int(1) required: String + scala> val z: (List[String], List[Int]) = (List(1), List("a")) --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 3:val z: (List[String], List[Int]) = (List(1), List("a")) ^ - type mismatch: found: Int(1) required: String --- Error: ------------------------------------------------------------ + +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 3:val z: (List[String], List[Int]) = (List(1), List("a")) - ^ - type mismatch: + ^^^ found: String("a") required: Int + scala> val a: Inv[String] = new Inv(new Inv(1)) --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 4:val a: Inv[String] = new Inv(new Inv(1)) - ^^^^ - type mismatch: - found: Inv[T] - required: String - - where T is a type variable with constraint >: Int(1) + ^^^^^ + found: Inv[T] + required: String + + T is a type variable with constraint >: Int(1) scala> val b: Inv[String] = new Inv(1) --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 4:val b: Inv[String] = new Inv(1) ^ - type mismatch: found: Int(1) required: String + scala> abstract class C { type T val x: T @@ -58,22 +57,20 @@ scala> abstract class C { } } } --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 8: var y: T = x ^ - type mismatch: - found: C.this.T(C.this.x) - required: T' - - where T is a type in class C - T' is a type in the initalizer of value s which is an alias of String --- Error: ------------------------------------------------------------ + found: C.this.T(C.this.x) + required: T + + T is a type in class C + T' is a type in the initalizer of value s which is an alias of String +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 12: val z: T = y ^ -type mismatch: -found: T(y) -required: T' - -where T is a type in the initalizer of value s which is an alias of String - T' is a type in method f which is an alias of Int + found: T(y) + required: T + + T is a type in the initalizer of value s which is an alias of String + T' is a type in method f which is an alias of Int scala> :quit diff --git a/tests/repl/imports.check b/tests/repl/imports.check index f9e48c011..2e8d5dcf9 100644 --- a/tests/repl/imports.check +++ b/tests/repl/imports.check @@ -7,18 +7,18 @@ defined module o scala> import o._ import o._ scala> buf += xs --- Error: ------------------------------------------------------------ +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 10:buf += xs ^^ - type mismatch: found: scala.collection.immutable.List[Int](o.xs) required: String --- Error: ------------------------------------------------------------ + +-- [E006] Type Mismatch Error: ------------------------------------------------------------------------------- 10:buf += xs ^^^^^^^^^ - type mismatch: found: String required: scala.collection.mutable.ListBuffer[Int] + scala> buf ++= xs res1: scala.collection.mutable.ListBuffer[Int] = ListBuffer(1, 2, 3) scala> :quit -- cgit v1.2.3