Repl truncation copes with null (#17336)

Fixes #17333

compiler/src/dotty/tools/repl/Rendering.scala

@@ -50,36 +50,40 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None):
         // We need to use the ScalaRunTime class coming from the scala-library
         // on the user classpath, and not the one available in the current
         // classloader, so we use reflection instead of simply calling
-        // `ScalaRunTime.replStringOf`. Probe for new API without extraneous newlines.
-        // For old API, try to clean up extraneous newlines by stripping suffix and maybe prefix newline.
+        // `ScalaRunTime.stringOf`. Also probe for new stringOf that does string quoting, etc.
         val scalaRuntime = Class.forName("scala.runtime.ScalaRunTime", true, myClassLoader)
         val renderer = "stringOf"
-        def stringOfMaybeTruncated(value: Object, maxElements: Int): String = {
-          try {
-            val meth = scalaRuntime.getMethod(renderer, classOf[Object], classOf[Int], classOf[Boolean])
-            val truly = java.lang.Boolean.TRUE
-            meth.invoke(null, value, maxElements, truly).asInstanceOf[String]
-          } catch {
-            case _: NoSuchMethodException =>
-              val meth = scalaRuntime.getMethod(renderer, classOf[Object], classOf[Int])
-              meth.invoke(null, value, maxElements).asInstanceOf[String]
-          }
-        }
-
-        (value: Object, maxElements: Int, maxCharacters: Int) => {
-          // `ScalaRuntime.stringOf` may truncate the output, in which case we want to indicate that fact to the user
-          // In order to figure out if it did get truncated, we invoke it twice - once with the `maxElements` that we
-          // want to print, and once without a limit. If the first is shorter, truncation did occur.
-          val notTruncated = stringOfMaybeTruncated(value, Int.MaxValue)
-          val maybeTruncatedByElementCount = stringOfMaybeTruncated(value, maxElements)
-          val maybeTruncated = truncate(maybeTruncatedByElementCount, maxCharacters)
-
-          // our string representation may have been truncated by element and/or character count
-          // if so, append an info string - but only once
-          if (notTruncated.length == maybeTruncated.length) maybeTruncated
-          else s"$maybeTruncated ... large output truncated, print value to show all"
-        }
-
+        val stringOfInvoker: (Object, Int) => String =
+          def richStringOf: (Object, Int) => String =
+            val method = scalaRuntime.getMethod(renderer, classOf[Object], classOf[Int], classOf[Boolean])
+            val richly = java.lang.Boolean.TRUE // add a repl option for enriched output
+            (value, maxElements) => method.invoke(null, value, maxElements, richly).asInstanceOf[String]
+          def poorStringOf: (Object, Int) => String =
+            try
+              val method = scalaRuntime.getMethod(renderer, classOf[Object], classOf[Int])
+              (value, maxElements) => method.invoke(null, value, maxElements).asInstanceOf[String]
+            catch case _: NoSuchMethodException => (value, maxElements) => String.valueOf(value).take(maxElements)
+          try richStringOf
+          catch case _: NoSuchMethodException => poorStringOf
+        def stringOfMaybeTruncated(value: Object, maxElements: Int): String = stringOfInvoker(value, maxElements)
+
+        // require value != null
+        // `ScalaRuntime.stringOf` returns null iff value.toString == null, let caller handle that.
+        // `ScalaRuntime.stringOf` may truncate the output, in which case we want to indicate that fact to the user
+        // In order to figure out if it did get truncated, we invoke it twice - once with the `maxElements` that we
+        // want to print, and once without a limit. If the first is shorter, truncation did occur.
+        // Note that `stringOf` has new API in flight to handle truncation, see stringOfMaybeTruncated.
+        (value: Object, maxElements: Int, maxCharacters: Int) =>
+          stringOfMaybeTruncated(value, Int.MaxValue) match
+            case null => null
+            case notTruncated =>
+              val maybeTruncated =
+                val maybeTruncatedByElementCount = stringOfMaybeTruncated(value, maxElements)
+                truncate(maybeTruncatedByElementCount, maxCharacters)
+              // our string representation may have been truncated by element and/or character count
+              // if so, append an info string - but only once
+              if notTruncated.length == maybeTruncated.length then maybeTruncated
+              else s"$maybeTruncated ... large output truncated, print value to show all"
       }
       myClassLoader
     }
@@ -90,13 +94,18 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None):
     else str.substring(0, str.offsetByCodePoints(0, maxPrintCharacters - 1))
 
   /** Return a String representation of a value we got from `classLoader()`. */
-  private[repl] def replStringOf(value: Object)(using Context): String =
+  private[repl] def replStringOf(sym: Symbol, value: Object)(using Context): String =
     assert(myReplStringOf != null,
       "replStringOf should only be called on values creating using `classLoader()`, but `classLoader()` has not been called so far")
     val maxPrintElements = ctx.settings.VreplMaxPrintElements.valueIn(ctx.settingsState)
     val maxPrintCharacters = ctx.settings.VreplMaxPrintCharacters.valueIn(ctx.settingsState)
-    val res = myReplStringOf(value, maxPrintElements, maxPrintCharacters)
-    if res == null then "null // non-null reference has null-valued toString" else res
+    // stringOf returns null if value.toString returns null. Show some text as a fallback.
+    def fallback = s"""null // result of "${sym.name}.toString" is null"""
+    if value == null then "null" else
+      myReplStringOf(value, maxPrintElements, maxPrintCharacters) match
+        case null => fallback
+        case res  => res
+    end if
 
   /** Load the value of the symbol using reflection.
    *
@@ -108,17 +117,15 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None):
     val symValue = resObj
       .getDeclaredMethods.find(_.getName == sym.name.encode.toString)
       .flatMap(result => rewrapValueClass(sym.info.classSymbol, result.invoke(null)))
-    val valueString = symValue.map(replStringOf)
+    symValue
+      .filter(_ => sym.is(Flags.Method) || sym.info != defn.UnitType)
+      .map(value => stripReplPrefix(replStringOf(sym, value)))
 
-    if (!sym.is(Flags.Method) && sym.info == defn.UnitType)
-      None
+  private def stripReplPrefix(s: String): String =
+    if (s.startsWith(REPL_WRAPPER_NAME_PREFIX))
+      s.drop(REPL_WRAPPER_NAME_PREFIX.length).dropWhile(c => c.isDigit || c == '$')
     else
-      valueString.map { s =>
-        if (s.startsWith(REPL_WRAPPER_NAME_PREFIX))
-          s.drop(REPL_WRAPPER_NAME_PREFIX.length).dropWhile(c => c.isDigit || c == '$')
-        else
-          s
-      }
+      s
 
   /** Rewrap value class to their Wrapper class
    *

compiler/test/dotty/tools/repl/ReplCompilerTests.scala

@@ -4,8 +4,9 @@ import scala.language.unsafeNulls
 
 import java.util.regex.Pattern
 
-import org.junit.Assert.{assertTrue => assert, _}
-import org.junit.{Ignore, Test}
+import org.junit.Assert.{assertEquals, assertFalse, assertTrue}
+import org.junit.Assert.{assertTrue => assert}
+import org.junit.Test
 import dotty.tools.dotc.core.Contexts.Context
 
 class ReplCompilerTests extends ReplTest:
@@ -107,28 +108,21 @@ class ReplCompilerTests extends ReplTest:
     assertEquals(expected, lines())
   }
 
-  // FIXME: Tests are not run in isolation, the classloader is corrupted after the first exception
-  @Ignore @Test def i3305: Unit = {
-    initially {
-      run("null.toString")
-      assert(storedOutput().startsWith("java.lang.NullPointerException"))
-    }
+  @Test def `i3305 SOE meh`: Unit = initially:
+    run("def foo: Int = 1 + foo; foo")
+    assert(storedOutput().startsWith("java.lang.StackOverflowError"))
 
-    initially {
-      run("def foo: Int = 1 + foo; foo")
-      assert(storedOutput().startsWith("def foo: Int\njava.lang.StackOverflowError"))
-    }
+  @Test def `i3305 NPE`: Unit = initially:
+    run("null.toString")
+    assert(storedOutput().startsWith("java.lang.NullPointerException"))
 
-    initially {
-      run("""throw new IllegalArgumentException("Hello")""")
-      assert(storedOutput().startsWith("java.lang.IllegalArgumentException: Hello"))
-    }
+  @Test def `i3305 IAE`: Unit = initially:
+    run("""throw new IllegalArgumentException("Hello")""")
+    assertTrue(storedOutput().startsWith("java.lang.IllegalArgumentException: Hello"))
 
-    initially {
-      run("val (x, y) = null")
-      assert(storedOutput().startsWith("scala.MatchError: null"))
-    }
-  }
+  @Test def `i3305 ME`: Unit = initially:
+    run("val (x, y) = null")
+    assert(storedOutput().startsWith("scala.MatchError: null"))
 
   @Test def i2789: Unit = initially {
     run("(x: Int) => println(x)")
@@ -437,6 +431,23 @@ class ReplCompilerTests extends ReplTest:
     s2
   }
 
+  @Test def `i17333 print null result of toString`: Unit =
+    initially:
+      run("val tpolecat = new Object { override def toString(): String = null }")
+    .andThen:
+      val last = lines().last
+      assertTrue(last, last.startsWith("val tpolecat: Object = null"))
+      assertTrue(last, last.endsWith("""// result of "tpolecat.toString" is null"""))
+
+  @Test def `i17333 print toplevel object with null toString`: Unit =
+    initially:
+      run("object tpolecat { override def toString(): String = null }")
+    .andThen:
+      run("tpolecat")
+      val last = lines().last
+      assertTrue(last, last.startsWith("val res0: tpolecat.type = null"))
+      assertTrue(last, last.endsWith("""// result of "res0.toString" is null"""))
+
 object ReplCompilerTests:
 
   private val pattern = Pattern.compile("\\r[\\n]?|\\n");