Merge pull request #9451 from som-snytt/issue/12276

REPL: improve handling of non-printable characters (to prevent messing up terminal)

src/library/scala/util/Properties.scala

@@ -53,18 +53,18 @@ private[scala] trait PropertiesTrait {
 
   def propIsSet(name: String)                   = System.getProperty(name) != null
   def propIsSetTo(name: String, value: String)  = propOrNull(name) == value
-  def propOrElse(name: String, alt: => String)     = Option(System.getProperty(name)).getOrElse(alt)
+  def propOrElse(name: String, alt: => String)  = Option(System.getProperty(name)).getOrElse(alt)
   def propOrEmpty(name: String)                 = propOrElse(name, "")
   def propOrNull(name: String)                  = propOrElse(name, null)
   def propOrNone(name: String)                  = Option(propOrNull(name))
   def propOrFalse(name: String)                 = propOrNone(name) exists (x => List("yes", "on", "true") contains x.toLowerCase)
   def setProp(name: String, value: String)      = System.setProperty(name, value)
   def clearProp(name: String)                   = System.clearProperty(name)
 
-  def envOrElse(name: String, alt: => String)      = Option(System getenv name) getOrElse alt
+  def envOrElse(name: String, alt: => String)   = Option(System getenv name) getOrElse alt
   def envOrNone(name: String)                   = Option(System getenv name)
 
-  def envOrSome(name: String, alt: => Option[String])       = envOrNone(name) orElse alt
+  def envOrSome(name: String, alt: => Option[String])    = envOrNone(name) orElse alt
 
   // for values based on propFilename, falling back to System properties
   def scalaPropOrElse(name: String, alt: => String): String = scalaPropOrNone(name).getOrElse(alt)

src/partest/scala/tools/partest/ReplTest.scala

@@ -13,7 +13,7 @@
 package scala.tools.partest
 
 import scala.tools.nsc.Settings
-import scala.tools.nsc.interpreter.shell.ILoop
+import scala.tools.nsc.interpreter.shell.{ILoop, ShellConfig}
 import scala.util.matching.Regex
 import scala.util.matching.Regex.Match
 
@@ -41,20 +41,24 @@ abstract class ReplTest extends DirectTest {
     }
     transformSettings(s)
   }
-  def normalize(s: String) = s
+  /** Transform a line of output, for comparison to expected output. */
+  protected def normalize(s: String): String = s
   /** True for SessionTest to preserve session text. */
-  def inSession: Boolean = false
-  def eval() = {
-    val s = settings
-    log("eval(): settings = " + s)
-    val transcript = ILoop.runForTranscript(code, s, inSession = inSession)
+  protected def inSession: Boolean = false
+  /** Config for test. */
+  protected def shellConfig(testSettings: Settings): ShellConfig = ILoop.TestConfig(testSettings)
+  /** The normalized output from evaluating the `code` script. */
+  protected def eval(): Iterator[String] = {
+    val testSettings = settings
+    log(s"eval(): settings = $testSettings")
+    val transcript = ILoop.runForTranscript(code, testSettings, shellConfig(testSettings), inSession = inSession)
     log(s"transcript[[$transcript]]")
     transcript.linesIterator.map(normalize)
   }
-  def show() = eval() foreach println
+  /** Print the transcript produced by `eval`. */
+  override def show() = eval().foreach(println)
 }
 
-
 /** Strip Any.toString's id@abcdef16 hashCodes. These are generally at end of result lines. */
 trait Hashless extends ReplTest {
   import Hashless._
@@ -82,13 +86,13 @@ trait StackCleaner extends ReplTest {
 }
 object StackCleaner {
   private val elidedAndMore = """(\s+\.{3} )\d+( elided and )\d+( more)""".r
-  private val elidedOrMore        = """(\s+\.{3} )\d+( (?:elided|more))""".r
+  private val elidedOrMore  = """(\s+\.{3} )\d+( (?:elided|more))""".r
   private val frame         = """(\s+at [^(]+\(<console>:)\d+(\))""".r
   private def stripFrameCount(line: String) = line match {
-    case elidedAndMore(ellipsis, infix, suffix) => s"$ellipsis???$infix???$suffix" // must be before `elided`
-    case elidedOrMore(ellipsis, suffix) => s"$ellipsis???$suffix"
-    case frame(prefix, suffix)    => s"${prefix}XX${suffix}"
-    case s                        => s
+    case elidedAndMore(ellipsis, infix, suffix) => s"$ellipsis???$infix???$suffix" // must precede `elidedOrMore`
+    case elidedOrMore(ellipsis, suffix)         => s"$ellipsis???$suffix"
+    case frame(prefix, suffix)                  => s"${prefix}XX${suffix}"
+    case _                                      => line
   }
 }
 

src/partest/scala/tools/partest/package.scala

@@ -19,6 +19,7 @@ import scala.concurrent.duration.Duration
 import scala.io.Codec
 import scala.jdk.CollectionConverters._
 import scala.tools.nsc.util.Exceptional
+import scala.util.chaining._
 
 package object partest {
   type File         = java.io.File
@@ -193,4 +194,17 @@ package object partest {
   def isDebug                = sys.props.contains("partest.debug") || sys.env.contains("PARTEST_DEBUG")
   def debugSettings          = sys.props.getOrElse("partest.debug.settings", "")
   def log(msg: => Any): Unit = if (isDebug) Console.err.println(msg)
+
+  private val printable = raw"\p{Print}".r
+
+  def hexdump(s: String): Iterator[String] = {
+    var offset = 0
+    def hex(bytes: Array[Byte])   = bytes.map(b => f"$b%02x").mkString(" ")
+    def charFor(byte: Byte): Char = byte.toChar match { case c @ printable() => c ; case _ => '.' }
+    def ascii(bytes: Array[Byte]) = bytes.map(charFor).mkString
+    def format(bytes: Array[Byte]): String =
+      f"$offset%08x  ${hex(bytes.slice(0, 8))}%-24s ${hex(bytes.slice(8, 16))}%-24s |${ascii(bytes)}|"
+        .tap(_ => offset += bytes.length)
+    s.getBytes(codec.charSet).grouped(16).map(format)
+  }
 }

src/repl-frontend/scala/tools/nsc/interpreter/shell/ILoop.scala

@@ -978,29 +978,30 @@ class ILoop(config: ShellConfig, inOverride: BufferedReader = null,
 object ILoop {
   implicit def loopToInterpreter(repl: ILoop): Repl = repl.intp
 
-  def testConfig(settings: Settings) =
-    new ShellConfig {
-      private val delegate = ShellConfig(settings)
-
-      val filesToPaste: List[String] = delegate.filesToPaste
-      val filesToLoad: List[String] = delegate.filesToLoad
-      val batchText: String = delegate.batchText
-      val batchMode: Boolean = delegate.batchMode
-      val doCompletion: Boolean = delegate.doCompletion
-      val haveInteractiveConsole: Boolean = delegate.haveInteractiveConsole
-
-      // No truncated output, because the result changes on Windows because of line endings
-      override val maxPrintString = {
-        val p = sys.Prop[Int]("wtf")
-        p.set("0")
-        p
-      }
-    }
+  class TestConfig(delegate: ShellConfig) extends ShellConfig {
+    def filesToPaste: List[String] = delegate.filesToPaste
+    def filesToLoad: List[String] = delegate.filesToLoad
+    def batchText: String = delegate.batchText
+    def batchMode: Boolean = delegate.batchMode
+    def doCompletion: Boolean = delegate.doCompletion
+    def haveInteractiveConsole: Boolean = delegate.haveInteractiveConsole
+
+    override val colorOk = delegate.colorOk
+
+    // No truncated output, because the result changes on Windows because of line endings
+    override val maxPrintString = sys.Prop[Int]("wtf").tap(_.set("0"))
+  }
+  object TestConfig {
+    def apply(settings: Settings) = new TestConfig(ShellConfig(settings))
+  }
 
   // Designed primarily for use by test code: take a String with a
   // bunch of code, and prints out a transcript of what it would look
   // like if you'd just typed it into the repl.
-  def runForTranscript(code: String, settings: Settings, inSession: Boolean = false): String = {
+  def runForTranscript(code: String, settings: Settings, inSession: Boolean = false): String =
+    runForTranscript(code, settings, TestConfig(settings), inSession)
+
+  def runForTranscript(code: String, settings: Settings, config: ShellConfig, inSession: Boolean): String = {
     import java.io.{BufferedReader, OutputStreamWriter, StringReader}
     import java.lang.System.{lineSeparator => EOL}
 
@@ -1028,7 +1029,6 @@ object ILoop {
           }
         }
 
-        val config = testConfig(settings)
         val repl = new ILoop(config, input, output) {
           // remove welcome message as it has versioning info (for reproducible test results),
           override def welcome = ""

src/repl/scala/tools/nsc/interpreter/Naming.scala

@@ -12,33 +12,40 @@
 
 package scala.tools.nsc.interpreter
 
-import scala.util.Properties.lineSeparator
 import scala.util.matching.Regex
 
 /** This is for name logic which is independent of the compiler (notice there's no Global.)
  *  That includes at least generating, metaquoting, mangling, and unmangling.
  */
 object Naming {
-  def unmangle(str: String): String = {
-    val ESC = '\u001b'
-    val cleaned = lineRegex.replaceAllIn(str, "")
-    // Looking to exclude binary data which hoses the terminal, but
-    // let through the subset of it we need, like whitespace and also
-    // <ESC> for ansi codes.
-    val binaryChars = cleaned count (ch => ch < 32 && !ch.isWhitespace && ch != ESC)
-    // Lots of binary chars - translate all supposed whitespace into spaces
-    // except supposed line endings, otherwise scrubbed lines run together
-    if (binaryChars > 5) // more than one can count while holding a hamburger
-      cleaned map {
-        case c if lineSeparator contains c => c
-        case c if c.isWhitespace => ' '
-        case c if c < 32 => '?'
-        case c => c
-      }
-    // Not lots - preserve whitespace and ESC
-    else
-      cleaned map (ch => if (ch.isWhitespace || ch == ESC) ch else if (ch < 32) '?' else ch)
-  }
+  // The CSI pattern matches a subset of the following spec:
+  // For CSI, or "Control Sequence Introducer" commands,
+  // the ESC [ is followed by any number (including none) of "parameter bytes" in the range 0x30–0x3F (ASCII 0–9:;<=>?),
+  // then by any number of "intermediate bytes" in the range 0x20–0x2F (ASCII space and !"#$%&'()*+,-./),
+  // then finally by a single "final byte" in the range 0x40–0x7E (ASCII @A–Z[\]^_`a–z{|}~)
+  private final val esc = "\u001b"    // "\N{escape}"
+  private val csi = raw"$esc\[[0-9;]*([\x40-\x7E])"
+
+  // Matches one of 3 alternatives:
+  // group 1 is the CSI command letter, where 'm' is color rendition
+  // group 2 is a sequence of chars to be rendered as `?`: anything non-printable and not some space char
+  // additional groups are introduced by linePattern but not used
+  private lazy val cleaner = raw"$csi|([^\p{Print}\p{Space}]+)|$linePattern".r
+
+  /** Final pass to clean up REPL output.
+   *
+   *  Substrings representing REPL artifacts are stripped.
+   *
+   *  Attempt to replace dangerous characters with '?', which might otherwise
+   *  put the terminal in a bad state. Allow SGR (select graphic rendition, "m")
+   *  control sequences, but restrict otherwise.
+   */
+  def unmangle(str: String): String = cleaner.replaceSomeIn(str, clean)
+
+  private def clean(m: Regex.Match): Option[String] =
+    if ("m" == m.group(1) ) None
+    else if (m.group(1) != null || m.group(2) != null) Some("?" * (m.end - m.start))
+    else Some("")
 
   // Uncompiled regex pattern to detect `line` package and members
   // `read`, `eval`, `print`, for purposes of filtering output and stack traces.
@@ -47,17 +54,23 @@ object Naming {
   //
   // $line3.$read.$iw.Bippy =
   //   $line3.$read$$iw$$Bippy@4a6a00ca
-  lazy val lineRegex: Regex = {
-    val sn = sessionNames
+  //
+  // This needs to be aware of LambdaMetafactory generated classnames to strip the correct number of '$' delimiters.
+  // A lambda hosted in a module `$iw` (which has a module class `$iw$` is named `$iw$ $ $Lambda1234` (spaces added
+  // here for clarification.) This differs from an explicitly declared inner classes named `$Foo`, which would be
+  // `$iw$$Foo`.
+  //
+  // (\Q$line\E\d+(\Q$read\E)?|\Q$read\E(\.INSTANCE)?(\$\Q$iw\E)?|\Q$eval\E|\Q$print\E|\Q$iw\E)(\.this\.|\.|/|\$\$(?=\$Lambda)|\$|$)
+  //
+  private def linePattern: String = {
     import Regex.{quote => q}
-    val lineN = q(sn.line) + """\d+"""
-    val lineNRead = lineN + raw"""(${q(sn.read)})?"""
-    // This needs to be aware of LambdaMetafactory generated classnames to strip the correct number of '$' delimiters.
-    // A lambda hosted in a module `$iw` (which has a module class `$iw$` is named `$iw$ $ $Lambda1234` (spaces added
-    // here for clarification.) This differs from an explicitly declared inner classes named `$Foo`, which would be
-    // `$iw$$Foo`.
-    (raw"""($lineNRead|${q(sn.read)}(\.INSTANCE)?(\$$${q(sn.iw)})?|${q(sn.eval)}|${q(sn.print)}|${q(sn.iw)})""" + """(\.this\.|\.|/|\$\$(?=\$Lambda)|\$|$)""").r
+    val sn        = sessionNames
+    val lineN     = raw"${q(sn.line)}\d+"
+    val lineNRead = raw"$lineN(${q(sn.read)})?"
+    val lambda    = """(\.this\.|\.|/|\$\$(?=\$Lambda)|\$|$)"""
+    raw"($lineNRead|${q(sn.read)}(\.INSTANCE)?(\$$${q(sn.iw)})?|${q(sn.eval)}|${q(sn.print)}|${q(sn.iw)})$lambda"
   }
+  lazy val lineRegex: Regex = linePattern.r
 
   object sessionNames {
     // All values are configurable by passing e.g. -Dscala.repl.name.read=XXX

test/files/run/t12276.check

@@ -0,0 +1,86 @@
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 6a 61 76 61 2e  |scala> .[0mjava.|
+00000010  6e 69 6f 2e 43 68 61 72  42 75 66 66 65 72 2e 61  |nio.CharBuffer.a|
+00000020  6c 6c 6f 63 61 74 65 28  35 29                    |llocate(5)|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 72 65 73  |val .[1m.[34mres|
+00000010  30 1b 5b 30 6d 3a 20 1b  5b 31 6d 1b 5b 33 32 6d  |0.[0m: .[1m.[32m|
+00000020  6a 61 76 61 2e 6e 69 6f  2e 43 68 61 72 42 75 66  |java.nio.CharBuf|
+00000030  66 65 72 1b 5b 30 6d 20  3d 20 3f 3f 3f 3f 3f     |fer.[0m = ?????|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 6a 61 76 61 2e  |scala> .[0mjava.|
+00000010  6e 69 6f 2e 43 68 61 72  42 75 66 66 65 72 2e 61  |nio.CharBuffer.a|
+00000020  6c 6c 6f 63 61 74 65 28  36 29                    |llocate(6)|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 72 65 73  |val .[1m.[34mres|
+00000010  31 1b 5b 30 6d 3a 20 1b  5b 31 6d 1b 5b 33 32 6d  |1.[0m: .[1m.[32m|
+00000020  6a 61 76 61 2e 6e 69 6f  2e 43 68 61 72 42 75 66  |java.nio.CharBuf|
+00000030  66 65 72 1b 5b 30 6d 20  3d 20 3f 3f 3f 3f 3f 3f  |fer.[0m = ??????|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 63 6c 61 73 73  |scala> .[0mclass|
+00000010  20 43                                             | C|
+
+00000000  63 6c 61 73 73 20 43                              |class C|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 63 6c 61 73 73  |scala> .[0mclass|
+00000010  4f 66 5b 43 5d                                    |Of[C]|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 72 65 73  |val .[1m.[34mres|
+00000010  32 1b 5b 30 6d 3a 20 1b  5b 31 6d 1b 5b 33 32 6d  |2.[0m: .[1m.[32m|
+00000020  43 6c 61 73 73 5b 43 5d  1b 5b 30 6d 20 3d 20 63  |Class[C].[0m = c|
+00000030  6c 61 73 73 20 43                                 |lass C|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 76 61 6c 20 65  |scala> .[0mval e|
+00000010  73 63 20 3d 20 30 78 31  62 2e 74 6f 43 68 61 72  |sc = 0x1b.toChar|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 65 73 63  |val .[1m.[34mesc|
+00000010  1b 5b 30 6d 3a 20 1b 5b  31 6d 1b 5b 33 32 6d 43  |.[0m: .[1m.[32mC|
+00000020  68 61 72 1b 5b 30 6d 20  3d 20 3f                 |har.[0m = ?|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 63 6c 61 73 73  |scala> .[0mclass|
+00000010  4f 66 5b 43 5d 2e 74 6f  53 74 72 69 6e 67 20 2b  |Of[C].toString +|
+00000020  20 65 73 63 20 2b 20 22  5b 33 7a 22              | esc + "[3z"|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 72 65 73  |val .[1m.[34mres|
+00000010  33 1b 5b 30 6d 3a 20 1b  5b 31 6d 1b 5b 33 32 6d  |3.[0m: .[1m.[32m|
+00000020  53 74 72 69 6e 67 1b 5b  30 6d 20 3d 20 63 6c 61  |String.[0m = cla|
+00000030  73 73 20 43 3f 3f 3f 3f                           |ss C????|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 63 6c 61 73 73  |scala> .[0mclass|
+00000010  4f 66 5b 43 5d 2e 74 6f  53 74 72 69 6e 67 20 2b  |Of[C].toString +|
+00000020  20 65 73 63 20 2b 20 22  5b 33 21 22              | esc + "[3!"|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 72 65 73  |val .[1m.[34mres|
+00000010  34 1b 5b 30 6d 3a 20 1b  5b 31 6d 1b 5b 33 32 6d  |4.[0m: .[1m.[32m|
+00000020  53 74 72 69 6e 67 1b 5b  30 6d 20 3d 20 63 6c 61  |String.[0m = cla|
+00000030  73 73 20 43 3f 5b 33 21                           |ss C?[3!|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 63 6c 61 73 73  |scala> .[0mclass|
+00000010  4f 66 5b 43 5d 2e 74 6f  53 74 72 69 6e 67 20 2b  |Of[C].toString +|
+00000020  20 73 63 61 6c 61 2e 69  6f 2e 41 6e 73 69 43 6f  | scala.io.AnsiCo|
+00000030  6c 6f 72 2e 59 45 4c 4c  4f 57                    |lor.YELLOW|
+
+00000000  76 61 6c 20 1b 5b 31 6d  1b 5b 33 34 6d 72 65 73  |val .[1m.[34mres|
+00000010  35 1b 5b 30 6d 3a 20 1b  5b 31 6d 1b 5b 33 32 6d  |5.[0m: .[1m.[32m|
+00000020  53 74 72 69 6e 67 1b 5b  30 6d 20 3d 20 63 6c 61  |String.[0m = cla|
+00000030  73 73 20 43 1b 5b 33 33  6d                       |ss C.[33m|
+
+00000000  1b 5b 33 35 6d                                    |.[35m|
+
+00000000  73 63 61 6c 61 3e 20 1b  5b 30 6d 3a 71 75 69 74  |scala> .[0m:quit|
+

test/files/run/t12276.scala

@@ -0,0 +1,22 @@
+import scala.tools.nsc.Settings
+import scala.tools.nsc.interpreter.shell.{ILoop, ShellConfig}
+import scala.tools.partest.{hexdump, ReplTest}
+
+object Test extends ReplTest {
+  def code = """
+    |java.nio.CharBuffer.allocate(5)
+    |java.nio.CharBuffer.allocate(6)
+    |class C
+    |classOf[C]
+    |val esc = 0x1b.toChar
+    |classOf[C].toString + esc + "[3z"
+    |classOf[C].toString + esc + "[3!"
+    |classOf[C].toString + scala.io.AnsiColor.YELLOW
+    |""".stripMargin
+
+  override protected def shellConfig(testSettings: Settings) =
+    new ILoop.TestConfig(ShellConfig(testSettings)) {
+      override val colorOk = true
+    }
+  override def normalize(s: String) = hexdump(s).mkString("", "\n", "\n")
+}