Skip to content

Commit a6d28b4

Browse files
dwijnanddwijnand
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 c1f71db commit a6d28b4

File tree

12 files changed

+160
-73
lines changed

12 files changed

+160
-73
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
118118
if (kind != overallKind) {
119119
bad = true
120120
report.error(
121-
em"export overload conflicts with export of $firstSym: they are of different types ($kind / $overallKind)",
121+
em"export overload conflicts with export of $firstSym: they are of different types (${kind.show} / ${overallKind.show})",
122122
info.pos)
123123
}
124124
}

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

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

5-
import annotation.tailrec
6-
import Symbols._
7-
import Contexts._, Names._, Phases._, printing.Texts._
8-
import collection.mutable.ListBuffer
9-
import dotty.tools.dotc.transform.MegaPhase
10-
import printing.Formatting._
5+
import scala.annotation.tailrec
6+
import scala.collection.mutable.ListBuffer
7+
import scala.util.control.NonFatal
8+
9+
import Contexts._, Names._, Phases._, Symbols._
10+
import printing.{ Printer, Showable }, printing.Formatting._, printing.Texts._
11+
import transform.MegaPhase
1112

1213
/** This object provides useful implicit decorators for types defined elsewhere */
1314
object Decorators {
@@ -246,13 +247,30 @@ object Decorators {
246247
}
247248

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

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

270288
extension (sc: StringContext)
271289
/** General purpose string formatting */
272-
def i(args: Any*)(using Context): String =
290+
def i(args: Shown*)(using Context): String =
273291
new StringFormatter(sc).assemble(args)
274292

275293
/** Formatting for error messages: Like `i` but suppress follow-on
276294
* error messages after the first one if some of their arguments are "non-sensical".
277295
*/
278-
def em(args: Any*)(using Context): String =
296+
def em(args: Shown*)(using Context): String =
279297
new ErrorMessageFormatter(sc).assemble(args)
280298

281299
/** Formatting with added explanations: Like `em`, but add explanations to
282300
* give more info about type variables and to disambiguate where needed.
283301
*/
284-
def ex(args: Any*)(using Context): String =
302+
def ex(args: Shown*)(using Context): String =
285303
explained(em(args: _*))
286304

287305
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
@@ -2710,15 +2710,16 @@ object TypeComparer {
27102710
*/
27112711
val Fresh: Repr = 4
27122712

2713-
extension (approx: Repr)
2714-
def low: Boolean = (approx & LoApprox) != 0
2715-
def high: Boolean = (approx & HiApprox) != 0
2716-
def addLow: Repr = approx | LoApprox
2717-
def addHigh: Repr = approx | HiApprox
2718-
def show: String =
2719-
val lo = if low then " (left is approximated)" else ""
2720-
val hi = if high then " (right is approximated)" else ""
2721-
lo ++ hi
2713+
object Repr:
2714+
extension (approx: Repr)
2715+
def low: Boolean = (approx & LoApprox) != 0
2716+
def high: Boolean = (approx & HiApprox) != 0
2717+
def addLow: Repr = approx | LoApprox
2718+
def addHigh: Repr = approx | HiApprox
2719+
def show: String =
2720+
val lo = if low then " (left is approximated)" else ""
2721+
val hi = if high then " (right is approximated)" else ""
2722+
lo ++ hi
27222723
end ApproxState
27232724
type ApproxState = ApproxState.Repr
27242725

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ object Parsers {
599599
if startIndentWidth <= nextIndentWidth then
600600
i"""Line is indented too far to the right, or a `{` is missing before:
601601
|
602-
|$t"""
602+
|${t.show}"""
603603
else
604604
in.spaceTabMismatchMsg(startIndentWidth, nextIndentWidth),
605605
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
@@ -1751,7 +1751,7 @@ import transform.SymUtils._
17511751

17521752
class ClassAndCompanionNameClash(cls: Symbol, other: Symbol)(using Context)
17531753
extends NamingMsg(ClassAndCompanionNameClashID) {
1754-
def msg = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix}"
1754+
def msg = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix.show}"
17551755
def explain =
17561756
em"""|A ${cls.kindString} and its companion object cannot both define a ${hl("class")}, ${hl("trait")} or ${hl("object")} with the same name:
17571757
| - ${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
@@ -774,7 +774,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
774774
case n: Name =>
775775
h = nameHash(n, h)
776776
case elem =>
777-
cannotHash(what = i"`$elem` of unknown class ${elem.getClass}", elem, tree)
777+
cannotHash(what = i"`${elem.show}` of unknown class ${elem.getClass}", elem, tree)
778778
h
779779
end iteratorHash
780780

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

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

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class DropOuterAccessors extends MiniPhase with IdentityDenotTransformer:
8080
cpy.Block(rhs)(inits.filterNot(dropOuterInit), expr)
8181
})
8282
assert(droppedParamAccessors.isEmpty,
83-
i"""Failed to eliminate: $droppedParamAccessors
83+
i"""Failed to eliminate: ${droppedParamAccessors.toList}
8484
when dropping outer accessors for ${ctx.owner} with
8585
$impl""")
8686
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
@@ -266,7 +266,7 @@ object Erasure {
266266
def constant(tree: Tree, const: Tree)(using Context): Tree =
267267
(if (isPureExpr(tree)) const else Block(tree :: Nil, const)).withSpan(tree.span)
268268

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

289-
def unbox(tree: Tree, pt: Type)(using Context): Tree = trace(i"unboxing ${tree.showSummary}: ${tree.tpe} as a $pt") {
289+
def unbox(tree: Tree, pt: Type)(using Context): Tree = trace(i"unboxing ${tree.showSummary()}: ${tree.tpe} as a $pt") {
290290
pt match {
291291
case ErasedValueType(tycon, underlying) =>
292292
def unboxedTree(t: Tree) =
@@ -1030,7 +1030,7 @@ object Erasure {
10301030
}
10311031

10321032
override def adapt(tree: Tree, pt: Type, locked: TypeVars, tryGadtHealing: Boolean)(using Context): Tree =
1033-
trace(i"adapting ${tree.showSummary}: ${tree.tpe} to $pt", show = true) {
1033+
trace(i"adapting ${tree.showSummary()}: ${tree.tpe} to $pt", show = true) {
10341034
if ctx.phase != erasurePhase && ctx.phase != erasurePhase.next then
10351035
// this can happen when reading annotations loaded during erasure,
10361036
// 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
@@ -1195,7 +1195,7 @@ trait Checking {
11951195
case _: TypeTree =>
11961196
case _ =>
11971197
if tree.tpe.typeParams.nonEmpty then
1198-
val what = if tree.symbol.exists then tree.symbol else i"type $tree"
1198+
val what = if tree.symbol.exists then tree.symbol.show else i"type $tree"
11991199
report.error(em"$what takes type parameters", tree.srcPos)
12001200

12011201
/** 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: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
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._
5+
import ast.{ Trees, tpd }
6+
import core.Names._
7+
import core.Symbols._
8+
import core.Decorators._
79
import dotty.tools.dotc.core.Contexts.Context
810

911
import org.junit.Assert.assertEquals
@@ -49,4 +51,26 @@ class PrinterTests extends DottyTest {
4951
assertEquals("Int & (Boolean | String)", bar.tpt.show)
5052
}
5153
}
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)
5276
}

0 commit comments

Comments
 (0)