Revert 1499101, 81fec99, d434150: The Repl Changes

## What changes were proposed in this pull request?
Revert "SPARK-2632, SPARK-2576. Fixed by only importing what is necessary during class definition." This reverts commit 1499101.

This patch was originally authored by marmbrus.

Author: Michael Armbrust <[email protected]>

Closes apache#44 from rxin/rxin-repl-revert.

repl/scala-2.10/src/main/scala/org/apache/spark/repl/SparkILoop.scala

@@ -250,20 +250,6 @@ class SparkILoop(
       case xs       => xs find (_.name == cmd)
     }
   }
-  private var fallbackMode = false
-
-  private def toggleFallbackMode() {
-    val old = fallbackMode
-    fallbackMode = !old
-    System.setProperty("spark.repl.fallback", fallbackMode.toString)
-    echo(s"""
-      |Switched ${if (old) "off" else "on"} fallback mode without restarting.
-      |       If you have defined classes in the repl, it would
-      |be good to redefine them incase you plan to use them. If you still run
-      |into issues it would be good to restart the repl and turn on `:fallback`
-      |mode as first command.
-      """.stripMargin)
-  }
 
   /** Show the history */
   private lazy val historyCommand = new LoopCommand("history", "show the history (optional num is commands to show)") {
@@ -346,9 +332,6 @@ class SparkILoop(
     nullary("reset", "reset the repl to its initial state, forgetting all session entries", resetCommand),
     shCommand,
     nullary("silent", "disable/enable automatic printing of results", verbosity),
-    nullary("fallback", """
-                           |disable/enable advanced repl changes, these fix some issues but may introduce others.
-                           |This mode will be removed once these fixes stablize""".stripMargin, toggleFallbackMode),
     cmd("type", "[-v] <expr>", "display the type of an expression without evaluating it", typeCommand),
     nullary("warnings", "show the suppressed warnings from the most recent line which had any", warningsCommand)
   )

repl/scala-2.10/src/main/scala/org/apache/spark/repl/SparkIMain.scala

@@ -998,7 +998,7 @@ import org.apache.spark.annotation.DeveloperApi
    *
    *  Read! Eval! Print! Some of that not yet centralized here.
    */
-  class ReadEvalPrint(val lineId: Int) {
+  class ReadEvalPrint(lineId: Int) {
     def this() = this(freshLineId())
 
     private var lastRun: Run = _
@@ -1152,16 +1152,11 @@ import org.apache.spark.annotation.DeveloperApi
     def definedTypeSymbol(name: String) = definedSymbols(newTypeName(name))
     def definedTermSymbol(name: String) = definedSymbols(newTermName(name))
 
-    val definedClasses = handlers.exists {
-      case _: ClassHandler => true
-      case _ => false
-    }
-
     /** Code to import bound names from previous lines - accessPath is code to
      * append to objectName to access anything bound by request.
      */
     val SparkComputedImports(importsPreamble, importsTrailer, accessPath) =
-      importsCode(referencedNames.toSet, definedClasses)
+      importsCode(referencedNames.toSet)
 
     /** Code to access a variable with the specified name */
     def fullPath(vname: String) = {
@@ -1669,10 +1664,7 @@ import org.apache.spark.annotation.DeveloperApi
     // old style
     beSilentDuring(parse(code)) foreach { ts =>
       ts foreach { t =>
-        if (isShow || isShowRaw)
-          withoutUnwrapping(echo(asCompactString(t)))
-        else
-          withoutUnwrapping(logDebug(asCompactString(t)))
+        withoutUnwrapping(logDebug(asCompactString(t)))
       }
     }
   }

repl/scala-2.10/src/main/scala/org/apache/spark/repl/SparkImports.scala

@@ -108,9 +108,8 @@ private[repl] trait SparkImports {
    * last one imported is actually usable.
    */
   case class SparkComputedImports(prepend: String, append: String, access: String)
-  def fallback = System.getProperty("spark.repl.fallback", "false").toBoolean
 
-  protected def importsCode(wanted: Set[Name], definedClass: Boolean): SparkComputedImports = {
+  protected def importsCode(wanted: Set[Name]): SparkComputedImports = {
     /** 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.
@@ -126,14 +125,8 @@ private[repl] trait SparkImports {
         // 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 {
-       /* This case clause tries to "precisely" import only what is required. And in this
-        * it may miss out on some implicits, because implicits are not known in `wanted`. Thus 
-        * it is suitable for defining classes. AFAIK while defining classes implicits are not
-        * needed.*/
-          case h: ImportHandler if definedClass && !fallback => 
-            h.importedNames.exists(x => wanted.contains(x))
-          case _: ImportHandler  => true
-          case x                 => x.definesImplicit || (x.definedNames exists wanted)
+          case _: ImportHandler => true
+          case x                => x.definesImplicit || (x.definedNames exists wanted)
         }
 
         reqs match {
@@ -190,26 +183,15 @@ private[repl] trait SparkImports {
         // ambiguity errors will not be generated. Also, quote
         // the name of the variable, so that we don't need to
         // handle quoting keywords separately.
-        case x: ClassHandler if !fallback =>
-        // I am trying to guess if the import is a defined class
-        // This is an ugly hack, I am not 100% sure of the consequences.
-        // Here we, let everything but "defined classes" use the import with val.
-        // The reason for this is, otherwise the remote executor tries to pull the
-        // classes involved and may fail.
-          for (imv <- x.definedNames) {
-            val objName = req.lineRep.readPath
-            code.append("import " + objName + ".INSTANCE" + req.accessPath + ".`" + imv + "`\n")
-          }
-
         case x =>
           for (imv <- x.definedNames) {
             if (currentImps contains imv) addWrapper()
             val objName = req.lineRep.readPath
-            val valName = "$VAL" + newValId()
+            val valName = "$VAL" + newValId();
 
             if(!code.toString.endsWith(".`" + imv + "`;\n")) { // Which means already imported
-                code.append("val " + valName + " = " + objName + ".INSTANCE;\n")
-                code.append("import " + valName + req.accessPath + ".`" + imv + "`;\n")
+               code.append("val " + valName + " = " + objName + ".INSTANCE;\n")
+               code.append("import " + valName + req.accessPath + ".`" + imv + "`;\n")
             }
             // code.append("val " + valName + " = " + objName + ".INSTANCE;\n")
             // code.append("import " + valName + req.accessPath + ".`" + imv + "`;\n")

repl/scala-2.10/src/test/scala/org/apache/spark/repl/ReplSuite.scala

@@ -232,18 +232,6 @@ class ReplSuite extends SparkFunSuite {
     assertContains("res4: Array[Int] = Array(0, 0, 0, 0, 0)", output)
   }
 
-  test("SPARK-1199 two instances of same class don't type check.") {
-    val output = runInterpreter("local",
-      """
-        |case class Sum(exp: String, exp2: String)
-        |val a = Sum("A", "B")
-        |def b(a: Sum): String = a match { case Sum(_, _) => "Found Sum" }
-        |b(a)
-      """.stripMargin)
-    assertDoesNotContain("error:", output)
-    assertDoesNotContain("Exception", output)
-  }
-
   test("SPARK-2452 compound statements.") {
     val output = runInterpreter("local",
       """
@@ -364,3 +352,4 @@ class ReplSuite extends SparkFunSuite {
     assertContains("ret: Array[(Int, Iterable[Foo])] = Array((1,", output)
   }
 }
+