Merge pull request #4915 from dotty-staging/fix-4230

Fix #4230: Handle import in the REPL correctly

Author: allanrenucci

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -111,7 +111,7 @@ class Compiler {
     List(new Flatten,                // Lift all inner classes to package scope
          new RenameLifted,           // Renames lifted classes to local numbering scheme
          new TransformWildcards,     // Replace wildcards with default values
-         new MoveStatics,            // Move static methods to companion classes
+         new MoveStatics,            // Move static methods from companion to the class itself
          new ExpandPrivate,          // Widen private definitions accessed from nested classes
          new RestoreScopes,          // Repair scopes rendered invalid by moving definitions in prior phases of the group
          new SelectStatic,           // get rid of selects that would be compiled into GetStatic

compiler/src/dotty/tools/dotc/Run.scala

@@ -147,7 +147,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
     compileUnits()(ctx)
   }
 
-  protected def compileUnits()(implicit ctx: Context) = Stats.maybeMonitored {
+  private def compileUnits()(implicit ctx: Context) = Stats.maybeMonitored {
     if (!ctx.mode.is(Mode.Interactive)) // IDEs might have multi-threaded access, accesses are synchronized
       ctx.base.checkSingleThreaded()
 

compiler/src/dotty/tools/dotc/transform/Pickler.scala

@@ -112,8 +112,8 @@ class Pickler extends Phase {
     if (previous != unpickled) {
       output("before-pickling.txt", previous)
       output("after-pickling.txt", unpickled)
-      ctx.error(s"""pickling difference for ${cls} in ${cls.sourceFile}, for details:
+      ctx.error(s"""pickling difference for $cls in ${cls.sourceFile}, for details:
                    |
-                   |  diff before-pickling.txt after-pickling.txt""")
+                   |  diff before-pickling.txt after-pickling.txt""".stripMargin)
     }
 }

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

@@ -0,0 +1,30 @@
+package dotty.tools.repl
+
+import dotty.tools.dotc.ast.Trees._
+import dotty.tools.dotc.ast.tpd
+import dotty.tools.dotc.core.Contexts.Context
+import dotty.tools.dotc.core.Phases.Phase
+
+/** A phase that collects user defined top level imports.
+ *
+ *  These imports must be collected as typed trees and therefore
+ *  after Typer.
+ */
+class CollectTopLevelImports extends Phase {
+  import tpd._
+
+  def phaseName = "collectTopLevelImports"
+
+  private[this] var myImports: List[Import] = _
+  def imports = myImports
+
+  def run(implicit ctx: Context): Unit = {
+    def topLevelImports(tree: Tree) = {
+      val PackageDef(_, _ :: TypeDef(_, rhs: Template) :: Nil) = tree
+      rhs.body.collect { case tree: Import => tree }
+    }
+
+    val tree = ctx.compilationUnit.tpdTree
+    myImports = topLevelImports(tree)
+  }
+}

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

@@ -14,6 +14,7 @@ import dotty.tools.dotc.core.Phases.Phase
 import dotty.tools.dotc.core.StdNames._
 import dotty.tools.dotc.core.Symbols._
 import dotty.tools.dotc.reporting.diagnostic.messages
+import dotty.tools.dotc.transform.PostTyper
 import dotty.tools.dotc.typer.{FrontEnd, ImportInfo}
 import dotty.tools.dotc.util.Positions._
 import dotty.tools.dotc.util.SourceFile
@@ -23,42 +24,52 @@ import dotty.tools.repl.results._
 
 import scala.collection.mutable
 
-/** This subclass of `Compiler` replaces the appropriate phases in order to
- *  facilitate the REPL
+/** This subclass of `Compiler` is adapted for use in the REPL.
  *
- *  Specifically it replaces the front end with `REPLFrontEnd`, and adds a
- *  custom subclass of `GenBCode`. The custom `GenBCode`, `REPLGenBCode`, works
- *  in conjunction with a specialized class loader in order to load virtual
- *  classfiles.
+ *  - compiles parsed expression in the current REPL state:
+ *    - adds the appropriate imports in scope
+ *    - wraps expressions into a dummy object
+ *  - provides utility to query the type of an expression
+ *  - provides utility to query the documentation of an expression
  */
 class ReplCompiler extends Compiler {
-  override protected def frontendPhases: List[List[Phase]] =
-    Phases.replace(classOf[FrontEnd], _ => new REPLFrontEnd :: Nil, super.frontendPhases)
 
-  def newRun(initCtx: Context, objectIndex: Int) = new Run(this, initCtx) {
-    override protected[this] def rootContext(implicit ctx: Context) =
-      addMagicImports(super.rootContext)
-
-    private def addMagicImports(initCtx: Context): Context = {
-      def addImport(path: TermName)(implicit ctx: Context) = {
-        val importInfo = ImportInfo.rootImport { () =>
-          ctx.requiredModuleRef(path)
-        }
-        ctx.fresh.setNewScope.setImportInfo(importInfo)
+  override protected def frontendPhases: List[List[Phase]] = List(
+    List(new REPLFrontEnd),
+    List(new CollectTopLevelImports),
+    List(new PostTyper)
+  )
+
+  def newRun(initCtx: Context, state: State): Run = new Run(this, initCtx) {
+
+    /** Import previous runs and user defined imports */
+    override protected[this] def rootContext(implicit ctx: Context): Context = {
+      def importContext(imp: tpd.Import)(implicit ctx: Context) =
+        ctx.importContext(imp, imp.symbol)
+
+      def importPreviousRun(id: Int)(implicit ctx: Context) = {
+        // we first import the wrapper object id
+        val path = nme.EMPTY_PACKAGE ++ "." ++ objectNames(id)
+        val importInfo = ImportInfo.rootImport(() =>
+          ctx.requiredModuleRef(path))
+        val ctx0 = ctx.fresh.setNewScope.setImportInfo(importInfo)
+
+        // then its user defined imports
+        val imports = state.imports.getOrElse(id, Nil)
+        if (imports.isEmpty) ctx0
+        else imports.foldLeft(ctx0.fresh.setNewScope)((ctx, imp) =>
+          importContext(imp)(ctx))
       }
 
-      (1 to objectIndex)
-        .foldLeft(initCtx) { (ictx, i) =>
-          addImport(nme.EMPTY_PACKAGE ++ "." ++ objectNames(i))(ictx)
-        }
+      (1 to state.objectIndex).foldLeft(super.rootContext)((ctx, id) =>
+        importPreviousRun(id)(ctx))
     }
   }
 
   private[this] val objectNames = mutable.Map.empty[Int, TermName]
   private def objectName(state: State) =
-    objectNames.getOrElseUpdate(state.objectIndex, {
-      (str.REPL_SESSION_LINE + state.objectIndex).toTermName
-    })
+    objectNames.getOrElseUpdate(state.objectIndex,
+      (str.REPL_SESSION_LINE + state.objectIndex).toTermName)
 
   private case class Definitions(stats: List[untpd.Tree], state: State)
 
@@ -86,7 +97,7 @@ class ReplCompiler extends Compiler {
     }
 
     Definitions(
-      state.imports ++ defs,
+      defs,
       state.copy(
         objectIndex = state.objectIndex + (if (defs.isEmpty) 0 else 1),
         valIndex = valIdx
@@ -158,19 +169,18 @@ class ReplCompiler extends Compiler {
   def docOf(expr: String)(implicit state: State): Result[String] = {
     implicit val ctx: Context = state.context
 
-    /**
-     * Extract the "selected" symbol from `tree`.
+    /** Extract the "selected" symbol from `tree`.
      *
-     * Because the REPL typechecks an expression, special syntax is needed to get the documentation
-     * of certain symbols:
+     *  Because the REPL typechecks an expression, special syntax is needed to get the documentation
+     *  of certain symbols:
      *
-     * - To select the documentation of classes, the user needs to pass a call to the class' constructor
-     *   (e.g. `new Foo` to select `class Foo`)
-     * - When methods are overloaded, the user needs to enter a lambda to specify which functions he wants
-     *   (e.g. `foo(_: Int)` to select `def foo(x: Int)` instead of `def foo(x: String)`
+     *  - To select the documentation of classes, the user needs to pass a call to the class' constructor
+     *    (e.g. `new Foo` to select `class Foo`)
+     *  - When methods are overloaded, the user needs to enter a lambda to specify which functions he wants
+     *    (e.g. `foo(_: Int)` to select `def foo(x: Int)` instead of `def foo(x: String)`
      *
-     * This function returns the right symbol for the received expression, and all the symbols that are
-     * overridden.
+     *  This function returns the right symbol for the received expression, and all the symbols that are
+     *  overridden.
      */
     def extractSymbols(tree: tpd.Tree): Iterator[Symbol] = {
       val sym = tree match {
@@ -210,7 +220,7 @@ class ReplCompiler extends Compiler {
         import untpd._
 
         val valdef = ValDef("expr".toTermName, TypeTree(), Block(trees, unitLiteral))
-        val tmpl = Template(emptyConstructor, Nil, EmptyValDef, state.imports :+ valdef)
+        val tmpl = Template(emptyConstructor, Nil, EmptyValDef, List(valdef))
         val wrapper = TypeDef("$wrapper".toTypeName, tmpl)
           .withMods(Modifiers(Final))
           .withPos(Position(0, expr.length))
@@ -261,9 +271,8 @@ class ReplCompiler extends Compiler {
 
       if (errorsAllowed || !ctx.reporter.hasErrors)
         unwrapped(unit.tpdTree, src)
-      else {
-        ctx.reporter.removeBufferedMessages.errors
-      }
+      else
+        ctx.reporter.removeBufferedMessages.errors[tpd.ValDef] // Workaround #4988
     }
   }
 }

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

@@ -44,12 +44,12 @@ import scala.collection.JavaConverters._
  *
  *  @param objectIndex the index of the next wrapper
  *  @param valIndex    the index of next value binding for free expressions
- *  @param imports     the list of user defined imports
+ *  @param imports     a map from object index to the list of user defined imports
  *  @param context     the latest compiler context
  */
 case class State(objectIndex: Int,
                  valIndex: Int,
-                 imports: List[untpd.Import],
+                 imports: Map[Int, List[tpd.Import]],
                  context: Context)
 
 /** Main REPL instance, orchestrating input, compilation and presentation */
@@ -64,14 +64,14 @@ class ReplDriver(settings: Array[String],
 
   /** Create a fresh and initialized context with IDE mode enabled */
   private[this] def initialCtx = {
-    val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive).addMode(Mode.ReadComments)
+    val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions | Mode.Interactive | Mode.ReadComments)
     val ictx = setup(settings, rootCtx)._2
     ictx.base.initialize()(ictx)
     ictx
   }
 
   /** the initial, empty state of the REPL session */
-  final def initialState = State(0, 0, Nil, rootCtx)
+  final def initialState = State(0, 0, Map.empty, rootCtx)
 
   /** Reset state of repl to the initial state
    *
@@ -144,7 +144,7 @@ class ReplDriver(settings: Array[String],
     Console.withOut(out) { Console.withErr(out) { op } }
 
   private def newRun(state: State) = {
-    val run = compiler.newRun(rootCtx.fresh.setReporter(newStoreReporter), state.objectIndex)
+    val run = compiler.newRun(rootCtx.fresh.setReporter(newStoreReporter), state)
     state.copy(context = run.runContext)
   }
 
@@ -177,9 +177,6 @@ class ReplDriver(settings: Array[String],
       .getOrElse(Nil)
   }
 
-  private def extractImports(trees: List[untpd.Tree]): List[untpd.Import] =
-    trees.collect { case imp: untpd.Import => imp }
-
   private def interpret(res: ParseResult)(implicit state: State): State = {
     val newState = res match {
       case parsed: Parsed if parsed.trees.nonEmpty =>
@@ -209,6 +206,9 @@ class ReplDriver(settings: Array[String],
       case _ => nme.NO_NAME
     }
 
+    def extractTopLevelImports(ctx: Context): List[tpd.Import] =
+      ctx.phases.collectFirst { case phase: CollectTopLevelImports => phase.imports }.get
+
     implicit val state = newRun(istate)
     compiler
       .compile(parsed)
@@ -217,8 +217,11 @@ class ReplDriver(settings: Array[String],
         {
           case (unit: CompilationUnit, newState: State) =>
             val newestWrapper = extractNewestWrapper(unit.untpdTree)
-            val newImports = newState.imports ++ extractImports(parsed.trees)
-            val newStateWithImports = newState.copy(imports = newImports)
+            val newImports = extractTopLevelImports(newState.context)
+            var allImports = newState.imports
+            if (newImports.nonEmpty)
+              allImports += (newState.objectIndex -> newImports)
+            val newStateWithImports = newState.copy(imports = allImports)
 
             val warnings = newState.context.reporter.removeBufferedMessages(newState.context)
             displayErrors(warnings)(newState) // display warnings
@@ -315,7 +318,10 @@ class ReplDriver(settings: Array[String],
       initialState
 
     case Imports =>
-      state.imports.foreach(i => out.println(SyntaxHighlighting(i.show(state.context))))
+      for {
+        objectIndex <- 1 to state.objectIndex
+        imp <- state.imports.getOrElse(objectIndex, Nil)
+      } out.println(imp.show(state.context))
       state
 
     case Load(path) =>

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

@@ -12,7 +12,6 @@ import dotc.core.Contexts.Context
  *  compiler pipeline.
  */
 private[repl] class REPLFrontEnd extends FrontEnd {
-  override def phaseName = "frontend"
 
   override def isRunnable(implicit ctx: Context) = true
 

compiler/test-resources/repl/import-shadowing

@@ -0,0 +1,27 @@
+scala> object A { def f = 1 }
+// defined object A
+
+scala> object B { def f = 2 }
+// defined object B
+
+scala> import A._
+
+scala> val x0 = f
+val x0: Int = 1
+
+scala> import B._
+
+scala> val x1 = f
+val x1: Int = 2
+
+scala> def f = 3
+def f: Int
+
+scala> val x2 = f
+val x2: Int = 3
+
+scala> def f = 4; import A._
+def f: Int
+
+scala> val x3 = f
+val x3: Int = 1

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

@@ -60,4 +60,24 @@ class TabcompleteTests extends ReplTest {
       val expected = List("comp1", "comp2", "comp3")
       assertEquals(expected, tabComplete("(new Foo).comp").sorted)
     }
+
+  @Test def completeFromPreviousState2 =
+    fromInitialState { implicit state  =>
+      val src = "def hello = 1"
+      run(src)
+    }
+    .andThen { implicit state =>
+      val expected = List("hello")
+      assertEquals(expected, tabComplete("hel"))
+    }
+
+  @Test def tabCompleteFromPreviousImport =
+    fromInitialState { implicit state =>
+      val src = "import java.io.FileDescriptor"
+      run(src)
+    }
+    .andThen { implicit state =>
+      val expected = List("FileDescriptor")
+      assertEquals(expected, tabComplete("val foo: FileDesc"))
+    }
 }