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 9852984
Show file tree
Hide file tree
Showing 29 changed files with 255 additions and 154 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.take(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.drop(exp.length - 1)

import scala.io.AnsiColor.*
val ellip = BLACK + BOLD + "..." + RESET
val compactPre = if pre.length <= 20 then pre else ellip + pre.drop(pre.length - 20)
val compactSuf = if suf.length <= 20 then suf else suf.take(20) + ellip
def extractDiff(s: String) = s.slice(pre.length, s.length - suf.length)
s"""|Comparison Failure:
| expected: $compactPre${CYAN }${extractDiff(exp)}$RESET$compactSuf
| obtained: $compactPre$MAGENTA${extractDiff(obt)}$RESET$compactSuf
|""".stripMargin
3 changes: 2 additions & 1 deletion compiler/test/dotty/tools/dotc/printing/PrintingTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import scala.io.Source
import org.junit.Test
import scala.util.Using
import java.io.File

class PrintingTest {

def options(phase: String, flags: List[String]) =
Expand All @@ -45,7 +46,7 @@ class PrintingTest {
}

val actualLines = byteStream.toString(StandardCharsets.UTF_8.name).linesIterator
FileDiff.checkAndDump(path.toString, actualLines.toIndexedSeq, checkFilePath)
FileDiff.checkAndDumpOrUpdate(path.toString, actualLines.toIndexedSeq, checkFilePath)
}

def testIn(testsDir: String, phase: String) =
Expand Down
15 changes: 0 additions & 15 deletions compiler/test/dotty/tools/vulpix/FileDiff.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,6 @@ object FileDiff {
outFile.writeAll(content.mkString("", EOL, EOL))
}

def checkAndDump(sourceTitle: String, actualLines: Seq[String], checkFilePath: String): Boolean = {
val outFilePath = checkFilePath + ".out"
FileDiff.check(sourceTitle, actualLines, checkFilePath) match {
case Some(msg) =>
FileDiff.dump(outFilePath, actualLines)
println(msg)
println(FileDiff.diffMessage(checkFilePath, outFilePath))
false
case _ =>
val jOutFilePath = Paths.get(outFilePath)
Files.deleteIfExists(jOutFilePath)
true
}
}

def checkAndDumpOrUpdate(sourceTitle: String, actualLines: Seq[String], checkFilePath: String): Boolean = {
val outFilePath = checkFilePath + ".out"
FileDiff.check(sourceTitle, actualLines, checkFilePath) match {
Expand Down
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
2 changes: 1 addition & 1 deletion tests/neg/i14432.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-- Error: tests/neg/i14432.scala:13:33 ---------------------------------------------------------------------------------
13 |val mFoo = summon[Mirror.Of[Foo]] // error: no mirror found
| ^
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
| * class Foo is not a generic product because the constructor of class Foo is innaccessible from the calling scope.
| * class Foo is not a generic sum because it is not a sealed class
2 changes: 1 addition & 1 deletion tests/neg/i14432a.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-- Error: tests/neg/i14432a.scala:14:43 --------------------------------------------------------------------------------
14 | val mFoo = summon[Mirror.Of[example.Foo]] // error: no mirror found
| ^
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
| * class Foo is not a generic product because the constructor of class Foo is innaccessible from the calling scope.
| * class Foo is not a generic sum because it is not a sealed class
2 changes: 1 addition & 1 deletion tests/neg/i14432b.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-- Error: tests/neg/i14432b.scala:15:43 --------------------------------------------------------------------------------
15 | val mFoo = summon[Mirror.Of[example.Foo]] // error: no mirror found
| ^
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
| * class Foo is not a generic product because the constructor of class Foo is innaccessible from the calling scope.
| * class Foo is not a generic sum because it is not a sealed class
2 changes: 1 addition & 1 deletion tests/neg/i14432c.check
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
-- Error: tests/neg/i14432c.scala:16:43 --------------------------------------------------------------------------------
16 | val mFoo = summon[Mirror.Of[example.Foo]] // error: no mirror
| ^
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
| * class Foo is not a generic product because the constructor of class Foo is innaccessible from the calling scope.
| * class Foo is not a generic sum because it is not a sealed class
2 changes: 1 addition & 1 deletion tests/neg/i14432d.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-- Error: tests/neg/i14432d.scala:17:45 --------------------------------------------------------------------------------
17 | val mFoo = summon[Mirror.Of[example.Foo]] // error
| ^
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
|No given instance of type deriving.Mirror.Of[example.Foo] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[example.Foo]:
| * class Foo is not a generic product because the constructor of class Foo is innaccessible from the calling scope.
| * class Foo is not a generic sum because it is not a sealed class
Loading

0 comments on commit 9852984

Please sign in to comment.