Skip to content

Commit ae5c5a5

Browse files
committed
Switch our string interpolators to use Show/Shown
As it was our string interpolators were taking Any values and then trying to pattern match back their classes to try to show them nicely. This inevitably fails for things like the opaque type FlagSet, which interpolates as an indecipherable long number. Now, instead, they take "Shown" arguments, for which there is an implicit conversion in scope, given there is a Show instance for value. I captured some desired results in some new unit test cases. In the process a small handful of bugs were discovered, the only particularly bad one was consuming a Iterator when the "transforms" printer was enabled (accessorDefs), followed by an unintentional eta-expansion of a method with a defaulted argument (showSummary). I also lifted out the Showable and exception fallback function as an extension method, so I could use it more broadly. The use of WrappedResult and its `result` in `showing` was also impacted, because the new expected Shown type was driving `result`'s context lookup. Fortunately I was able to continue to use WrappedResult and `result` as defined by handling this API change inside `showing` alone. I wasn't, however, able to find a solution to the impact the new Shown expected type was having on the `stripModuleClassSuffix` extension method, sadly. JSExportsGen is interpolating a private type, so rather than open its access or giving it a Show instance I changed the string interpolation. SyntaxFormatter was no longer used, since the "hl" interpolator was dropped.
1 parent af832ba commit ae5c5a5

File tree

12 files changed

+164
-77
lines changed

12 files changed

+164
-77
lines changed

compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
126126
if (kind != overallKind) {
127127
bad = true
128128
report.error(
129-
em"export overload conflicts with export of $firstSym: they are of different types ($kind / $overallKind)",
129+
em"export overload conflicts with export of $firstSym: they are of different types (${kind.show} / ${overallKind.show})",
130130
info.pos)
131131
}
132132
}

compiler/src/dotty/tools/dotc/core/Decorators.scala

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@ package dotty.tools
22
package dotc
33
package core
44

5-
import annotation.tailrec
6-
import Symbols._
7-
import Contexts._, Names._, Phases._, printing.Texts._, printing.Printer
8-
import util.Spans.Span
9-
import collection.mutable.ListBuffer
10-
import dotty.tools.dotc.transform.MegaPhase
11-
import ast.tpd._
125
import scala.language.implicitConversions
13-
import printing.Formatting._
6+
7+
import scala.annotation.tailrec
8+
import scala.collection.mutable.ListBuffer
9+
import scala.util.control.NonFatal
10+
11+
import Contexts._, Names._, Phases._, Symbols._
12+
import ast.tpd._
13+
import printing.{ Printer, Showable }, printing.Formatting._, printing.Texts._
14+
import transform.MegaPhase
15+
import util.Spans.Span
1416

1517
/** This object provides useful implicit decorators for types defined elsewhere */
1618
object Decorators {
@@ -249,13 +251,30 @@ object Decorators {
249251
}
250252

251253
extension [T](x: T)
252-
def showing(
253-
op: WrappedResult[T] ?=> String,
254-
printer: config.Printers.Printer = config.Printers.default): T = {
255-
printer.println(op(using WrappedResult(x)))
254+
def showing[U](
255+
op: WrappedResult[U] ?=> String,
256+
printer: config.Printers.Printer = config.Printers.default)(using c: Conversion[T, U] = null): T = {
257+
// either the use of `$result` was driven by the expected type of `Shown`
258+
// which lead to the summoning of `Conversion[T, Shown]` (which we'll invoke)
259+
// or no such conversion was found so we'll consume the result as it is instead
260+
val obj = if c == null then x.asInstanceOf[U] else c(x)
261+
printer.println(op(using WrappedResult(obj)))
256262
x
257263
}
258264

265+
/** Instead of `toString` call `show` on `Showable` values, falling back to `toString` if an exception is raised. */
266+
def show(using Context)(using z: Show[T] = null): String = x match
267+
case _ if z != null => z.show(x).toString
268+
case x: Showable =>
269+
try x.show
270+
catch
271+
case ex: CyclicReference => "... (caught cyclic reference) ..."
272+
case NonFatal(ex)
273+
if !ctx.mode.is(Mode.PrintShowExceptions) && !ctx.settings.YshowPrintErrors.value =>
274+
val msg = ex match { case te: TypeError => te.toMessage case _ => ex.getMessage }
275+
s"[cannot display due to $msg, raw string = $x]"
276+
case _ => String.valueOf(x)
277+
259278
extension [T](x: T)
260279
def assertingErrorsReported(using Context): T = {
261280
assert(ctx.reporter.errorsReported)
@@ -272,19 +291,19 @@ object Decorators {
272291

273292
extension (sc: StringContext)
274293
/** General purpose string formatting */
275-
def i(args: Any*)(using Context): String =
294+
def i(args: Shown*)(using Context): String =
276295
new StringFormatter(sc).assemble(args)
277296

278297
/** Formatting for error messages: Like `i` but suppress follow-on
279298
* error messages after the first one if some of their arguments are "non-sensical".
280299
*/
281-
def em(args: Any*)(using Context): String =
300+
def em(args: Shown*)(using Context): String =
282301
new ErrorMessageFormatter(sc).assemble(args)
283302

284303
/** Formatting with added explanations: Like `em`, but add explanations to
285304
* give more info about type variables and to disambiguate where needed.
286305
*/
287-
def ex(args: Any*)(using Context): String =
306+
def ex(args: Shown*)(using Context): String =
288307
explained(em(args: _*))
289308

290309
extension [T <: AnyRef](arr: Array[T])

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2701,15 +2701,16 @@ object TypeComparer {
27012701
*/
27022702
val Fresh: Repr = 4
27032703

2704-
extension (approx: Repr)
2705-
def low: Boolean = (approx & LoApprox) != 0
2706-
def high: Boolean = (approx & HiApprox) != 0
2707-
def addLow: Repr = approx | LoApprox
2708-
def addHigh: Repr = approx | HiApprox
2709-
def show: String =
2710-
val lo = if low then " (left is approximated)" else ""
2711-
val hi = if high then " (right is approximated)" else ""
2712-
lo ++ hi
2704+
object Repr:
2705+
extension (approx: Repr)
2706+
def low: Boolean = (approx & LoApprox) != 0
2707+
def high: Boolean = (approx & HiApprox) != 0
2708+
def addLow: Repr = approx | LoApprox
2709+
def addHigh: Repr = approx | HiApprox
2710+
def show: String =
2711+
val lo = if low then " (left is approximated)" else ""
2712+
val hi = if high then " (right is approximated)" else ""
2713+
lo ++ hi
27132714
end ApproxState
27142715
type ApproxState = ApproxState.Repr
27152716

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ object Parsers {
594594
if startIndentWidth <= nextIndentWidth then
595595
i"""Line is indented too far to the right, or a `{` is missing before:
596596
|
597-
|$t"""
597+
|${t.show}"""
598598
else
599599
in.spaceTabMismatchMsg(startIndentWidth, nextIndentWidth),
600600
in.next.offset

compiler/src/dotty/tools/dotc/printing/Formatting.scala

Lines changed: 80 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,101 @@
1-
package dotty.tools.dotc
1+
package dotty.tools
2+
package dotc
23
package printing
34

5+
import scala.collection.mutable
6+
47
import core._
58
import Texts._, Types._, Flags._, Symbols._, Contexts._
6-
import collection.mutable
79
import Decorators._
8-
import scala.util.control.NonFatal
910
import reporting.Message
1011
import util.DiffUtil
1112
import Highlighting._
1213

1314
object Formatting {
1415

16+
object ShownDef:
17+
/** Represents a value that has been "shown" and can be consumed by StringFormatter.
18+
* Not just a string because it may be a Seq that StringFormatter will intersperse with the trailing separator.
19+
* Also, it's not a `String | Seq[String]` because then we'd need to a Context to call `Showable#show`. We could
20+
* make Context a requirement for a Show instance but then we'd have lots of instances instead of just one ShowAny
21+
* instance. We could also try to make `Show#show` require the Context, but then that breaks the Conversion. */
22+
opaque type Shown = Any
23+
object Shown:
24+
given [A: Show]: Conversion[A, Shown] = Show[A].show(_)
25+
26+
sealed abstract class Show[-T]:
27+
/** Show a value T by returning a "shown" result. */
28+
def show(x: T): Shown
29+
30+
/** The base implementation, passing the argument to StringFormatter which will try to `.show` it. */
31+
object ShowAny extends Show[Any]:
32+
def show(x: Any): Shown = x
33+
34+
class ShowImplicits1:
35+
given Show[ImplicitRef] = ShowAny
36+
given Show[Names.Designator] = ShowAny
37+
given Show[util.SrcPos] = ShowAny
38+
39+
object Show extends ShowImplicits1:
40+
inline def apply[A](using inline z: Show[A]): Show[A] = z
41+
42+
given [X: Show]: Show[Seq[X]] with
43+
def show(x: Seq[X]) = x.map(Show[X].show)
44+
45+
given [A: Show, B: Show]: Show[(A, B)] with
46+
def show(x: (A, B)) = (Show[A].show(x._1), Show[B].show(x._2))
47+
48+
given Show[FlagSet] with
49+
def show(x: FlagSet) = x.flagsString
50+
51+
given Show[TypeComparer.ApproxState] with
52+
def show(x: TypeComparer.ApproxState) = TypeComparer.ApproxState.Repr.show(x)
53+
54+
given Show[Showable] = ShowAny
55+
given Show[Shown] = ShowAny
56+
given Show[Int] = ShowAny
57+
given Show[Char] = ShowAny
58+
given Show[Boolean] = ShowAny
59+
given Show[String] = ShowAny
60+
given Show[Class[?]] = ShowAny
61+
given Show[Exception] = ShowAny
62+
given Show[StringBuffer] = ShowAny
63+
given Show[Atoms] = ShowAny
64+
given Show[Highlight] = ShowAny
65+
given Show[HighlightBuffer] = ShowAny
66+
given Show[CompilationUnit] = ShowAny
67+
given Show[Mode] = ShowAny
68+
given Show[Phases.Phase] = ShowAny
69+
given Show[Signature] = ShowAny
70+
given Show[TyperState] = ShowAny
71+
given Show[config.ScalaVersion] = ShowAny
72+
given Show[io.AbstractFile] = ShowAny
73+
given Show[parsing.Scanners.IndentWidth] = ShowAny
74+
given Show[parsing.Scanners.Scanner] = ShowAny
75+
given Show[util.SourceFile] = ShowAny
76+
given Show[util.Spans.Span] = ShowAny
77+
given Show[dotty.tools.tasty.TastyBuffer.Addr] = ShowAny
78+
given Show[tasty.TreeUnpickler#OwnerTree] = ShowAny
79+
given Show[interactive.Completion] = ShowAny
80+
given Show[transform.sjs.JSSymUtils.JSName] = ShowAny
81+
given Show[org.scalajs.ir.Position] = ShowAny
82+
end Show
83+
end ShownDef
84+
export ShownDef.{ Show, Shown }
85+
1586
/** General purpose string formatter, with the following features:
1687
*
17-
* 1) On all Showables, `show` is called instead of `toString`
18-
* 2) Exceptions raised by a `show` are handled by falling back to `toString`.
19-
* 3) Sequences can be formatted using the desired separator between two `%` signs,
88+
* 1. Invokes the `show` extension method on the interpolated arguments.
89+
* 2. Sequences can be formatted using the desired separator between two `%` signs,
2090
* eg `i"myList = (${myList}%, %)"`
21-
* 4) Safe handling of multi-line margins. Left margins are skipped om the parts
91+
* 3. Safe handling of multi-line margins. Left margins are stripped on the parts
2292
* of the string context *before* inserting the arguments. That way, we guard
2393
* against accidentally treating an interpolated value as a margin.
2494
*/
2595
class StringFormatter(protected val sc: StringContext) {
26-
protected def showArg(arg: Any)(using Context): String = arg match {
27-
case arg: Showable =>
28-
try arg.show
29-
catch {
30-
case ex: CyclicReference => "... (caught cyclic reference) ..."
31-
case NonFatal(ex)
32-
if !ctx.mode.is(Mode.PrintShowExceptions) &&
33-
!ctx.settings.YshowPrintErrors.value =>
34-
val msg = ex match
35-
case te: TypeError => te.toMessage
36-
case _ => ex.getMessage
37-
s"[cannot display due to $msg, raw string = ${arg.toString}]"
38-
}
39-
case _ => String.valueOf(arg)
40-
}
96+
protected def showArg(arg: Any)(using Context): String = arg.show
4197

42-
private def treatArg(arg: Any, suffix: String)(using Context): (Any, String) = arg match {
98+
private def treatArg(arg: Shown, suffix: String)(using Context): (Any, String) = arg match {
4399
case arg: Seq[?] if suffix.nonEmpty && suffix.head == '%' =>
44100
val (rawsep, rest) = suffix.tail.span(_ != '%')
45101
val sep = StringContext.processEscapes(rawsep)
@@ -49,7 +105,7 @@ object Formatting {
49105
(showArg(arg), suffix)
50106
}
51107

52-
def assemble(args: Seq[Any])(using Context): String = {
108+
def assemble(args: Seq[Shown])(using Context): String = {
53109
def isLineBreak(c: Char) = c == '\n' || c == '\f' // compatible with StringLike#isLineBreak
54110
def stripTrailingPart(s: String) = {
55111
val (pre, post) = s.span(c => !isLineBreak(c))
@@ -75,18 +131,6 @@ object Formatting {
75131
override protected def showArg(arg: Any)(using Context): String =
76132
wrapNonSensical(arg, super.showArg(arg)(using errorMessageCtx))
77133

78-
class SyntaxFormatter(sc: StringContext) extends StringFormatter(sc) {
79-
override protected def showArg(arg: Any)(using Context): String =
80-
arg match {
81-
case hl: Highlight =>
82-
hl.show
83-
case hb: HighlightBuffer =>
84-
hb.toString
85-
case _ =>
86-
SyntaxHighlighting.highlight(super.showArg(arg))
87-
}
88-
}
89-
90134
private def wrapNonSensical(arg: Any, str: String)(using Context): String = {
91135
import Message._
92136
def isSensical(arg: Any): Boolean = arg match {

compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1752,7 +1752,7 @@ import transform.SymUtils._
17521752

17531753
class ClassAndCompanionNameClash(cls: Symbol, other: Symbol)(using Context)
17541754
extends NamingMsg(ClassAndCompanionNameClashID) {
1755-
def msg = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix}"
1755+
def msg = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix.show}"
17561756
def explain =
17571757
em"""|A ${cls.kindString} and its companion object cannot both define a ${hl("class")}, ${hl("trait")} or ${hl("object")} with the same name:
17581758
| - ${cls.owner} defines ${cls}

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
776776
case n: Name =>
777777
h = nameHash(n, h)
778778
case elem =>
779-
cannotHash(what = i"`$elem` of unknown class ${elem.getClass}", elem, tree)
779+
cannotHash(what = i"`${elem.show}` of unknown class ${elem.getClass}", elem, tree)
780780
h
781781
end iteratorHash
782782

compiler/src/dotty/tools/dotc/transform/AccessProxies.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ abstract class AccessProxies {
6060

6161
/** Add all needed accessors to the `body` of class `cls` */
6262
def addAccessorDefs(cls: Symbol, body: List[Tree])(using Context): List[Tree] = {
63-
val accDefs = accessorDefs(cls)
63+
val accDefs = accessorDefs(cls).toList
6464
transforms.println(i"add accessors for $cls: $accDefs%, %")
6565
if (accDefs.isEmpty) body else body ++ accDefs
6666
}

compiler/src/dotty/tools/dotc/transform/DropOuterAccessors.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class DropOuterAccessors extends MiniPhase with IdentityDenotTransformer:
8282
cpy.Block(rhs)(inits.filterNot(dropOuterInit), expr)
8383
})
8484
assert(droppedParamAccessors.isEmpty,
85-
i"""Failed to eliminate: $droppedParamAccessors
85+
i"""Failed to eliminate: ${droppedParamAccessors.toList}
8686
when dropping outer accessors for ${ctx.owner} with
8787
$impl""")
8888
cpy.Template(impl)(constr = constr1, body = body1)

compiler/src/dotty/tools/dotc/transform/Erasure.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ object Erasure {
268268
def constant(tree: Tree, const: Tree)(using Context): Tree =
269269
(if (isPureExpr(tree)) const else Block(tree :: Nil, const)).withSpan(tree.span)
270270

271-
final def box(tree: Tree, target: => String = "")(using Context): Tree = trace(i"boxing ${tree.showSummary}: ${tree.tpe} into $target") {
271+
final def box(tree: Tree, target: => String = "")(using Context): Tree = trace(i"boxing ${tree.showSummary()}: ${tree.tpe} into $target") {
272272
tree.tpe.widen match {
273273
case ErasedValueType(tycon, _) =>
274274
New(tycon, cast(tree, underlyingOfValueClass(tycon.symbol.asClass)) :: Nil) // todo: use adaptToType?
@@ -288,7 +288,7 @@ object Erasure {
288288
}
289289
}
290290

291-
def unbox(tree: Tree, pt: Type)(using Context): Tree = trace(i"unboxing ${tree.showSummary}: ${tree.tpe} as a $pt") {
291+
def unbox(tree: Tree, pt: Type)(using Context): Tree = trace(i"unboxing ${tree.showSummary()}: ${tree.tpe} as a $pt") {
292292
pt match {
293293
case ErasedValueType(tycon, underlying) =>
294294
def unboxedTree(t: Tree) =
@@ -1032,7 +1032,7 @@ object Erasure {
10321032
}
10331033

10341034
override def adapt(tree: Tree, pt: Type, locked: TypeVars, tryGadtHealing: Boolean)(using Context): Tree =
1035-
trace(i"adapting ${tree.showSummary}: ${tree.tpe} to $pt", show = true) {
1035+
trace(i"adapting ${tree.showSummary()}: ${tree.tpe} to $pt", show = true) {
10361036
if ctx.phase != erasurePhase && ctx.phase != erasurePhase.next then
10371037
// this can happen when reading annotations loaded during erasure,
10381038
// since these are loaded at phase typer.

compiler/src/dotty/tools/dotc/typer/Checking.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,7 @@ trait Checking {
11981198
case _: TypeTree =>
11991199
case _ =>
12001200
if tree.tpe.typeParams.nonEmpty then
1201-
val what = if tree.symbol.exists then tree.symbol else i"type $tree"
1201+
val what = if tree.symbol.exists then tree.symbol.show else i"type $tree"
12021202
report.error(em"$what takes type parameters", tree.srcPos)
12031203

12041204
/** Check that we are in an inline context (inside an inline method or in inline code) */

compiler/test/dotty/tools/dotc/printing/PrinterTests.scala

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
package dotty.tools.dotc.printing
1+
package dotty.tools
2+
package dotc
3+
package printing
24

3-
import dotty.tools.DottyTest
4-
import dotty.tools.dotc.ast.{Trees,tpd}
5-
import dotty.tools.dotc.core.Names._
6-
import dotty.tools.dotc.core.Symbols._
7-
import dotty.tools.dotc.core.Decorators._
8-
import dotty.tools.dotc.core.Contexts.{Context, ctx}
5+
import ast.{ Trees, tpd }
6+
import core.Names._
7+
import core.Symbols._
8+
import core.Decorators._
9+
import core.Contexts.{ Context, ctx }
910

1011
import org.junit.Assert.assertEquals
1112
import org.junit.Test
@@ -50,4 +51,26 @@ class PrinterTests extends DottyTest {
5051
assertEquals("Int & (Boolean | String)", bar.tpt.show)
5152
}
5253
}
54+
55+
@Test def string: Unit = assertEquals("foo", i"${"foo"}")
56+
57+
import core.Flags._
58+
@Test def flagsSingle: Unit = assertEquals("final", i"$Final")
59+
@Test def flagsSeq: Unit = assertEquals("<static>, final", i"${Seq(JavaStatic, Final)}%, %")
60+
@Test def flagsTuple: Unit = assertEquals("(<static>,final)", i"${(JavaStatic, Final)}")
61+
@Test def flagsSeqOfTuple: Unit = assertEquals("(final,given), (private,lazy)", i"${Seq((Final, Given), (Private, Lazy))}%, %")
62+
63+
class StorePrinter extends config.Printers.Printer:
64+
var string: String = "<never set>"
65+
override def println(msg: => String) = string = msg
66+
67+
@Test def testShowing: Unit =
68+
val store = StorePrinter()
69+
(JavaStatic | Final).showing(i"flags=$result", store)
70+
assertEquals("flags=final <static>", store.string)
71+
72+
@Test def TestShowingWithOriginalType: Unit =
73+
val store = StorePrinter()
74+
(JavaStatic | Final).showing(i"flags=${if result.is(Private) then result &~ Private else result | Private}", store)
75+
assertEquals("flags=private final <static>", store.string)
5376
}

0 commit comments

Comments
 (0)