Fix scala#4230: Handle import in the REPL correctly

We used to collect user defined imports from the parsed tree and
insert them as untyped trees at the top of the REPL wrapper object.
This caused members shadowing issues.

We now introduce a phase in the REPL compiler that collects imports
after type checking and store them as typed tree. We can then create a
context with its imports set in the correct order and use it to compile
future expressions.

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/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
@@ -26,39 +27,43 @@ import scala.collection.mutable
 /** This subclass of `Compiler` replaces the appropriate phases in order to
  *  facilitate 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.
+ *  Specifically it replaces the front end with `REPLFrontEnd`.
  */
 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)
+  override protected def frontendPhases: List[List[Phase]] = List(
+    List(new REPLFrontEnd),
+    List(new CollectTopLevelImports),
+    List(new PostTyper)
+  )
 
-    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)
-      }
+  def newRunContext(initCtx: Context, state: State): Context = {
+    def addUserDefinedImport(imp: tpd.Import)(implicit ctx: Context) =
+      ctx.importContext(imp, imp.symbol)
 
-      (1 to objectIndex)
-        .foldLeft(initCtx) { (ictx, i) =>
-          addImport(nme.EMPTY_PACKAGE ++ "." ++ objectNames(i))(ictx)
-        }
+    def importModule(path: TermName)(implicit ctx: Context) = {
+      val importInfo = ImportInfo.rootImport(() =>
+        ctx.requiredModuleRef(path))
+      ctx.fresh.setNewScope.setImportInfo(importInfo)
+    }
+
+    val run = newRun(initCtx.fresh.setReporter(newStoreReporter))
+    (1 to state.objectIndex).foldLeft(run.runContext) { (ctx, i) =>
+      // we first import the wrapper object i
+      val path = nme.EMPTY_PACKAGE ++ "." ++ objectNames(i)
+      val ctx0 = importModule(path)(ctx)
+      // then its user defined imports
+      val imports = state.imports.getOrElse(i, Nil)
+      if (imports.isEmpty) ctx0
+      else imports.foldLeft(ctx0.fresh.setNewScope)((ctx, imp) =>
+        addUserDefinedImport(imp)(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 +91,7 @@ class ReplCompiler extends Compiler {
     }
 
     Definitions(
-      state.imports ++ defs,
+      defs,
       state.copy(
         objectIndex = state.objectIndex + (if (defs.isEmpty) 0 else 1),
         valIndex = valIdx
@@ -130,7 +135,7 @@ class ReplCompiler extends Compiler {
 
   private def runCompilationUnit(unit: CompilationUnit, state: State): Result[(CompilationUnit, State)] = {
     val ctx = state.context
-    ctx.run.compileUnits(unit :: Nil)
+    ctx.run.compileUnits(unit :: Nil, ctx)
 
     if (!ctx.reporter.hasErrors) (unit, state).result
     else ctx.reporter.removeBufferedMessages(ctx).errors
@@ -158,19 +163,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 +214,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 +265,8 @@ class ReplCompiler extends Compiler {
 
       if (errorsAllowed || !ctx.reporter.hasErrors)
         unwrapped(unit.tpdTree, src)
-      else {
+      else
         ctx.reporter.removeBufferedMessages.errors
-      }
     }
   }
 }

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

@@ -43,12 +43,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 */
@@ -63,14 +63,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 */
-  protected[this] def initState = State(0, 0, Nil, rootCtx)
+  protected[this] def initState = State(0, 0, Map.empty, rootCtx)
 
   /** Reset state of repl to the initial state
    *
@@ -140,8 +140,8 @@ 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)
-    state.copy(context = run.runContext)
+    val run = compiler.newRunContext(rootCtx, state)
+    state.copy(context = run)
   }
 
   /** Extract possible completions at the index of `cursor` in `expr` */
@@ -173,9 +173,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 =>
@@ -205,6 +202,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)
@@ -213,8 +213,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
@@ -311,7 +314,10 @@ class ReplDriver(settings: Array[String],
       initState
 
     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