Fix aspects of the Show setup

dwijnand · dwijnand · commit e424a59b6d47 · 2022-08-02T22:07:46.000+01:00
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.
diff --git 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
@@ -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
@@ -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 CtxShown, 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,10 @@ 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]) = (ctx ?=> x.map(show1)): CtxShow
 
       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)) = (ctx ?=> (show1(x._1), show1(x._2))): CtxShow
 
       given [X: Show]: Show[X | Null] with
         def show(x: X | Null) = if x == null then "null" else Show[X].show(x.nn)
@@ -67,10 +70,10 @@ object Formatting {
         def show(x: TypeComparer.ApproxState) = TypeComparer.ApproxState.Repr.show(x)
 
       given Show[Showable]                            = ShowAny
-      given Show[Shown]                               = ShowAny
       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 +87,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 +108,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 +141,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 def wrapNonSensical(arg: Any, str: String)(using Context): String = {
+  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, text: Text)(using Context): Text = {
     import Message._
     def isSensical(arg: Any): Boolean = arg match {
       case tpe: Type =>
@@ -151,8 +160,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 +212,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,18 +316,22 @@ object Formatting {
   }
 
   private def errorMessageCtx(using Context): Context =
-    ctx.property(MessageLimiter) match
-      case Some(_: ErrorMessageLimiter) => ctx
-      case _ => ctx.fresh.setProperty(MessageLimiter, ErrorMessageLimiter())
+    ctx.withProperty(MessageLimiter)(ErrorMessageLimiter()).withPrinter(new ErrorMessagePrinter(_))
 
   /** Context with correct printer set for explanations */
   private def explainCtx(seen: Seen)(using Context): Context =
-    val ectx = errorMessageCtx
-    ectx.printer match
-      case dp: ExplainingPrinter =>
-        ectx // re-use outer printer and defer explanation to it
-      case _ =>
-        ectx.fresh.setPrinterFn(ctx => new ExplainingPrinter(seen)(ctx))
+    errorMessageCtx.withPrinter(new ExplainingPrinter(seen)(_))
+
+  extension (ctx: Context)
+    private def withProperty[V, T <: V](key: util.Property.Key[V])(mk: => T)(using reflect.TypeTest[V, T]) =
+      ctx.property(key) match
+        case Some(_: T) => ctx
+        case _          => ctx.fresh.setProperty(key, mk)
+
+    private def withPrinter[T <: Printer](mk: Context => T)(using reflect.TypeTest[Printer, T]) =
+      ctx.printer match
+        case _: T => ctx // re-use outer printer and defer to it
+        case _    => ctx.fresh.setPrinterFn(mk(_))
 
   /** Entrypoint for explanation string interpolator:
     *
@@ -364,8 +377,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
@@ -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
@@ -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("<static>, final", i"${Seq(JavaStatic, Final)}%, %")
+  @Test def flagsTup   = check("(<static>,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 = "<never set>"
+    override def println(msg: => String) = string = msg
+
+  @Test def testShowing: Unit =
+    val store = StorePrinter()
+    (JavaStatic | Final).showing(i"flags=$result", store)
+    assertEquals("flags=final <static>", 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 <static>", 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("<nonsensical>type Err</nonsensical>", 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, <nonsensical>type Err</nonsensical>]", em"${Seq(defn.AnyClass, Err)}")
+  @Test def seqSeqErr  = check("class Any; <nonsensical>type Err</nonsensical>", em"${Seq(defn.AnyClass, Err)}%; %")
+  @Test def tupleErr   = check("(1,<nonsensical>type Err</nonsensical>)", 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("<nonsensical>type Err</nonsensical>", 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, <nonsensical>type Err</nonsensical>]", ex"${Seq(defn.AnyClass, Err)}")
+  @Test def seqSeqErr  = check("class Any; <nonsensical>type Err</nonsensical>", ex"${Seq(defn.AnyClass, Err)}%; %")
+  @Test def tupleErr   = check("(1,<nonsensical>type Err</nonsensical>)", 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
@@ -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("<static>, final", i"${Seq(JavaStatic, Final)}%, %")
-  @Test def flagsTuple: Unit       = assertEquals("(<static>,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 = "<never set>"
-    override def println(msg: => String) = string = msg
-
-  @Test def testShowing: Unit =
-    val store = StorePrinter()
-    (JavaStatic | Final).showing(i"flags=$result", store)
-    assertEquals("flags=final <static>", 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 <static>", store.string)
 }
