SI-7747 Make REPL wrappers serialization friendly

Spark has been shipping a forked version of our REPL for
sometime. We have been trying to fold the patches back into
the mainline so they can defork. This is the last outstanding
issue.

Consider this REPL session:

```
scala> val x = StdIn.readInt

scala> class A(a: Int)

scala> serializedAndExecuteRemotely {
  () => new A(x)
}
```

As shown by the enclosed test, the REPL, even with the
Spark friendly option `-Yrepl-class-based`, will re-initialize
`x` on the remote system.

This test simulates this by running a REPL session, and then
deserializing the resulting closure into a fresh classloader
based on the class files generated by that session. Before this
patch, it printed "evaluating x" twice.

This is based on the Spark change described:

  https://github.com/mesos/spark/pull/535#discussion_r3541925

A followup commit will avoid the `val lineN$read = ` part if we
import classes or type aliases only.

[Original commit from Prashant Sharma, test case from Jason Zaugg]

6 files changed, 162 insertions, 25 deletions

diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala
index c281126d5f..e355d9f864 100644
--- a/src/repl/scala/tools/nsc/interpreter/IMain.scala
+++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala
@@ -69,6 +69,8 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
   // Used in a test case.
   def showDirectory() = replOutput.show(out)
 
+  lazy val isClassBased: Boolean = settings.Yreplclassbased.value
+
   private[nsc] var printResults               = true      // whether to print result lines
   private[nsc] var totalSilence               = false     // whether to print anything
   private var _initializeComplete             = false     // compiler is initialized
@@ -310,8 +312,14 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
   }
 
   def originalPath(name: String): String = originalPath(TermName(name))
-  def originalPath(name: Name): String   = typerOp path name
-  def originalPath(sym: Symbol): String  = typerOp path sym
+  def originalPath(name: Name): String   = translateOriginalPath(typerOp path name)
+  def originalPath(sym: Symbol): String  = translateOriginalPath(typerOp path sym)
+  /** For class based repl mode we use an .INSTANCE accessor. */
+  val readInstanceName = if(isClassBased) ".INSTANCE" else ""
+  def translateOriginalPath(p: String): String = {
+    val readName = java.util.regex.Matcher.quoteReplacement(sessionNames.read)
+    p.replaceFirst(readName, readName + readInstanceName)
+  }
   def flatPath(sym: Symbol): String      = flatOp shift sym.javaClassName
 
   def translatePath(path: String) = {
@@ -758,11 +766,13 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     // object and we can do that much less wrapping.
     def packageDecl = "package " + packageName
 
+    def pathToInstance(name: String)   = packageName + "." + name + readInstanceName
     def pathTo(name: String)   = packageName + "." + name
     def packaged(code: String) = packageDecl + "\n\n" + code
 
-    def readPath  = pathTo(readName)
-    def evalPath  = pathTo(evalName)
+    def readPathInstance  = pathToInstance(readName)
+    def readPath = pathTo(readName)
+    def evalPath = pathTo(evalName)
 
     def call(name: String, args: Any*): AnyRef = {
       val m = evalMethod(name)
@@ -802,7 +812,8 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     /** The innermost object inside the wrapper, found by
       * following accessPath into the outer one.
       */
-    def resolvePathToSymbol(accessPath: String): Symbol = {
+    def resolvePathToSymbol(fullAccessPath: String): Symbol = {
+      val accessPath = fullAccessPath.stripPrefix(readPath)
       val readRoot = readRootPath(readPath) // the outermost wrapper
       (accessPath split '.').foldLeft(readRoot: Symbol) {
         case (sym, "")    => sym
@@ -849,7 +860,6 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     def defines    = defHandlers flatMap (_.definedSymbols)
     def imports    = importedSymbols
     def value      = Some(handlers.last) filter (h => h.definesValue) map (h => definedSymbols(h.definesTerm.get)) getOrElse NoSymbol
-
     val lineRep = new ReadEvalPrint()
 
     private var _originalLine: String = null
@@ -858,6 +868,11 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
 
     /** handlers for each tree in this request */
     val handlers: List[MemberHandler] = trees map (memberHandlers chooseHandler _)
+    val definesClass = handlers.exists {
+      case _: ClassHandler => true
+      case _ => false
+    }
+
     def defHandlers = handlers collect { case x: MemberDefHandler => x }
 
     /** list of names used by this expression */
@@ -875,13 +890,13 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       * append to objectName to access anything bound by request.
       */
     lazy val ComputedImports(importsPreamble, importsTrailer, accessPath) =
-      exitingTyper(importsCode(referencedNames.toSet, ObjectSourceCode))
+      exitingTyper(importsCode(referencedNames.toSet, ObjectSourceCode, definesClass))
 
     /** the line of code to compute */
     def toCompute = line
 
     /** The path of the value that contains the user code. */
-    def fullAccessPath = s"${lineRep.readPath}$accessPath"
+    def fullAccessPath = s"${lineRep.readPathInstance}$accessPath"
 
     /** The path of the given member of the wrapping instance. */
     def fullPath(vname: String) = s"$fullAccessPath.`$vname`"
@@ -911,7 +926,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       def postwrap: String
     }
 
-    private class ObjectBasedWrapper extends Wrapper {
+    class ObjectBasedWrapper extends Wrapper {
       def preambleHeader = "object %s {"
 
       def postamble = importsTrailer + "\n}"
@@ -919,13 +934,16 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       def postwrap = "}\n"
     }
 
-    private class ClassBasedWrapper extends Wrapper {
-      def preambleHeader = "class %s extends Serializable {"
+    class ClassBasedWrapper extends Wrapper {
+      def preambleHeader = "class %s extends Serializable { "
 
       /** Adds an object that instantiates the outer wrapping class. */
-      def postamble  = s"""$importsTrailer
+      def postamble  = s"""
+                          |$importsTrailer
+                          |}
+                          |object ${lineRep.readName} {
+                          |   val INSTANCE = new ${lineRep.readName}();
                           |}
-                          |object ${lineRep.readName} extends ${lineRep.readName}
                           |""".stripMargin
 
       import nme.{ INTERPRETER_IMPORT_WRAPPER => iw }
@@ -935,7 +953,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     }
 
     private lazy val ObjectSourceCode: Wrapper =
-      if (settings.Yreplclassbased) new ClassBasedWrapper else new ObjectBasedWrapper
+      if (isClassBased) new ClassBasedWrapper else new ObjectBasedWrapper
 
     private object ResultObjectSourceCode extends IMain.CodeAssembler[MemberHandler] {
       /** We only want to generate this code when the result
@@ -994,7 +1012,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       }
     }
 
-    lazy val resultSymbol = lineRep.resolvePathToSymbol(accessPath)
+    lazy val resultSymbol = lineRep.resolvePathToSymbol(fullAccessPath)
     def applyToResultMember[T](name: Name, f: Symbol => T) = exitingTyper(f(resultSymbol.info.nonPrivateDecl(name)))
 
     /* typeOf lookup with encoding */
diff --git a/src/repl/scala/tools/nsc/interpreter/Imports.scala b/src/repl/scala/tools/nsc/interpreter/Imports.scala
index 5244858a62..97798cd017 100644
--- a/src/repl/scala/tools/nsc/interpreter/Imports.scala
+++ b/src/repl/scala/tools/nsc/interpreter/Imports.scala
@@ -92,7 +92,7 @@ trait Imports {
    * last one imported is actually usable.
    */
   case class ComputedImports(prepend: String, append: String, access: String)
-  protected def importsCode(wanted: Set[Name], wrapper: Request#Wrapper): ComputedImports = {
+  protected def importsCode(wanted: Set[Name], wrapper: Request#Wrapper, definesClass: Boolean): ComputedImports = {
     /** Narrow down the list of requests from which imports
      *  should be taken.  Removes requests which cannot contribute
      *  useful imports for the specified set of wanted names.
@@ -107,6 +107,8 @@ trait Imports {
         // Single symbol imports might be implicits! See bug #1752.  Rather than
         // try to finesse this, we will mimic all imports for now.
         def keepHandler(handler: MemberHandler) = handler match {
+        /* While defining classes in class based mode - implicits are not needed. */
+          case h: ImportHandler if isClassBased && definesClass => h.importedNames.exists(x => wanted.contains(x))
           case _: ImportHandler => true
           case x                => x.definesImplicit || (x.definedNames exists wanted)
         }
@@ -146,7 +148,10 @@ trait Imports {
 
     // loop through previous requests, adding imports for each one
     wrapBeforeAndAfter {
+      // Reusing a single temporary value when import from a line with multiple definitions.
+      val tempValLines = mutable.Set[Int]()
       for (ReqAndHandler(req, handler) <- reqsToUse) {
+        val objName = req.lineRep.readPathInstance
         handler match {
           // If the user entered an import, then just use it; add an import wrapping
           // level if the import might conflict with some other import
@@ -157,6 +162,18 @@ trait Imports {
             code append (x.member + "\n")
             currentImps ++= x.importedNames
 
+          case x if isClassBased =>
+            for (imv <- x.definedNames) {
+              if (!currentImps.contains(imv)) {
+                val valName = req.lineRep.packageName + req.lineRep.readName
+                if (!tempValLines.contains(req.lineRep.lineId)) {
+                  code.append(s"val $valName = $objName\n")
+                  tempValLines += req.lineRep.lineId
+                }
+                code.append(s"import $valName ${req.accessPath}.`$imv`;\n")
+                currentImps += imv
+              }
+            }
           // For other requests, import each defined name.
           // import them explicitly instead of with _, so that
           // ambiguity errors will not be generated. Also, quote
diff --git a/test/files/run/repl-serialization.check b/test/files/run/repl-serialization.check
new file mode 100644
index 0000000000..c8439ea023
--- /dev/null
+++ b/test/files/run/repl-serialization.check
@@ -0,0 +1,21 @@
+== evaluating lines
+extract: AnyRef => Unit = <function1>
+  evaluating x
+x: Int = 0
+y: Int = <lazy>
+  evaluating z
+  evaluating zz
+defined class D
+z: Int = 0
+zz: Int = 0
+defined object O
+defined class A
+defined type alias AA
+== evaluating lambda
+  evaluating y
+  evaluating O
+  constructing A
+== reconstituting into a fresh classloader
+  evaluating O
+== evaluating reconstituted lambda
+  constructing A
diff --git a/test/files/run/repl-serialization.scala b/test/files/run/repl-serialization.scala
new file mode 100644
index 0000000000..64915ce51e
--- /dev/null
+++ b/test/files/run/repl-serialization.scala
@@ -0,0 +1,65 @@
+import java.io._
+
+import scala.reflect.io.AbstractFile
+import scala.tools.nsc.Settings
+import scala.tools.nsc.interpreter.IMain
+import scala.tools.nsc.util._
+import scala.reflect.internal.util.AbstractFileClassLoader
+
+object Test {
+  def main(args: Array[String]) {
+    run()
+  }
+
+  def run(): Unit = {
+    val settings = new Settings()
+    settings.Yreplclassbased.value = true
+    settings.usejavacp.value = true
+
+    var imain: IMain = null
+    object extract extends ((AnyRef) => Unit) with Serializable {
+      var value: AnyRef = null
+
+      def apply(a: AnyRef) = value = a
+    }
+
+    val code =
+      """val x = {println("  evaluating x"); 0 }
+        |lazy val y = {println("  evaluating y"); 0 }
+        |class D; val z = {println("  evaluating z"); 0}; val zz = {println("  evaluating zz"); 0}
+        |object O extends Serializable { val apply = {println("  evaluating O"); 0} }
+        |class A(i: Int) { println("  constructing A") }
+        |type AA = A
+        |extract(() => new AA(x + y + z + zz + O.apply))
+      """.stripMargin
+
+    imain = new IMain(settings)
+    println("== evaluating lines")
+    imain.directBind("extract", "(AnyRef => Unit)", extract)
+    code.lines.foreach(imain.interpret)
+
+    val virtualFile: AbstractFile = extract.value.getClass.getClassLoader.asInstanceOf[AbstractFileClassLoader].root
+    val newLoader = new AbstractFileClassLoader(virtualFile, getClass.getClassLoader)
+
+    def deserializeInNewLoader(string: Array[Byte]): AnyRef = {
+      val bis = new ByteArrayInputStream(string)
+      val in = new ObjectInputStream(bis) {
+        override def resolveClass(desc: ObjectStreamClass) = Class.forName(desc.getName, false, newLoader)
+      }
+      in.readObject()
+    }
+    def serialize(o: AnyRef): Array[Byte] = {
+      val bos = new ByteArrayOutputStream()
+      val out = new ObjectOutputStream(bos)
+      out.writeObject(o)
+      out.close()
+      bos.toByteArray
+    }
+    println("== evaluating lambda")
+    extract.value.asInstanceOf[() => Any].apply()
+    println("== reconstituting into a fresh classloader")
+    val reconstituted = deserializeInNewLoader(serialize(extract.value)).asInstanceOf[() => Any]
+    println("== evaluating reconstituted lambda")
+    reconstituted.apply() // should not print("evaluating x") a second time
+  }
+}
diff --git a/test/files/run/t7747-repl.check b/test/files/run/t7747-repl.check
index 105b238d01..6eb8c8cd99 100644
--- a/test/files/run/t7747-repl.check
+++ b/test/files/run/t7747-repl.check
@@ -112,7 +112,7 @@ scala> 55 ; ((2 + 2)) ; (1, 2, 3)
 res15: (Int, Int, Int) = (1,2,3)
 
 scala> 55 ; (x: Int) => x + 1 ; () => ((5))
-<console>:8: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+<console>:9: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
               55 ; (x: Int) => x + 1 ;;
               ^
 res16: () => Int = <function0>
@@ -258,12 +258,15 @@ class $read extends Serializable {
       super.<init>;
       ()
     };
-    import $line44.$read.$iw.$iw.BippyBups;
-    import $line44.$read.$iw.$iw.BippyBups;
-    import $line45.$read.$iw.$iw.PuppyPups;
-    import $line45.$read.$iw.$iw.PuppyPups;
-    import $line46.$read.$iw.$iw.Bingo;
-    import $line46.$read.$iw.$iw.Bingo;
+    val $line44$read = $line44.$read.INSTANCE;
+    import $line44$read.$iw.$iw.BippyBups;
+    import $line44$read.$iw.$iw.BippyBups;
+    val $line45$read = $line45.$read.INSTANCE;
+    import $line45$read.$iw.$iw.PuppyPups;
+    import $line45$read.$iw.$iw.PuppyPups;
+    val $line46$read = $line46.$read.INSTANCE;
+    import $line46$read.$iw.$iw.Bingo;
+    import $line46$read.$iw.$iw.Bingo;
     class $iw extends Serializable {
       def <init>() = {
         super.<init>;
@@ -275,12 +278,23 @@ class $read extends Serializable {
   };
   val $iw = new $iw.<init>
 }
-object $read extends $read {
+object $read extends scala.AnyRef {
   def <init>() = {
     super.<init>;
     ()
-  }
+  };
+  val INSTANCE = new $read.<init>
 }
 res3: List[Product with Serializable] = List(BippyBups(), PuppyPups(), Bingo())
 
+scala> :power
+** Power User mode enabled - BEEP WHIR GYVE **
+** :phase has been set to 'typer'.          **
+** scala.tools.nsc._ has been imported      **
+** global._, definitions._ also imported    **
+** Try  :help, :vals, power.<tab>           **
+
+scala> intp.lastRequest
+res4: $r.intp.Request = Request(line=def $ires3 = intp.global, 1 trees)
+
 scala> :quit
diff --git a/test/files/run/t7747-repl.scala b/test/files/run/t7747-repl.scala
index 0e64210460..a681eb8851 100644
--- a/test/files/run/t7747-repl.scala
+++ b/test/files/run/t7747-repl.scala
@@ -65,5 +65,7 @@ object Test extends ReplTest {
     |case class PuppyPups()
     |case class Bingo()
     |List(BippyBups(), PuppyPups(), Bingo()) // show
+    |:power
+    |intp.lastRequest
     |""".stripMargin
 }