Skip to content

Commit c497fae

Browse files
dwijnanddwijnand
committed
Fix Text wrapping
There was a problem with how some text wrapped. I noticed it in the printing of big tuple types and I started to write unit tests for those, but there turned out to also be a checkfile that demonstrates the fix: tests/neg/i14127.check: - | Int - |, Int, Int)] was found for parameter x [...] - | Int - |, Int, Int)]: + | Int, Int, Int)] was found for parameter [...] + | Int, Int, Int)]: The fix is to make appendIndented, which is used by append, use Fluid instead of switching to Vertical. It seems to me that, pre-layout, Vertical means "I want my elements to be vertical", i.e. wrap every one, and it's layout that takes that into account. Fluid instead means "wrap where necessary, pack where possible". Post-layout, instead, it looks like Fluid means one-per-line, as it's consumed by print. So the fix is to switch appendIndented from Vertical to Fluid. Following that I made some improvements like not needless boxing non-splittable text, like Str, into Closed and merging non-closed Fluid text during concat (`~`). That fixed the extra wrapping, but it meant that the ", " separator could be the text that is wrapped and indented, which didn't look nice. So I changed Text.apply to close the separator to the previous element. That encouraged lines ending with ", ", that is with a trailing space, so I made print strip trailing spaces. The change from Vertical to Fluid made the last parenthesis in tests/neg/i9185.check not wrap, which is fine, but that pulled the trailing "failed with" up to the previous line. As that's not code, I decided to move it down and a line away from the code.
1 parent bf808b3 commit c497fae

File tree

13 files changed

+163
-31
lines changed

13 files changed

+163
-31
lines changed

‎compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
111111
protected def refinementNameString(tp: RefinedType): String = nameString(tp.refinedName)
112112

113113
/** String representation of a refinement */
114-
protected def toTextRefinement(rt: RefinedType): Closed =
114+
protected def toTextRefinement(rt: RefinedType): Text =
115115
(refinementNameString(rt) ~ toTextRHS(rt.refinedInfo)).close
116116

117117
protected def argText(arg: Type): Text = homogenizeArg(arg) match {

‎compiler/src/dotty/tools/dotc/printing/Texts.scala

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@ object Texts {
1515
case Vertical(relems) => relems.isEmpty
1616
}
1717

18+
// Str Ver Clo Flu
19+
// isVertical F T F F
20+
// isClosed F T T F
21+
// isFluid F F T T
22+
// isSplittable F F F T
1823
def isVertical: Boolean = isInstanceOf[Vertical]
1924
def isClosed: Boolean = isVertical || isInstanceOf[Closed]
2025
def isFluid: Boolean = isInstanceOf[Fluid]
2126
def isSplittable: Boolean = isFluid && !isClosed
2227

23-
def close: Closed = new Closed(relems)
28+
def close: Text = if isSplittable then Closed(relems) else this
2429

2530
def remaining(width: Int): Int = this match {
2631
case Str(s, _) =>
@@ -53,7 +58,7 @@ object Texts {
5358
}
5459

5560
private def appendIndented(that: Text)(width: Int): Text =
56-
Vertical(that.layout(width - indentMargin).indented :: this.relems)
61+
Fluid(that.layout(width - indentMargin).indented :: this.relems)
5762

5863
private def append(width: Int)(that: Text): Text =
5964
if (this.isEmpty) that.layout(width)
@@ -113,7 +118,7 @@ object Texts {
113118
sb.append("|")
114119
}
115120
}
116-
sb.append(s)
121+
sb.append(s.replaceAll("[ ]+$", ""))
117122
case _ =>
118123
var follow = false
119124
for (elem <- relems.reverse) {
@@ -138,7 +143,13 @@ object Texts {
138143
def ~ (that: Text): Text =
139144
if (this.isEmpty) that
140145
else if (that.isEmpty) this
141-
else Fluid(that :: this :: Nil)
146+
else this match
147+
case Fluid(relems1) if !isClosed => that match
148+
case Fluid(relems2) if !that.isClosed => Fluid(relems2 ++ relems1)
149+
case _ => Fluid(that +: relems1)
150+
case _ => that match
151+
case Fluid(relems2) if !that.isClosed => Fluid(relems2 :+ this)
152+
case _ => Fluid(that :: this :: Nil)
142153

143154
def ~~ (that: Text): Text =
144155
if (this.isEmpty) that
@@ -161,9 +172,9 @@ object Texts {
161172
def apply(xs: Traversable[Text], sep: String = " "): Text =
162173
if (sep == "\n") lines(xs)
163174
else {
164-
val ys = xs filterNot (_.isEmpty)
175+
val ys = xs.filterNot(_.isEmpty)
165176
if (ys.isEmpty) Str("")
166-
else ys reduce (_ ~ sep ~ _)
177+
else ys.reduceRight((a, b) => (a ~ sep).close ~ b)
167178
}
168179

169180
/** The given texts `xs`, each on a separate line */
@@ -176,12 +187,16 @@ object Texts {
176187

177188
case class Str(s: String, lineRange: LineRange = EmptyLineRange) extends Text {
178189
override def relems: List[Text] = List(this)
190+
override def toString = this match
191+
case Str(s, EmptyLineRange) => s"Str($s)"
192+
case Str(s, lineRange) => s"Str($s, $lineRange)"
179193
}
180194

181195
case class Vertical(relems: List[Text]) extends Text
182196
case class Fluid(relems: List[Text]) extends Text
183197

184-
class Closed(relems: List[Text]) extends Fluid(relems)
198+
class Closed(relems: List[Text]) extends Fluid(relems):
199+
override def productPrefix = "Closed"
185200

186201
implicit def stringToText(s: String): Text = Str(s)
187202

‎compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,9 @@ object ErrorReporting {
187187
|The tests were made under $constraintText"""
188188

189189
def whyFailedStr(fail: FailedExtension) =
190-
i""" failed with
190+
i"""
191+
|
192+
| failed with:
191193
|
192194
|${fail.whyFailed.message.indented(8)}"""
193195

‎compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,9 +568,9 @@ object Implicits:
568568
if reasons.length > 1 then
569569
reasons.mkString("\n\t* ", "\n\t* ", "")
570570
else
571-
reasons.mkString
571+
reasons.mkString(" ", "", "")
572572

573-
def explanation(using Context) = em"Failed to synthesize an instance of type ${clarify(expectedType)}: ${formatReasons}"
573+
def explanation(using Context) = em"Failed to synthesize an instance of type ${clarify(expectedType)}:${formatReasons}"
574574

575575
end Implicits
576576

‎compiler/test/dotty/tools/dotc/TupleShowTests.scala

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package dotty.tools
2+
package dotc
3+
4+
import core.*, Decorators.*, Symbols.*
5+
import printing.Texts.*
6+
7+
import org.junit.Test
8+
9+
class TupleShowTests extends DottyTest:
10+
def IntType = defn.IntType
11+
def LongType = defn.LongType
12+
def ShortType = defn.ShortType
13+
def Types_10 = List.fill(5)(IntType) ::: List.fill(5)(LongType)
14+
def Types_20 = Types_10 ::: Types_10
15+
16+
val tup0 = defn.tupleType(Nil)
17+
val tup1 = defn.tupleType(IntType :: Nil)
18+
val tup2 = defn.tupleType(IntType :: LongType :: Nil)
19+
val tup3 = defn.tupleType(IntType :: LongType :: ShortType :: Nil)
20+
val tup21 = defn.tupleType(Types_20 ::: IntType :: Nil)
21+
val tup22 = defn.tupleType(Types_20 ::: IntType :: LongType :: Nil)
22+
val tup23 = defn.tupleType(Types_20 ::: IntType :: LongType :: ShortType :: Nil)
23+
val tup24 = defn.tupleType(Types_20 ::: IntType :: LongType :: ShortType :: ShortType :: Nil)
24+
25+
@Test def tup0_show = chkEq("EmptyTuple.type", i"$tup0")
26+
@Test def tup1_show = chkEq("Tuple1[Int]", i"$tup1")
27+
@Test def tup2_show = chkEq("(Int, Long)", i"$tup2")
28+
@Test def tup3_show = chkEq("(Int, Long, Short)", i"$tup3")
29+
@Test def tup21_show = chkEq(res21, i"$tup21")
30+
@Test def tup22_show = chkEq(res22, i"$tup22")
31+
@Test def tup23_show = chkEq(res23, i"$tup23")
32+
@Test def tup24_show = chkEq(res24, i"$tup24")
33+
34+
@Test def tup3_text =
35+
val obt = tup3.toText(ctx.printer)
36+
val exp = Fluid(List(
37+
Str(")"),
38+
Str("Short"),
39+
Closed(List(Str(", "), Str("Long"))),
40+
Closed(List(Str(", "), Str("Int"))),
41+
Str("("),
42+
))
43+
chkEq(exp, obt)
44+
45+
@Test def tup3_layout10 =
46+
val obt = tup3.toText(ctx.printer).layout(10)
47+
val exp = Fluid(List(
48+
Str(" Short)"),
49+
Str(" Long, "),
50+
Str("(Int, "),
51+
))
52+
chkEq(exp, obt)
53+
54+
@Test def tup3_show10 = chkEq("(Int,\n Long,\n Short)", tup3.toText(ctx.printer).mkString(10, false))
55+
56+
val res21 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
57+
| Int, Long, Long, Long, Long, Long, Int)""".stripMargin
58+
59+
val res22 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
60+
| Int, Long, Long, Long, Long, Long, Int, Long)""".stripMargin
61+
62+
val res23 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
63+
| Int, Long, Long, Long, Long, Long, Int, Long, Short)""".stripMargin
64+
65+
val res24 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
66+
| Int, Long, Long, Long, Long, Long, Int, Long, Short, Short)""".stripMargin
67+
68+
def chkEq[A](expected: A, obtained: A) = assert(expected == obtained, diff(s"$expected", s"$obtained"))
69+
70+
def diff(exp: String, obt: String) =
71+
val min = math.min(exp.length, obt.length)
72+
val pre =
73+
var i = 0
74+
while i < min && exp(i) == obt(i) do i += 1
75+
exp.take(i)
76+
val suf =
77+
val max = min - pre.length - 1
78+
var i = 0
79+
while i <= max && exp(exp.length - 1 - i) == obt(obt.length - 1 - i) do i += 1
80+
exp.drop(exp.length - 1)
81+
82+
import scala.io.AnsiColor.*
83+
val ellip = BLACK + BOLD + "..." + RESET
84+
val compactPre = if pre.length <= 20 then pre else ellip + pre.drop(pre.length - 20)
85+
val compactSuf = if suf.length <= 20 then suf else suf.take(20) + ellip
86+
def extractDiff(s: String) = s.slice(pre.length, s.length - suf.length)
87+
s"""|Comparison Failure:
88+
| expected: $compactPre${CYAN }${extractDiff(exp)}$RESET$compactSuf
89+
| obtained: $compactPre$MAGENTA${extractDiff(obt)}$RESET$compactSuf
90+
|""".stripMargin

‎tests/neg/enum-values.check

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
| meaning a values array is not defined.
77
| An extension method was tried, but could not be fully constructed:
88
|
9-
| example.Extensions.values(Tag) failed with
9+
| example.Extensions.values(Tag)
10+
|
11+
| failed with:
1012
|
1113
| Found: example.Tag.type
1214
| Required: Nothing
@@ -18,7 +20,9 @@
1820
| meaning a values array is not defined.
1921
| An extension method was tried, but could not be fully constructed:
2022
|
21-
| example.Extensions.values(ListLike) failed with
23+
| example.Extensions.values(ListLike)
24+
|
25+
| failed with:
2226
|
2327
| Found: Array[example.Tag[?]]
2428
| Required: Array[example.ListLike[?]]
@@ -30,7 +34,9 @@
3034
| meaning a values array is not defined.
3135
| An extension method was tried, but could not be fully constructed:
3236
|
33-
| example.Extensions.values(TypeCtorsK) failed with
37+
| example.Extensions.values(TypeCtorsK)
38+
|
39+
| failed with:
3440
|
3541
| Found: Array[example.Tag[?]]
3642
| Required: Array[example.TypeCtorsK[?[_$1]]]
@@ -63,7 +69,9 @@
6369
| value values is not a member of object example.NotAnEnum.
6470
| An extension method was tried, but could not be fully constructed:
6571
|
66-
| example.Extensions.values(NotAnEnum) failed with
72+
| example.Extensions.values(NotAnEnum)
73+
|
74+
| failed with:
6775
|
6876
| Found: example.NotAnEnum.type
6977
| Required: Nothing

‎tests/neg/i10901.check

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
| value º is not a member of object BugExp4Point2D.IntT.
55
| An extension method was tried, but could not be fully constructed:
66
|
7-
| º(x) failed with
7+
| º(x)
8+
|
9+
| failed with:
810
|
911
| Ambiguous overload. The overloaded alternatives of method º in object dsl with types
1012
| [T1, T2]
@@ -22,7 +24,9 @@
2224
|value º is not a member of object BugExp4Point2D.IntT.
2325
|An extension method was tried, but could not be fully constructed:
2426
|
25-
| º(x) failed with
27+
| º(x)
28+
|
29+
| failed with:
2630
|
2731
| Ambiguous overload. The overloaded alternatives of method º in object dsl with types
2832
| [T1, T2]
@@ -36,6 +40,8 @@
3640
| value foo is not a member of String.
3741
| An extension method was tried, but could not be fully constructed:
3842
|
39-
| Test.foo("abc")(/* missing */summon[C]) failed with
43+
| Test.foo("abc")(/* missing */summon[C])
44+
|
45+
| failed with:
4046
|
4147
| No given instance of type C was found for parameter x$2 of method foo in object Test

‎tests/neg/i13558.check

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
| value id is not a member of testcode.A.
55
| An extension method was tried, but could not be fully constructed:
66
|
7-
| testcode.ExtensionA.id(a) failed with
7+
| testcode.ExtensionA.id(a)
8+
|
9+
| failed with:
810
|
911
| Reference to id is ambiguous,
1012
| it is both imported by import testcode.ExtensionB._
@@ -15,7 +17,9 @@
1517
| value id is not a member of testcode.A.
1618
| An extension method was tried, but could not be fully constructed:
1719
|
18-
| testcode.ExtensionB.id(a) failed with
20+
| testcode.ExtensionB.id(a)
21+
|
22+
| failed with:
1923
|
2024
| Reference to id is ambiguous,
2125
| it is both imported by import testcode.ExtensionA._

‎tests/neg/i14127.check

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
-- Error: tests/neg/i14127.scala:6:55 ----------------------------------------------------------------------------------
22
6 | *: Int *: Int *: Int *: Int *: Int *: EmptyTuple)]] // error
33
| ^
4-
|No given instance of type deriving.Mirror.Of[(Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int,
5-
| Int
6-
|, Int, Int)] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[(Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int,
7-
| Int
8-
|, Int, Int)]:
4+
|No given instance of type deriving.Mirror.Of[(Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int,
5+
| Int, Int, Int)] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[(Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int,
6+
| Int, Int, Int)]:
97
| * class *: is not a generic product because it reduces to a tuple with arity 23, expected arity <= 22
108
| * class *: is not a generic sum because it does not have subclasses

‎tests/neg/i15000.check

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
|value apply is not a member of object ExtensionMethodReproduction.c.
1717
|An extension method was tried, but could not be fully constructed:
1818
|
19-
| apply(ExtensionMethodReproduction.c) failed with
19+
| apply(ExtensionMethodReproduction.c)
20+
|
21+
| failed with:
2022
|
2123
| Ambiguous overload. The overloaded alternatives of method apply in object ExtensionMethodReproduction with types
2224
| (c: ExtensionMethodReproduction.C)(x: Int, y: Int): String

‎tests/neg/i6183.check

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
| value render is not a member of Int.
55
| An extension method was tried, but could not be fully constructed:
66
|
7-
| render(42) failed with
7+
| render(42)
8+
|
9+
| failed with:
810
|
911
| Ambiguous overload. The overloaded alternatives of method render in object Test with types
1012
| [B](b: B)(using x$2: DummyImplicit): Char

‎tests/neg/i6779.check

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
| value f is not a member of T.
1212
| An extension method was tried, but could not be fully constructed:
1313
|
14-
| Test.f[G[T]](x)(given_Stuff) failed with
14+
| Test.f[G[T]](x)(given_Stuff)
15+
|
16+
| failed with:
1517
|
1618
| Found: (x : T)
1719
| Required: G[T]

‎tests/neg/i9185.check

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
|An extension method was tried, but could not be fully constructed:
66
|
77
| M.pure[A, F]("ola")(
8-
| /* ambiguous: both object listMonad in object M and object optionMonad in object M match type M[F] */summon[M[F]]
9-
| ) failed with
8+
| /* ambiguous: both object listMonad in object M and object optionMonad in object M match type M[F] */summon[M[F]])
9+
|
10+
| failed with:
1011
|
1112
| Ambiguous given instances: both object listMonad in object M and object optionMonad in object M match type M[F] of parameter m of method pure in object M
1213
-- Error: tests/neg/i9185.scala:8:28 -----------------------------------------------------------------------------------
@@ -19,7 +20,9 @@
1920
| value len is not a member of String.
2021
| An extension method was tried, but could not be fully constructed:
2122
|
22-
| M.len("abc") failed with
23+
| M.len("abc")
24+
|
25+
| failed with:
2326
|
2427
| Found: ("abc" : String)
2528
| Required: Int

0 commit comments

Comments
 (0)