Pretty-printing: handle type-operator precedence correctly

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)

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala

@@ -155,17 +155,24 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
       case _ => false
     }
 
-    def toTextInfixType(op: Type, args: List[Type]): Text = changePrec(InfixPrec) {
-      /* 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) atPrec(InfixPrec + 1) { argText(l) } else argText(l)
-      val rightArg = if (!isRightAssoc) atPrec(InfixPrec + 1) { 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, true)
+
+      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.classSymbol) }
+        }
+
+      // 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) =>

compiler/test-resources/type-printer/infix

@@ -16,20 +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 with 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) & String = ???
-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