From 63068a96c759a43d348c39b52c44fd1b3c60b825 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 2 Aug 2022 15:40:59 +0100 Subject: [PATCH] Fix aspects of the Show setup 1. Fix accidentally recursive ParamInfo instance, and test 2. Add a missing Integer instance, needed for VarianceMap, and test 3. Fix showing nested non-Showable, non-primitive values, for instance showing a value of type `List[(Type, Int)]` wasn't pretty-printing the Type. To fix this we need to introduce CtxShow, to defer the showing until we can wire a Context to it. And test. Additionally, we can do the non-sensical tagging with a Printer, like explainations does. 4. Addtionally, that ^ change brings the em/ex features to nested values too, and add tests for those. 5. Add some notes on how MessageLimiter works 6. Fix OrderingContraint assuming ctx.run is non null. --- .../dotty/tools/dotc/core/Decorators.scala | 4 +- .../tools/dotc/core/OrderingConstraint.scala | 2 +- .../tools/dotc/printing/Formatting.scala | 69 +++++++------ .../tools/dotc/printing/MessageLimiter.scala | 6 ++ .../tools/dotc/StringFormatterTest.scala | 96 +++++++++++++++++++ .../tools/dotc/printing/PrinterTests.scala | 41 +------- 6 files changed, 152 insertions(+), 66 deletions(-) create mode 100644 compiler/test/dotty/tools/dotc/StringFormatterTest.scala diff --git a/compiler/src/dotty/tools/dotc/core/Decorators.scala b/compiler/src/dotty/tools/dotc/core/Decorators.scala index 93a0a38ffc57..59440d1cb965 100644 --- a/compiler/src/dotty/tools/dotc/core/Decorators.scala +++ b/compiler/src/dotty/tools/dotc/core/Decorators.scala @@ -293,13 +293,13 @@ object Decorators { * error messages after the first one if some of their arguments are "non-sensical". */ def em(args: Shown*)(using Context): String = - new ErrorMessageFormatter(sc).assemble(args) + forErrorMessages(new StringFormatter(sc).assemble(args)) /** Formatting with added explanations: Like `em`, but add explanations to * give more info about type variables and to disambiguate where needed. */ def ex(args: Shown*)(using Context): String = - explained(em(args: _*)) + explained(new StringFormatter(sc).assemble(args)) extension [T <: AnyRef](arr: Array[T]) def binarySearch(x: T | Null): Int = java.util.Arrays.binarySearch(arr.asInstanceOf[Array[Object | Null]], x) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index a304fa9f5705..1a1e44295d9e 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -29,7 +29,7 @@ object OrderingConstraint { empty else val result = new OrderingConstraint(boundsMap, lowerMap, upperMap) - ctx.run.nn.recordConstraintSize(result, result.boundsMap.size) + if ctx.run != null then ctx.run.nn.recordConstraintSize(result, result.boundsMap.size) result /** A lens for updating a single entry array in one of the three constraint maps */ diff --git a/compiler/src/dotty/tools/dotc/printing/Formatting.scala b/compiler/src/dotty/tools/dotc/printing/Formatting.scala index 2c3d4f655ca8..348390d9c7e2 100644 --- a/compiler/src/dotty/tools/dotc/printing/Formatting.scala +++ b/compiler/src/dotty/tools/dotc/printing/Formatting.scala @@ -18,9 +18,8 @@ object Formatting { object ShownDef: /** Represents a value that has been "shown" and can be consumed by StringFormatter. * Not just a string because it may be a Seq that StringFormatter will intersperse with the trailing separator. - * Also, it's not a `String | Seq[String]` because then we'd need a Context to call `Showable#show`. We could - * make Context a requirement for a Show instance but then we'd have lots of instances instead of just one ShowAny - * instance. We could also try to make `Show#show` require the Context, but then that breaks the Conversion. */ + * It may also be a CtxShow, which allows the Show instance to finish showing the value with the string + * interpolator's correct context, that is with non-sensical tagging, message limiting, explanations, etc. */ opaque type Shown = Any object Shown: given [A: Show]: Conversion[A, Shown] = Show[A].show(_) @@ -29,6 +28,14 @@ object Formatting { /** Show a value T by returning a "shown" result. */ def show(x: T): Shown + trait CtxShow: + def run(using Context): Shown + + extension (s: Shown) + def ctxShow(using Context): Shown = s match + case cs: CtxShow => cs.run + case _ => s + /** The base implementation, passing the argument to StringFormatter which will try to `.show` it. */ object ShowAny extends Show[Any]: def show(x: Any): Shown = x @@ -37,11 +44,7 @@ object Formatting { given Show[Product] = ShowAny class ShowImplicits2 extends ShowImplicits3: - given Show[ParamInfo] with - def show(x: ParamInfo) = x match - case x: Symbol => Show[x.type].show(x) - case x: LambdaParam => Show[x.type].show(x) - case _ => ShowAny + given Show[ParamInfo] = ShowAny class ShowImplicits1 extends ShowImplicits2: given Show[ImplicitRef] = ShowAny @@ -52,10 +55,12 @@ object Formatting { inline def apply[A](using inline z: Show[A]): Show[A] = z given [X: Show]: Show[Seq[X]] with - def show(x: Seq[X]) = x.map(Show[X].show) + def show(x: Seq[X]) = new CtxShow: + def run(using Context) = x.map(show1) given [A: Show, B: Show]: Show[(A, B)] with - def show(x: (A, B)) = (Show[A].show(x._1), Show[B].show(x._2)) + def show(x: (A, B)) = new CtxShow: + def run(using Context) = (show1(x._1), show1(x._2)) given [X: Show]: Show[X | Null] with def show(x: X | Null) = if x == null then "null" else Show[X].show(x.nn) @@ -71,6 +76,7 @@ object Formatting { given Show[Int] = ShowAny given Show[Char] = ShowAny given Show[Boolean] = ShowAny + given Show[Integer] = ShowAny given Show[String] = ShowAny given Show[Class[?]] = ShowAny given Show[Throwable] = ShowAny @@ -84,6 +90,11 @@ object Formatting { given Show[util.SourceFile] = ShowAny given Show[util.Spans.Span] = ShowAny given Show[tasty.TreeUnpickler#OwnerTree] = ShowAny + + private def show1[A: Show](x: A)(using Context) = show2(Show[A].show(x).ctxShow) + private def show2(x: Shown)(using Context): String = x match + case seq: Seq[?] => seq.map(show2).mkString("[", ", ", "]") + case res => res.tryToShow end Show end ShownDef export ShownDef.{ Show, Shown } @@ -100,15 +111,14 @@ object Formatting { class StringFormatter(protected val sc: StringContext) { protected def showArg(arg: Any)(using Context): String = arg.tryToShow - private def treatArg(arg: Shown, suffix: String)(using Context): (Any, String) = arg match { - case arg: Seq[?] if suffix.nonEmpty && suffix.head == '%' => - val (rawsep, rest) = suffix.tail.span(_ != '%') - val sep = StringContext.processEscapes(rawsep) - if (rest.nonEmpty) (arg.map(showArg).mkString(sep), rest.tail) - else (arg, suffix) + private def treatArg(arg: Shown, suffix: String)(using Context): (String, String) = arg.ctxShow match { + case arg: Seq[?] if suffix.indexOf('%') == 0 && suffix.indexOf('%', 1) != -1 => + val end = suffix.indexOf('%', 1) + val sep = StringContext.processEscapes(suffix.substring(1, end)) + (arg.mkString(sep), suffix.substring(end + 1)) case arg: Seq[?] => (arg.map(showArg).mkString("[", ", ", "]"), suffix) - case _ => + case arg => (showArg(arg), suffix) } @@ -134,11 +144,13 @@ object Formatting { * like concatenation, stripMargin etc on the values returned by em"...", and in the current error * message composition methods, this is crucial. */ - class ErrorMessageFormatter(sc: StringContext) extends StringFormatter(sc): - override protected def showArg(arg: Any)(using Context): String = - wrapNonSensical(arg, super.showArg(arg)(using errorMessageCtx)) + def forErrorMessages(op: Context ?=> String)(using Context): String = op(using errorMessageCtx) + + private class ErrorMessagePrinter(_ctx: Context) extends RefinedPrinter(_ctx): + override def toText(tp: Type): Text = wrapNonSensical(tp, super.toText(tp)) + override def toText(sym: Symbol): Text = wrapNonSensical(sym, super.toText(sym)) - private def wrapNonSensical(arg: Any, str: String)(using Context): String = { + private def wrapNonSensical(arg: Any, text: Text)(using Context): Text = { import Message._ def isSensical(arg: Any): Boolean = arg match { case tpe: Type => @@ -151,8 +163,8 @@ object Formatting { case _ => true } - if (isSensical(arg)) str - else nonSensicalStartTag + str + nonSensicalEndTag + if (isSensical(arg)) text + else nonSensicalStartTag ~ text ~ nonSensicalEndTag } private type Recorded = Symbol | ParamRef | SkolemType @@ -203,7 +215,7 @@ object Formatting { } } - private class ExplainingPrinter(seen: Seen)(_ctx: Context) extends RefinedPrinter(_ctx) { + private class ExplainingPrinter(seen: Seen)(_ctx: Context) extends ErrorMessagePrinter(_ctx) { /** True if printer should a source module instead of its module class */ private def useSourceModule(sym: Symbol): Boolean = @@ -307,9 +319,12 @@ object Formatting { } private def errorMessageCtx(using Context): Context = - ctx.property(MessageLimiter) match + val ctx1 = ctx.property(MessageLimiter) match case Some(_: ErrorMessageLimiter) => ctx case _ => ctx.fresh.setProperty(MessageLimiter, ErrorMessageLimiter()) + ctx1.printer match + case _: ErrorMessagePrinter => ctx1 + case _ => ctx1.fresh.setPrinterFn(ctx => ErrorMessagePrinter(ctx)) /** Context with correct printer set for explanations */ private def explainCtx(seen: Seen)(using Context): Context = @@ -364,8 +379,8 @@ object Formatting { * highlight the difference */ def typeDiff(found: Type, expected: Type)(using Context): (String, String) = { - val fnd = wrapNonSensical(found, found.show) - val exp = wrapNonSensical(expected, expected.show) + val fnd = wrapNonSensical(found, found.toText(ctx.printer)).show + val exp = wrapNonSensical(expected, expected.toText(ctx.printer)).show DiffUtil.mkColoredTypeDiff(fnd, exp) match { case _ if ctx.settings.color.value == "never" => (fnd, exp) diff --git a/compiler/src/dotty/tools/dotc/printing/MessageLimiter.scala b/compiler/src/dotty/tools/dotc/printing/MessageLimiter.scala index 0fcd6a800c9b..c9ac4a5af4ce 100644 --- a/compiler/src/dotty/tools/dotc/printing/MessageLimiter.scala +++ b/compiler/src/dotty/tools/dotc/printing/MessageLimiter.scala @@ -50,6 +50,12 @@ class ErrorMessageLimiter extends MessageLimiter: override def recurseLimit = val freeFraction: Double = ((sizeLimit - textLength) max 0).toDouble / sizeLimit + // 10'000 - 0 / 10'0000 = 100% free + // 10'000 - 200 / 10'0000 = 98% free * 50 = 49 + // 10'000 - 1'000 / 10'0000 = 90% free * 50 = 45 + // 10'000 - 2'000 / 10'0000 = 80% free * 50 = 40 + // every 200 characters consumes a "recurseCount" + // which, additionally, is lowered from 100 to 50 here (initialRecurseLimit * freeFraction).toInt diff --git a/compiler/test/dotty/tools/dotc/StringFormatterTest.scala b/compiler/test/dotty/tools/dotc/StringFormatterTest.scala new file mode 100644 index 000000000000..7df64ad5bf3f --- /dev/null +++ b/compiler/test/dotty/tools/dotc/StringFormatterTest.scala @@ -0,0 +1,96 @@ +package dotty.tools +package dotc + +import core.*, Contexts.*, Decorators.*, Denotations.*, Flags.*, Names.*, StdNames.*, SymDenotations.*, Symbols.*, Types.* +import config.Printers.* +import printing.Formatting.Show + +import org.junit.Test +import org.junit.Assert.* + +class StringFormatterTest extends AbstractStringFormatterTest: + @Test def string = check("foo", i"${"foo"}") + @Test def integer = check("1", i"${Int.box(1)}") + @Test def type1 = check("Any", i"${defn.AnyType}") + @Test def symbol = check("class Any", i"${defn.AnyClass}") + @Test def paramInfo = check("class Any", i"${defn.AnyClass: ParamInfo}") + @Test def seq = check("[Any, String]", i"${Seq(defn.AnyType, defn.StringType)}") + @Test def seqSep = check("Any; String", i"${Seq(defn.AnyType, defn.StringType)}%; %") + @Test def tuple = check("(1,Any)", i"${(1, defn.AnyType)}") + @Test def seqOfTup = check("(1,Any), (2,String)", i"${Seq(1 -> defn.AnyType, 2 -> defn.StringType)}%, %") + @Test def flags1 = check("final", i"$Final") + @Test def flagsSeq = check(", final", i"${Seq(JavaStatic, Final)}%, %") + @Test def flagsTup = check("(,final)", i"${(JavaStatic, Final)}") + @Test def seqOfTup2 = check("(final,given), (private,lazy)", i"${Seq((Final, Given), (Private, Lazy))}%, %") + + class StorePrinter extends Printer: + var string: String = "" + override def println(msg: => String) = string = msg + + @Test def testShowing: Unit = + val store = StorePrinter() + (JavaStatic | Final).showing(i"flags=$result", store) + assertEquals("flags=final ", store.string) + + @Test def testShowingWithOriginalType: Unit = + val store = StorePrinter() + (JavaStatic | Final).showing(i"flags=${if result.is(Private) then result &~ Private else result | Private}", store) + assertEquals("flags=private final ", store.string) +end StringFormatterTest + +class EmStringFormatterTest extends AbstractStringFormatterTest: + @Test def seq = check("[Any, String]", em"${Seq(defn.AnyType, defn.StringType)}") + @Test def seqSeq = check("Any; String", em"${Seq(defn.AnyType, defn.StringType)}%; %") + @Test def ellipsis = assert(em"$Big".contains("...")) + @Test def err = check("type Err", em"$Err") + @Test def ambig = check("Foo vs Foo", em"$Foo vs $Foo") + @Test def cstrd = check("Foo; Bar", em"$mkCstrd%; %") + @Test def seqErr = check("[class Any, type Err]", em"${Seq(defn.AnyClass, Err)}") + @Test def seqSeqErr = check("class Any; type Err", em"${Seq(defn.AnyClass, Err)}%; %") + @Test def tupleErr = check("(1,type Err)", em"${(1, Err)}") + @Test def tupleAmb = check("(Foo,Foo)", em"${(Foo, Foo)}") + @Test def tupleFlags = check("(Foo,abstract)", em"${(Foo, Abstract)}") + @Test def seqOfTupleFlags = check("[(Foo,abstract)]", em"${Seq((Foo, Abstract))}") +end EmStringFormatterTest + +class ExStringFormatterTest extends AbstractStringFormatterTest: + @Test def seq = check("[Any, String]", ex"${Seq(defn.AnyType, defn.StringType)}") + @Test def seqSeq = check("Any; String", ex"${Seq(defn.AnyType, defn.StringType)}%; %") + @Test def ellipsis = assert(ex"$Big".contains("...")) + @Test def err = check("type Err", ex"$Err") + @Test def ambig = check("""Foo vs Foo² + | + |where: Foo is a type + | Foo² is a type + |""".stripMargin, ex"$Foo vs $Foo") + @Test def cstrd = check("""Foo; Bar + | + |where: Bar is a type variable with constraint <: String + | Foo is a type variable with constraint <: Int + |""".stripMargin, ex"$mkCstrd%; %") + @Test def seqErr = check("[class Any, type Err]", ex"${Seq(defn.AnyClass, Err)}") + @Test def seqSeqErr = check("class Any; type Err", ex"${Seq(defn.AnyClass, Err)}%; %") + @Test def tupleErr = check("(1,type Err)", ex"${(1, Err)}") + @Test def tupleAmb = check("""(Foo,Foo²) + | + |where: Foo is a type + | Foo² is a type + |""".stripMargin, ex"${(Foo, Foo)}") +end ExStringFormatterTest + +abstract class AbstractStringFormatterTest extends DottyTest: + override def initializeCtx(fc: FreshContext) = super.initializeCtx(fc.setSetting(fc.settings.color, "never")) + + def Foo = newSymbol(defn.RootClass, typeName("Foo"), EmptyFlags, TypeBounds.empty).typeRef + def Err = newErrorSymbol(defn.RootClass, typeName("Err"), "") + def Big = (1 to 120).foldLeft(defn.StringType)((tp, i) => RefinedType(tp, typeName("A" * 69 + i), TypeAlias(defn.IntType))) + + def mkCstrd = + val names = List(typeName("Foo"), typeName("Bar")) + val infos = List(TypeBounds.upper(defn.IntType), TypeBounds.upper(defn.StringType)) + val tl = PolyType(names)(_ => infos, _ => defn.AnyType) + TypeComparer.addToConstraint(tl, Nil) + tl.paramRefs + + def ckSub(obtained: String, snippet: String) = assert(obtained.contains(snippet)) + def check(expected: String, obtained: String) = assertEquals(expected, obtained) diff --git a/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala b/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala index 64049453b595..f46e56fdb18e 100644 --- a/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala +++ b/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala @@ -2,23 +2,14 @@ package dotty.tools package dotc package printing -import ast.{ Trees, tpd } -import core.Names._ -import core.Symbols._ -import core.Decorators._ -import dotty.tools.dotc.core.Contexts.Context +import core.*, Contexts.*, Decorators.*, Names.*, Symbols.* +import ast.tpd.* -import org.junit.Assert.assertEquals import org.junit.Test +import org.junit.Assert.* class PrinterTests extends DottyTest { - - private def newContext = { - initialCtx.setSetting(ctx.settings.color, "never") - } - ctx = newContext - - import tpd._ + override def initializeCtx(fc: FreshContext) = super.initializeCtx(fc.setSetting(fc.settings.color, "never")) @Test def packageObject: Unit = { @@ -47,30 +38,8 @@ class PrinterTests extends DottyTest { checkCompile("typer", source) { (tree, context) => given Context = context - val bar @ Trees.DefDef(_, _, _, _) = tree.find(tree => tree.symbol.name == termName("bar2")).get: @unchecked + val bar @ DefDef(_, _, _, _) = tree.find(tree => tree.symbol.name == termName("bar2")).get: @unchecked assertEquals("Int & (Boolean | String)", bar.tpt.show) } } - - @Test def string: Unit = assertEquals("foo", i"${"foo"}") - - import core.Flags._ - @Test def flagsSingle: Unit = assertEquals("final", i"$Final") - @Test def flagsSeq: Unit = assertEquals(", final", i"${Seq(JavaStatic, Final)}%, %") - @Test def flagsTuple: Unit = assertEquals("(,final)", i"${(JavaStatic, Final)}") - @Test def flagsSeqOfTuple: Unit = assertEquals("(final,given), (private,lazy)", i"${Seq((Final, Given), (Private, Lazy))}%, %") - - class StorePrinter extends config.Printers.Printer: - var string: String = "" - override def println(msg: => String) = string = msg - - @Test def testShowing: Unit = - val store = StorePrinter() - (JavaStatic | Final).showing(i"flags=$result", store) - assertEquals("flags=final ", store.string) - - @Test def TestShowingWithOriginalType: Unit = - val store = StorePrinter() - (JavaStatic | Final).showing(i"flags=${if result.is(Private) then result &~ Private else result | Private}", store) - assertEquals("flags=private final ", store.string) }