Merge pull request #4072 from dotty-staging/fix-#4068

Blaisorblade · web-flow · commit da2a245bfe4f · 2018-08-21T04:21:03.000+02:00
Pretty-printing: handle type-operator precedence correctly
diff --git a/compiler/src/dotty/tools/dotc/parsing/package.scala b/compiler/src/dotty/tools/dotc/parsing/package.scala
@@ -7,6 +7,13 @@ import core.NameOps._
 
 package object parsing {
 
+  /**
+   * Compute the precedence of infix operator `operator` according to the SLS [§ 6.12.3][SLS].
+   * We implement [SIP-33][SIP-33] and give type operators the same precedence as term operators.
+   *
+   * [SLS]: https://www.scala-lang.org/files/archive/spec/2.13/06-expressions.html#infix-operations
+   * [SIP-33]: https://docs.scala-lang.org/sips/priority-based-infix-type-precedence.html
+   */
   def precedence(operator: Name): Int =
     if (operator eq nme.ERROR) -1
     else {
diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
@@ -102,10 +102,15 @@ class PlainPrinter(_ctx: Context) extends Printer {
     (refinementNameString(rt) ~ toTextRHS(rt.refinedInfo)).close
 
   protected def argText(arg: Type): Text = homogenizeArg(arg) match {
-    case arg: TypeBounds => "_" ~ toTextGlobal(arg)
-    case arg => toTextGlobal(arg)
+    case arg: TypeBounds => "_" ~ toText(arg)
+    case arg => toText(arg)
   }
 
+  /** Pretty-print comma-separated type arguments for a constructor to be inserted among parentheses or brackets
+    * (hence with `GlobalPrec` precedence).
+    */
+  protected def argsText(args: List[Type]): Text = atPrec(GlobalPrec) { Text(args.map(arg => argText(arg) ), ", ") }
+
   /** The longest sequence of refinement types, starting at given type
    *  and following parents.
    */
@@ -144,7 +149,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
       case tp: SingletonType =>
         toTextLocal(tp.underlying) ~ "(" ~ toTextRef(tp) ~ ")"
       case AppliedType(tycon, args) =>
-        (toTextLocal(tycon) ~ "[" ~ Text(args map argText, ", ") ~ "]").close
+        (toTextLocal(tycon) ~ "[" ~ argsText(args) ~ "]").close
       case tp: RefinedType =>
         val parent :: (refined: List[RefinedType @unchecked]) =
           refinementChain(tp).reverse
@@ -156,9 +161,9 @@ class PlainPrinter(_ctx: Context) extends Printer {
         }
         finally openRecs = openRecs.tail
       case AndType(tp1, tp2) =>
-        changePrec(AndPrec) { toText(tp1) ~ " & " ~ toText(tp2) }
+        changePrec(AndTypePrec) { toText(tp1) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(tp2) } }
       case OrType(tp1, tp2) =>
-        changePrec(OrPrec) { toText(tp1) ~ " | " ~ toText(tp2) }
+        changePrec(OrTypePrec) { toText(tp1) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(tp2) } }
       case tp: ErrorType =>
         s"<error ${tp.msg.msg}>"
       case tp: WildcardType =>
diff --git a/compiler/src/dotty/tools/dotc/printing/Printer.scala b/compiler/src/dotty/tools/dotc/printing/Printer.scala
@@ -17,10 +17,24 @@ abstract class Printer {
 
   private[this] var prec: Precedence = GlobalPrec
 
-  /** The current precedence level */
+  /** The current precedence level.
+   *  When pretty-printing arguments of operator `op`, `currentPrecedence` must equal `op`'s precedence level,
+   *  so that pretty-printing expressions using lower-precedence operators can insert parentheses automatically
+   *  by calling `changePrec`.
+   */
   def currentPrecedence = prec
 
-  /** Generate text using `op`, assuming a given precedence level `prec`. */
+  /** Generate text using `op`, assuming a given precedence level `prec`.
+   *
+   *  ### `atPrec` vs `changePrec`
+   *
+   *  This is to be used when changing precedence inside some sort of parentheses:
+   *  for instance, to print T[A]` use
+   *  `toText(T) ~ '[' ~ atPrec(GlobalPrec) { toText(A) } ~ ']'`.
+   *
+   *  If the presence of the parentheses depends on precedence, inserting them manually is most certainly a bug.
+   *  Use `changePrec` instead to generate them exactly when needed.
+   */
   def atPrec(prec: Precedence)(op: => Text): Text = {
     val outerPrec = this.prec
     this.prec = prec
@@ -30,6 +44,27 @@ abstract class Printer {
 
   /** Generate text using `op`, assuming a given precedence level `prec`.
    *  If new level `prec` is lower than previous level, put text in parentheses.
+   *
+   *  ### `atPrec` vs `changePrec`
+   *
+   *  To pretty-print `A op B`, you need something like
+   *  `changePrec(parsing.precedence(op, isType)) { toText(a) ~ op ~ toText(b) }` // BUGGY
+   *  that will insert parentheses around `A op B` if, for instance, the
+   *  preceding operator has higher precedence.
+   *
+   *  But that does not handle infix operators with left- or right- associativity.
+   *
+   *  If op and op' have the same precedence and associativity,
+   *  A op B op' C parses as (A op B) op' C if op and op' are left-associative, and as
+   *  A op (B op' C) if they're right-associative, so we need respectively
+   *  ```scala
+   *  val isType = ??? // is this a term or type operator?
+   *  val prec = parsing.precedence(op, isType)
+   *  // either:
+   *  changePrec(prec) { toText(a) ~ op ~ atPrec(prec + 1) { toText(b) } } // for left-associative op and op'
+   *  // or:
+   *  changePrec(prec) { atPrec(prec + 1) { toText(a) } ~ op ~ toText(b) } // for right-associative op and op'
+   *  ```
    */
   def changePrec(prec: Precedence)(op: => Text): Text =
     if (prec < this.prec) atPrec(prec) ("(" ~ op ~ ")") else atPrec(prec)(op)
diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
@@ -130,7 +130,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
 
   override def toText(tp: Type): Text = controlled {
     def toTextTuple(args: List[Type]): Text =
-      "(" ~ Text(args.map(argText), ", ") ~ ")"
+      "(" ~ argsText(args) ~ ")"
 
     def toTextFunction(args: List[Type], isImplicit: Boolean, isErased: Boolean): Text =
       changePrec(GlobalPrec) {
@@ -155,17 +155,24 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
       case _ => false
     }
 
-    def toTextInfixType(op: Type, args: List[Type]): Text = {
-      /* SLS 3.2.8: all infix types have the same precedence.
-       * In A op B op' C, op and op' need the same associativity.
-       * Therefore, if op is left associative, anything on its right
-       * needs to be parenthesized if it's an infix type, and vice versa. */
-      val l :: r :: Nil = args
-      val isRightAssoc = op.typeSymbol.name.endsWith(":")
-      val leftArg = if (isRightAssoc && isInfixType(l)) "(" ~ argText(l) ~ ")" else argText(l)
-      val rightArg = if (!isRightAssoc && isInfixType(r)) "(" ~ argText(r) ~ ")" else argText(r)
-
-      leftArg ~ " " ~ simpleNameString(op.classSymbol) ~ " " ~ rightArg
+    def tyconName(tp: Type): Name = tp.typeSymbol.name
+    def checkAssocMismatch(tp: Type, isRightAssoc: Boolean) = tp match {
+      case AppliedType(tycon, _) => isInfixType(tp) && tyconName(tycon).endsWith(":") != isRightAssoc
+      case AndType(_, _) => isRightAssoc
+      case OrType(_, _) => isRightAssoc
+      case _ => false
+    }
+
+    def toTextInfixType(opName: Name, l: Type, r: Type)(op: => Text): Text = {
+      val isRightAssoc = opName.endsWith(":")
+      val opPrec = parsing.precedence(opName)
+
+      changePrec(opPrec) {
+        val leftPrec = if (isRightAssoc || checkAssocMismatch(l, isRightAssoc)) opPrec + 1 else opPrec
+        val rightPrec = if (!isRightAssoc || checkAssocMismatch(r, isRightAssoc)) opPrec + 1 else opPrec
+
+        atPrec(leftPrec) { argText(l) }  ~ " " ~ op ~ " " ~ atPrec(rightPrec) { argText(r) }
+      }
     }
 
     homogenize(tp) match {
@@ -174,7 +181,20 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
         if (tycon.isRepeatedParam) return toTextLocal(args.head) ~ "*"
         if (defn.isFunctionClass(cls)) return toTextFunction(args, cls.name.isImplicitFunction, cls.name.isErasedFunction)
         if (defn.isTupleClass(cls)) return toTextTuple(args)
-        if (isInfixType(tp)) return toTextInfixType(tycon, args)
+        if (isInfixType(tp)) {
+          val l :: r :: Nil = args
+          val opName = tyconName(tycon)
+
+          return toTextInfixType(tyconName(tycon), l, r) { simpleNameString(tycon.typeSymbol) }
+        }
+
+      // Since RefinedPrinter, unlike PlainPrinter, can output right-associative type-operators, we must override handling
+      // of AndType and OrType to account for associativity
+      case AndType(tp1, tp2) =>
+        return toTextInfixType(tpnme.raw.AMP, tp1, tp2) { toText(tpnme.raw.AMP) }
+      case OrType(tp1, tp2) =>
+        return toTextInfixType(tpnme.raw.BAR, tp1, tp2) { toText(tpnme.raw.BAR) }
+
       case EtaExpansion(tycon) =>
         return toText(tycon)
       case tp: RefinedType if defn.isFunctionType(tp) =>
@@ -201,7 +221,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
         // (they don't need to because we keep the original type tree with
         //  the original annotation anyway. Therefore, there will always be
         //  one version of the annotation tree that has the correct positions).
-        withoutPos(super.toText(tp))
+        return withoutPos(super.toText(tp))
       case tp: SelectionProto =>
         return "?{ " ~ toText(tp.name) ~
            (Str(" ") provided !tp.name.toSimpleName.last.isLetterOrDigit) ~
@@ -375,9 +395,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
       case SingletonTypeTree(ref) =>
         toTextLocal(ref) ~ "." ~ keywordStr("type")
       case AndTypeTree(l, r) =>
-        changePrec(AndPrec) { toText(l) ~ " & " ~ toText(r) }
+        changePrec(AndTypePrec) { toText(l) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(r) } }
       case OrTypeTree(l, r) =>
-        changePrec(OrPrec) { toText(l) ~ " | " ~ toText(r) }
+        changePrec(OrTypePrec) { toText(l) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(r) } }
       case RefinedTypeTree(tpt, refines) =>
         toTextLocal(tpt) ~ " " ~ blockText(refines)
       case AppliedTypeTree(tpt, args) =>
diff --git a/compiler/src/dotty/tools/dotc/printing/Texts.scala b/compiler/src/dotty/tools/dotc/printing/Texts.scala
@@ -4,7 +4,7 @@ import language.implicitConversions
 
 object Texts {
 
-  abstract class Text {
+  sealed abstract class Text {
 
     protected def indentMargin = 2
 
@@ -46,9 +46,11 @@ object Texts {
           case Str(s1, lines2) => Str(s1 + s2, lines1 union lines2)
           case Fluid(Str(s1, lines2) :: prev) => Fluid(Str(s1 + s2, lines1 union lines2) :: prev)
           case Fluid(relems) => Fluid(that :: relems)
+          case Vertical(_) => throw new IllegalArgumentException("Unexpected Vertical.appendToLastLine")
         }
       case Fluid(relems) =>
         (this /: relems.reverse)(_ appendToLastLine _)
+      case Vertical(_) => throw new IllegalArgumentException("Unexpected Text.appendToLastLine(Vertical(...))")
     }
 
     private def appendIndented(that: Text)(width: Int): Text =
diff --git a/compiler/src/dotty/tools/dotc/printing/package.scala b/compiler/src/dotty/tools/dotc/printing/package.scala
@@ -1,6 +1,6 @@
 package dotty.tools.dotc
 
-import core.StdNames.nme
+import core.StdNames.{nme,tpnme}
 import parsing.{precedence, minPrec, maxPrec, minInfixPrec}
 import util.Property.Key
 
@@ -9,7 +9,8 @@ package object printing {
   type Precedence = Int
 
   val DotPrec       = parsing.maxPrec
-  val AndPrec       = parsing.precedence(nme.raw.AMP)
+  val AndTypePrec   = parsing.precedence(tpnme.raw.AMP)
+  val OrTypePrec    = parsing.precedence(tpnme.raw.BAR)
   val OrPrec        = parsing.precedence(nme.raw.BAR)
   val InfixPrec     = parsing.minInfixPrec
   val GlobalPrec    = parsing.minPrec
diff --git a/compiler/test-resources/type-printer/functions b/compiler/test-resources/type-printer/functions
@@ -1,2 +1,10 @@
-scala> val toInt: Any => Int = new { def apply(a: Any) = 1; override def toString() = "<func1>" }
-val toInt: Any => Int = <func1>
+scala> def toInt: Any => Int = ???
+def toInt: Any => Int
+scala> def hoFun: (Int => Int) => Int = ???
+def hoFun: (Int => Int) => Int
+scala> def curriedFun: Int => (Int => Int) = ???
+def curriedFun: Int => Int => Int
+scala> def tupFun: ((Int, Int)) => Int = ???
+def tupFun: ((Int, Int)) => Int
+scala> def binFun: (Int, Int) => Int = ???
+def binFun: (Int, Int) => Int
diff --git a/compiler/test-resources/type-printer/infix b/compiler/test-resources/type-printer/infix
@@ -16,12 +16,46 @@ scala> def foo: (Int && String) &: Boolean = ???
 def foo: (Int && String) &: Boolean
 scala> def foo: Int && (Boolean &: String) = ???
 def foo: Int && (Boolean &: String)
+scala> def foo: (Int &: String) && Boolean = ???
+def foo: (Int &: String) && Boolean
+scala> def foo: Int &: (Boolean && String) = ???
+def foo: Int &: (Boolean && String)
+scala> def foo: (Int & String) &: Boolean = ???
+def foo: (Int & String) &: Boolean
+scala> def foo: Int & (Boolean &: String) = ???
+def foo: Int & (Boolean &: String)
+scala> def foo: (Int &: String) & Boolean = ???
+def foo: (Int &: String) & Boolean
+scala> def foo: Int &: (Boolean & String) = ???
+def foo: Int &: (Boolean & String)
 scala> import scala.annotation.showAsInfix
 scala> @scala.annotation.showAsInfix class Mappy[T,U]
 // defined class Mappy
+scala> def foo: (Int Mappy Boolean) && String = ???
+def foo: (Int Mappy Boolean) && String
+scala> def foo: Int Mappy Boolean && String = ???
+def foo: Int Mappy Boolean && String
 scala> def foo: Int Mappy (Boolean && String) = ???
-def foo: Int Mappy (Boolean && String)
+def foo: Int Mappy Boolean && String
 scala> @scala.annotation.showAsInfix(false) class ||[T,U]
 // defined class ||
 scala> def foo: Int || Boolean = ???
 def foo: ||[Int, Boolean]
+scala> def foo: Int && Boolean & String = ???
+def foo: Int && Boolean & String
+scala> def foo: (Int && Boolean) & String = ???
+def foo: Int && Boolean & String
+scala> def foo: Int && (Boolean & String) = ???
+def foo: Int && (Boolean & String)
+scala> def foo: Int && (Boolean with String) = ???
+def foo: Int && (Boolean & String)
+scala> def foo: (Int && Boolean) with String = ???
+def foo: Int && Boolean & String
+scala> def foo: Int && Boolean with String = ???
+def foo: Int && (Boolean & String)
+scala> def foo: Int && Boolean | String = ???
+def foo: Int && Boolean | String
+scala> def foo: Int && (Boolean | String) = ???
+def foo: Int && (Boolean | String)
+scala> def foo: (Int && Boolean) | String = ???
+def foo: Int && Boolean | String
diff --git a/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala b/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala
@@ -1,13 +1,20 @@
 package dotty.tools.dotc.printing
 
 import dotty.tools.DottyTest
-import dotty.tools.dotc.ast.tpd
+import dotty.tools.dotc.ast.{Trees,tpd}
 import dotty.tools.dotc.core.Names._
 import dotty.tools.dotc.core.Symbols._
+import dotty.tools.dotc.core.Decorators._
 import org.junit.Assert.assertEquals
 import org.junit.Test
 
 class PrinterTests extends DottyTest {
+
+  private def newContext = {
+    initialCtx.setSetting(ctx.settings.color, "never")
+  }
+  ctx = newContext
+
   import tpd._
 
   @Test
@@ -24,4 +31,21 @@ class PrinterTests extends DottyTest {
       assertEquals("package object foo", bar.symbol.owner.show)
     }
   }
+
+  @Test
+  def tpTreeInfixOps: Unit = {
+    val source = """
+      |class &&[T,U]
+      |object Foo {
+      |  def bar1: Int && (Boolean | String) = ???
+      |  def bar2: Int & (Boolean | String) = ???
+      |}
+    """.stripMargin
+
+    checkCompile("frontend", source) { (tree, context) =>
+      implicit val ctx = context
+      val bar @ Trees.DefDef(_, _, _, _, _) = tree.find(tree => tree.symbol.name == termName("bar2")).get
+      assertEquals("Int & (Boolean | String)", bar.tpt.show)
+    }
+  }
 }
diff --git a/tests/patmat/andtype-opentype-interaction.check b/tests/patmat/andtype-opentype-interaction.check
@@ -3,4 +3,4 @@
 31: Pattern Match Exhaustivity: _: SealedTrait & OpenClass, _: Trait & OpenClass
 35: Pattern Match Exhaustivity: _: SealedTrait & OpenTrait & OpenClass, _: Trait & OpenTrait & OpenClass
 43: Pattern Match Exhaustivity: _: SealedTrait & OpenAbstractClass, _: Trait & OpenAbstractClass
-47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & OpenTrait & OpenClassSubclass, _: Trait & OpenClass & OpenTrait & OpenClassSubclass
+47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & (OpenTrait & OpenClassSubclass), _: Trait & OpenClass & (OpenTrait & OpenClassSubclass)
diff --git a/tests/patmat/andtype-refinedtype-interaction.check b/tests/patmat/andtype-refinedtype-interaction.check
@@ -1,6 +1,6 @@
 32: Pattern Match Exhaustivity: _: Trait & C1{x: Int}
-48: Pattern Match Exhaustivity: _: Clazz & (C1 | C2 | T1){x: Int} & (C3 | C4 | T2){x: Int}, _: Trait & (C1 | C2 | T1){x: Int} & (C3 | C4 | T2){x: Int}
-54: Pattern Match Exhaustivity: _: Trait & (C1 | C2 | T1){x: Int} & C3{x: Int}
+48: Pattern Match Exhaustivity: _: Clazz & (C1 | (C2 | T1)){x: Int} & (C3 | (C4 | T2)){x: Int}, _: Trait & (C1 | (C2 | T1)){x: Int} & (C3 | (C4 | T2)){x: Int}
+54: Pattern Match Exhaustivity: _: Trait & (C1 | (C2 | T1)){x: Int} & C3{x: Int}
 65: Pattern Match Exhaustivity: _: Trait & (C1 | C2){x: Int} & (C3 | SubC1){x: Int}
 72: Pattern Match Exhaustivity: _: Trait & (T1 & (C1 | SubC2)){x: Int} & (T2 & (C2 | C3 | SubC1)){x: Int} &
   SubSubC1{x: Int}
