Skip to content

Commit

Permalink
Fix Text wrapping
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dwijnand committed Nov 3, 2022
1 parent bf808b3 commit 1355dbc
Show file tree
Hide file tree
Showing 13 changed files with 163 additions and 31 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
protected def refinementNameString(tp: RefinedType): String = nameString(tp.refinedName)

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

protected def argText(arg: Type): Text = homogenizeArg(arg) match {
Expand Down
29 changes: 22 additions & 7 deletions compiler/src/dotty/tools/dotc/printing/Texts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@ object Texts {
case Vertical(relems) => relems.isEmpty
}

// Str Ver Clo Flu
// isVertical F T F F
// isClosed F T T F
// isFluid F F T T
// isSplittable F F F T
def isVertical: Boolean = isInstanceOf[Vertical]
def isClosed: Boolean = isVertical || isInstanceOf[Closed]
def isFluid: Boolean = isInstanceOf[Fluid]
def isSplittable: Boolean = isFluid && !isClosed

def close: Closed = new Closed(relems)
def close: Text = if isSplittable then Closed(relems) else this

def remaining(width: Int): Int = this match {
case Str(s, _) =>
Expand Down Expand Up @@ -53,7 +58,7 @@ object Texts {
}

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

private def append(width: Int)(that: Text): Text =
if (this.isEmpty) that.layout(width)
Expand Down Expand Up @@ -113,7 +118,7 @@ object Texts {
sb.append("|")
}
}
sb.append(s)
sb.append(s.replaceAll("[ ]+$", ""))
case _ =>
var follow = false
for (elem <- relems.reverse) {
Expand All @@ -138,7 +143,13 @@ object Texts {
def ~ (that: Text): Text =
if (this.isEmpty) that
else if (that.isEmpty) this
else Fluid(that :: this :: Nil)
else this match
case Fluid(relems1) if !isClosed => that match
case Fluid(relems2) if !that.isClosed => Fluid(relems2 ++ relems1)
case _ => Fluid(that +: relems1)
case _ => that match
case Fluid(relems2) if !that.isClosed => Fluid(relems2 :+ this)
case _ => Fluid(that :: this :: Nil)

def ~~ (that: Text): Text =
if (this.isEmpty) that
Expand All @@ -161,9 +172,9 @@ object Texts {
def apply(xs: Traversable[Text], sep: String = " "): Text =
if (sep == "\n") lines(xs)
else {
val ys = xs filterNot (_.isEmpty)
val ys = xs.filterNot(_.isEmpty)
if (ys.isEmpty) Str("")
else ys reduce (_ ~ sep ~ _)
else ys.reduceRight((a, b) => (a ~ sep).close ~ b)
}

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

case class Str(s: String, lineRange: LineRange = EmptyLineRange) extends Text {
override def relems: List[Text] = List(this)
override def toString = this match
case Str(s, EmptyLineRange) => s"Str($s)"
case Str(s, lineRange) => s"Str($s, $lineRange)"
}

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

class Closed(relems: List[Text]) extends Fluid(relems)
class Closed(relems: List[Text]) extends Fluid(relems):
override def productPrefix = "Closed"

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

Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ object ErrorReporting {
|The tests were made under $constraintText"""

def whyFailedStr(fail: FailedExtension) =
i""" failed with
i"""
|
| failed with:
|
|${fail.whyFailed.message.indented(8)}"""

Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,9 @@ object Implicits:
if reasons.length > 1 then
reasons.mkString("\n\t* ", "\n\t* ", "")
else
reasons.mkString
reasons.mkString(" ", "", "")

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

end Implicits

Expand Down
90 changes: 90 additions & 0 deletions compiler/test/dotty/tools/dotc/TupleShowTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package dotty.tools
package dotc

import core.*, Decorators.*, Symbols.*
import printing.Texts.*

import org.junit.Test

class TupleShowTests extends DottyTest:
def IntType = defn.IntType
def LongType = defn.LongType
def ShortType = defn.ShortType
def Types_10 = List.fill(5)(IntType) ::: List.fill(5)(LongType)
def Types_20 = Types_10 ::: Types_10

val tup0 = defn.tupleType(Nil)
val tup1 = defn.tupleType(IntType :: Nil)
val tup2 = defn.tupleType(IntType :: LongType :: Nil)
val tup3 = defn.tupleType(IntType :: LongType :: ShortType :: Nil)
val tup21 = defn.tupleType(Types_20 ::: IntType :: Nil)
val tup22 = defn.tupleType(Types_20 ::: IntType :: LongType :: Nil)
val tup23 = defn.tupleType(Types_20 ::: IntType :: LongType :: ShortType :: Nil)
val tup24 = defn.tupleType(Types_20 ::: IntType :: LongType :: ShortType :: ShortType :: Nil)

@Test def tup0_show = chkEq("EmptyTuple.type", i"$tup0")
@Test def tup1_show = chkEq("Tuple1[Int]", i"$tup1")
@Test def tup2_show = chkEq("(Int, Long)", i"$tup2")
@Test def tup3_show = chkEq("(Int, Long, Short)", i"$tup3")
@Test def tup21_show = chkEq(res21, i"$tup21")
@Test def tup22_show = chkEq(res22, i"$tup22")
@Test def tup23_show = chkEq(res23, i"$tup23")
@Test def tup24_show = chkEq(res24, i"$tup24")

@Test def tup3_text =
val obt = tup3.toText(ctx.printer)
val exp = Fluid(List(
Str(")"),
Str("Short"),
Closed(List(Str(", "), Str("Long"))),
Closed(List(Str(", "), Str("Int"))),
Str("("),
))
chkEq(exp, obt)

@Test def tup3_layout10 =
val obt = tup3.toText(ctx.printer).layout(10)
val exp = Fluid(List(
Str(" Short)"),
Str(" Long, "),
Str("(Int, "),
))
chkEq(exp, obt)

@Test def tup3_show10 = chkEq("(Int,\n Long,\n Short)", tup3.toText(ctx.printer).mkString(10, false))

val res21 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
| Int, Long, Long, Long, Long, Long, Int)""".stripMargin

val res22 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
| Int, Long, Long, Long, Long, Long, Int, Long)""".stripMargin

val res23 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
| Int, Long, Long, Long, Long, Long, Int, Long, Short)""".stripMargin

val res24 = """|(Int, Int, Int, Int, Int, Long, Long, Long, Long, Long, Int, Int, Int, Int,
| Int, Long, Long, Long, Long, Long, Int, Long, Short, Short)""".stripMargin

def chkEq[A](expected: A, obtained: A) = assert(expected == obtained, diff(s"$expected", s"$obtained"))

def diff(exp: String, obt: String) =
val min = math.min(exp.length, obt.length)
val pre =
var i = 0
while i < min && exp(i) == obt(i) do i += 1
exp.substring(0, i)
val suf =
val max = min - pre.length - 1
var i = 0
while i <= max && exp(exp.length - 1 - i) == obt(obt.length - 1 - i) do i += 1
exp.substring(exp.length - i)

import scala.io.AnsiColor.*
val ellip = BLACK + BOLD + "..." + RESET
val compactPre = if pre.length <= 20 then pre else ellip + pre.substring(pre.length - 20)
val compactSuf = if suf.length <= 20 then suf else suf.substring(0, 20) + ellip
def extractDiff(s: String) = s.substring(pre.length, s.length - suf.length)
s"""|Comparison Failure:
| expected: $compactPre${CYAN }${extractDiff(exp)}$RESET$compactSuf
| obtained: $compactPre$MAGENTA${extractDiff(obt)}$RESET$compactSuf
|""".stripMargin
16 changes: 12 additions & 4 deletions tests/neg/enum-values.check
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
| meaning a values array is not defined.
| An extension method was tried, but could not be fully constructed:
|
| example.Extensions.values(Tag) failed with
| example.Extensions.values(Tag)
|
| failed with:
|
| Found: example.Tag.type
| Required: Nothing
Expand All @@ -18,7 +20,9 @@
| meaning a values array is not defined.
| An extension method was tried, but could not be fully constructed:
|
| example.Extensions.values(ListLike) failed with
| example.Extensions.values(ListLike)
|
| failed with:
|
| Found: Array[example.Tag[?]]
| Required: Array[example.ListLike[?]]
Expand All @@ -30,7 +34,9 @@
| meaning a values array is not defined.
| An extension method was tried, but could not be fully constructed:
|
| example.Extensions.values(TypeCtorsK) failed with
| example.Extensions.values(TypeCtorsK)
|
| failed with:
|
| Found: Array[example.Tag[?]]
| Required: Array[example.TypeCtorsK[?[_$1]]]
Expand Down Expand Up @@ -63,7 +69,9 @@
| value values is not a member of object example.NotAnEnum.
| An extension method was tried, but could not be fully constructed:
|
| example.Extensions.values(NotAnEnum) failed with
| example.Extensions.values(NotAnEnum)
|
| failed with:
|
| Found: example.NotAnEnum.type
| Required: Nothing
Expand Down
12 changes: 9 additions & 3 deletions tests/neg/i10901.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
| value º is not a member of object BugExp4Point2D.IntT.
| An extension method was tried, but could not be fully constructed:
|
| º(x) failed with
| º(x)
|
| failed with:
|
| Ambiguous overload. The overloaded alternatives of method º in object dsl with types
| [T1, T2]
Expand All @@ -22,7 +24,9 @@
|value º is not a member of object BugExp4Point2D.IntT.
|An extension method was tried, but could not be fully constructed:
|
| º(x) failed with
| º(x)
|
| failed with:
|
| Ambiguous overload. The overloaded alternatives of method º in object dsl with types
| [T1, T2]
Expand All @@ -36,6 +40,8 @@
| value foo is not a member of String.
| An extension method was tried, but could not be fully constructed:
|
| Test.foo("abc")(/* missing */summon[C]) failed with
| Test.foo("abc")(/* missing */summon[C])
|
| failed with:
|
| No given instance of type C was found for parameter x$2 of method foo in object Test
8 changes: 6 additions & 2 deletions tests/neg/i13558.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
| value id is not a member of testcode.A.
| An extension method was tried, but could not be fully constructed:
|
| testcode.ExtensionA.id(a) failed with
| testcode.ExtensionA.id(a)
|
| failed with:
|
| Reference to id is ambiguous,
| it is both imported by import testcode.ExtensionB._
Expand All @@ -15,7 +17,9 @@
| value id is not a member of testcode.A.
| An extension method was tried, but could not be fully constructed:
|
| testcode.ExtensionB.id(a) failed with
| testcode.ExtensionB.id(a)
|
| failed with:
|
| Reference to id is ambiguous,
| it is both imported by import testcode.ExtensionA._
Expand Down
8 changes: 3 additions & 5 deletions tests/neg/i14127.check
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
-- Error: tests/neg/i14127.scala:6:55 ----------------------------------------------------------------------------------
6 | *: Int *: Int *: Int *: Int *: Int *: EmptyTuple)]] // error
| ^
|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,
| 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,
| Int
|, Int, Int)]:
|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,
| 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,
| Int, Int, Int)]:
| * class *: is not a generic product because it reduces to a tuple with arity 23, expected arity <= 22
| * class *: is not a generic sum because it does not have subclasses
4 changes: 3 additions & 1 deletion tests/neg/i15000.check
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
|value apply is not a member of object ExtensionMethodReproduction.c.
|An extension method was tried, but could not be fully constructed:
|
| apply(ExtensionMethodReproduction.c) failed with
| apply(ExtensionMethodReproduction.c)
|
| failed with:
|
| Ambiguous overload. The overloaded alternatives of method apply in object ExtensionMethodReproduction with types
| (c: ExtensionMethodReproduction.C)(x: Int, y: Int): String
Expand Down
4 changes: 3 additions & 1 deletion tests/neg/i6183.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
| value render is not a member of Int.
| An extension method was tried, but could not be fully constructed:
|
| render(42) failed with
| render(42)
|
| failed with:
|
| Ambiguous overload. The overloaded alternatives of method render in object Test with types
| [B](b: B)(using x$2: DummyImplicit): Char
Expand Down
4 changes: 3 additions & 1 deletion tests/neg/i6779.check
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
| value f is not a member of T.
| An extension method was tried, but could not be fully constructed:
|
| Test.f[G[T]](x)(given_Stuff) failed with
| Test.f[G[T]](x)(given_Stuff)
|
| failed with:
|
| Found: (x : T)
| Required: G[T]
Expand Down
9 changes: 6 additions & 3 deletions tests/neg/i9185.check
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
|An extension method was tried, but could not be fully constructed:
|
| M.pure[A, F]("ola")(
| /* ambiguous: both object listMonad in object M and object optionMonad in object M match type M[F] */summon[M[F]]
| ) failed with
| /* ambiguous: both object listMonad in object M and object optionMonad in object M match type M[F] */summon[M[F]])
|
| failed with:
|
| 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
-- Error: tests/neg/i9185.scala:8:28 -----------------------------------------------------------------------------------
Expand All @@ -19,7 +20,9 @@
| value len is not a member of String.
| An extension method was tried, but could not be fully constructed:
|
| M.len("abc") failed with
| M.len("abc")
|
| failed with:
|
| Found: ("abc" : String)
| Required: Int

0 comments on commit 1355dbc

Please sign in to comment.