Merge pull request #4051 from heathermiller/repl-cp-fix2

SI-6502 Reenables loading jars into the running REPL (regression in 2.10)

8 files changed, 354 insertions, 18 deletions

diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index 9cc9712b44..430424d0f8 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -8,12 +8,13 @@ package tools
 package nsc
 
 import java.io.{ File, FileOutputStream, PrintWriter, IOException, FileNotFoundException }
+import java.net.URL
 import java.nio.charset.{ Charset, CharsetDecoder, IllegalCharsetNameException, UnsupportedCharsetException }
 import scala.compat.Platform.currentTime
 import scala.collection.{ mutable, immutable }
 import io.{ SourceReader, AbstractFile, Path }
 import reporters.{ Reporter, ConsoleReporter }
-import util.{ ClassPath, StatisticsInfo, returning, stackTraceString }
+import util.{ ClassPath, MergedClassPath, StatisticsInfo, returning, stackTraceString }
 import scala.reflect.ClassTag
 import scala.reflect.internal.util.{ OffsetPosition, SourceFile, NoSourceFile, BatchSourceFile, ScriptSourceFile }
 import scala.reflect.internal.pickling.{ PickleBuffer, PickleFormat }
@@ -841,6 +842,150 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
     } reverse
   }
 
+  // ------------ REPL utilities ---------------------------------
+
+  /** Extend classpath of `platform` and rescan updated packages. */
+  def extendCompilerClassPath(urls: URL*): Unit = {
+    val newClassPath = platform.classPath.mergeUrlsIntoClassPath(urls: _*)
+    platform.currentClassPath = Some(newClassPath)
+    // Reload all specified jars into this compiler instance
+    invalidateClassPathEntries(urls.map(_.getPath): _*)
+  }
+
+  // ------------ Invalidations ---------------------------------
+
+  /** Is given package class a system package class that cannot be invalidated?
+   */
+  private def isSystemPackageClass(pkg: Symbol) =
+    pkg == RootClass || (pkg.hasTransOwner(definitions.ScalaPackageClass) && !pkg.hasTransOwner(this.rootMirror.staticPackage("scala.tools").moduleClass.asClass))
+
+  /** Invalidates packages that contain classes defined in a classpath entry, and
+   *  rescans that entry.
+   *
+   *  First, the classpath entry referred to by one of the `paths` is rescanned,
+   *  so that any new files or changes in subpackages are picked up.
+   *  Second, any packages for which one of the following conditions is met is invalidated:
+   *   - the classpath entry contained during the last compilation run now contains classfiles
+   *     that represent a member in the package;
+   *   - the classpath entry now contains classfiles that represent a member in the package;
+   *   - the set of subpackages has changed.
+   *
+   *  The invalidated packages are reset in their entirety; all member classes and member packages
+   *  are re-accessed using the new classpath.
+   *
+   *  System packages that the compiler needs to access as part of standard definitions
+   *  are not invalidated. A system package is:
+   *  Any package rooted in "scala", with the exception of packages rooted in "scala.tools".
+   *
+   *  @param paths  Fully-qualified names that refer to directories or jar files that are
+   *                entries on the classpath.
+   */
+  def invalidateClassPathEntries(paths: String*): Unit = {
+    implicit object ClassPathOrdering extends Ordering[PlatformClassPath] {
+      def compare(a:PlatformClassPath, b:PlatformClassPath) = a.asClasspathString compare b.asClasspathString
+    }
+    val invalidated, failed = new mutable.ListBuffer[ClassSymbol]
+    classPath match {
+      case cp: MergedClassPath[_] =>
+        def assoc(path: String): List[(PlatformClassPath, PlatformClassPath)] = {
+          val dir = AbstractFile.getDirectory(path)
+          val canonical = dir.canonicalPath
+          def matchesCanonical(e: ClassPath[_]) = e.origin match {
+            case Some(opath) =>
+              AbstractFile.getDirectory(opath).canonicalPath == canonical
+            case None =>
+              false
+          }
+          cp.entries find matchesCanonical match {
+            case Some(oldEntry) =>
+              List(oldEntry -> cp.context.newClassPath(dir))
+            case None =>
+              error(s"Error adding entry to classpath. During invalidation, no entry named $path in classpath $classPath")
+              List()
+          }
+        }
+        val subst = immutable.TreeMap(paths flatMap assoc: _*)
+        if (subst.nonEmpty) {
+          platform updateClassPath subst
+          informProgress(s"classpath updated on entries [${subst.keys mkString ","}]")
+          def mkClassPath(elems: Iterable[PlatformClassPath]): PlatformClassPath =
+            if (elems.size == 1) elems.head
+            else new MergedClassPath(elems, classPath.context)
+          val oldEntries = mkClassPath(subst.keys)
+          val newEntries = mkClassPath(subst.values)
+          mergeNewEntries(newEntries, RootClass, Some(classPath), Some(oldEntries), invalidated, failed)
+        }
+    }
+    def show(msg: String, syms: scala.collection.Traversable[Symbol]) =
+      if (syms.nonEmpty)
+        informProgress(s"$msg: ${syms map (_.fullName) mkString ","}")
+    show("invalidated packages", invalidated)
+    show("could not invalidate system packages", failed)
+  }
+
+  /** Merges new classpath entries into the symbol table
+   *
+   *  @param newEntries   The new classpath entries
+   *  @param root         The root symbol to be resynced (a package class)
+   *  @param allEntries   Optionally, the corresponding package in the complete current classpath
+   *  @param oldEntries   Optionally, the corresponding package in the old classpath entries
+   *  @param invalidated  A listbuffer collecting the invalidated package classes
+   *  @param failed       A listbuffer collecting system package classes which could not be invalidated
+   *
+   * The merging strategy is determined by the absence or presence of classes and packages.
+   *
+   * If either oldEntries or newEntries contains classes, root is invalidated provided that a corresponding package
+   * exists in allEntries. Otherwise it is removed.
+   * Otherwise, the action is determined by the following matrix, with columns:
+   *
+   *      old sym   action
+   *       +   +    recurse into all child packages of newEntries
+   *       -   +    invalidate root
+   *       -   -    create and enter root
+   *
+   *  Here, old means classpath, and sym means symboltable. + is presence of an entry in its column, - is absence.
+   */
+  private def mergeNewEntries(newEntries: PlatformClassPath, root: ClassSymbol,
+             allEntries: OptClassPath, oldEntries: OptClassPath,
+             invalidated: mutable.ListBuffer[ClassSymbol], failed: mutable.ListBuffer[ClassSymbol]) {
+    ifDebug(informProgress(s"syncing $root, $oldEntries -> $newEntries"))
+
+    val getName: ClassPath[AbstractFile] => String = (_.name)
+    def hasClasses(cp: OptClassPath) = cp.isDefined && cp.get.classes.nonEmpty
+    def invalidateOrRemove(root: ClassSymbol) = {
+      allEntries match {
+        case Some(cp) => root setInfo new loaders.PackageLoader(cp)
+        case None => root.owner.info.decls unlink root.sourceModule
+      }
+      invalidated += root
+    }
+    def subPackage(cp: PlatformClassPath, name: String): OptClassPath =
+      cp.packages find (cp1 => getName(cp1) == name)
+
+    val classesFound = hasClasses(oldEntries) || newEntries.classes.nonEmpty
+    if (classesFound && !isSystemPackageClass(root)) {
+      invalidateOrRemove(root)
+    } else {
+      if (classesFound) {
+        if (root.isRoot) invalidateOrRemove(EmptyPackageClass)
+        else failed += root
+      }
+      if (!oldEntries.isDefined) invalidateOrRemove(root)
+      else
+        for (pstr <- newEntries.packages.map(getName)) {
+          val pname = newTermName(pstr)
+          val pkg = (root.info decl pname) orElse {
+            // package does not exist in symbol table, create symbol to track it
+            assert(!subPackage(oldEntries.get, pstr).isDefined)
+            loaders.enterPackage(root, pstr, new loaders.PackageLoader(allEntries.get))
+          }
+          mergeNewEntries(subPackage(newEntries, pstr).get, pkg.moduleClass.asClass,
+                          subPackage(allEntries.get, pstr), subPackage(oldEntries.get, pstr),
+                          invalidated, failed)
+        }
+    }
+  }
+
   // ----------- Runs ---------------------------------------
 
   private var curRun: Run = null
diff --git a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
index 7236bf70d5..4877bd9b80 100644
--- a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
+++ b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
@@ -16,7 +16,7 @@ trait JavaPlatform extends Platform {
   import global._
   import definitions._
 
-  private var currentClassPath: Option[MergedClassPath[AbstractFile]] = None
+  private[nsc] var currentClassPath: Option[MergedClassPath[AbstractFile]] = None
 
   def classPath: ClassPath[AbstractFile] = {
     if (currentClassPath.isEmpty) currentClassPath = Some(new PathResolver(settings).result)
diff --git a/src/compiler/scala/tools/nsc/util/ClassPath.scala b/src/compiler/scala/tools/nsc/util/ClassPath.scala
index e89f08ec6b..e78dee5eee 100644
--- a/src/compiler/scala/tools/nsc/util/ClassPath.scala
+++ b/src/compiler/scala/tools/nsc/util/ClassPath.scala
@@ -197,6 +197,23 @@ abstract class ClassPath[T] {
   def packages: IndexedSeq[ClassPath[T]]
   def sourcepaths: IndexedSeq[AbstractFile]
 
+  /** The entries this classpath is composed of. In class `ClassPath` it's just the singleton list containing `this`.
+   *  Subclasses such as `MergedClassPath` typically return lists with more elements.
+   */
+  def entries: IndexedSeq[ClassPath[T]] = IndexedSeq(this)
+
+  /** Merge classpath of `platform` and `urls` into merged classpath */
+  def mergeUrlsIntoClassPath(urls: URL*): MergedClassPath[T] = {
+    // Collect our new jars/directories and add them to the existing set of classpaths
+    val allEntries =
+      (entries ++
+       urls.map(url => context.newClassPath(io.AbstractFile.getURL(url)))
+      ).distinct
+
+    // Combine all of our classpaths (old and new) into one merged classpath
+    new MergedClassPath(allEntries, context)
+  }
+
   /**
    * Represents classes which can be loaded with a ClassfileLoader and/or SourcefileLoader.
    */
@@ -322,7 +339,7 @@ extends MergedClassPath[T](original.entries map (e => subst getOrElse (e, e)), o
  * A classpath unifying multiple class- and sourcepath entries.
  */
 class MergedClassPath[T](
-  val entries: IndexedSeq[ClassPath[T]],
+  override val entries: IndexedSeq[ClassPath[T]],
   val context: ClassPathContext[T])
 extends ClassPath[T] {
   def this(entries: TraversableOnce[ClassPath[T]], context: ClassPathContext[T]) =
diff --git a/src/reflect/scala/reflect/io/AbstractFile.scala b/src/reflect/scala/reflect/io/AbstractFile.scala
index ac1159b2ac..bcefcc471f 100644
--- a/src/reflect/scala/reflect/io/AbstractFile.scala
+++ b/src/reflect/scala/reflect/io/AbstractFile.scala
@@ -48,14 +48,16 @@ object AbstractFile {
     else null
 
   /**
-   * If the specified URL exists and is a readable zip or jar archive,
-   * returns an abstract directory backed by it. Otherwise, returns
-   * `null`.
+   * If the specified URL exists and is a regular file or a directory, returns an
+   * abstract regular file or an abstract directory, respectively, backed by it.
+   * Otherwise, returns `null`.
    */
-  def getURL(url: URL): AbstractFile = {
-    if (url == null || !Path.isExtensionJarOrZip(url.getPath)) null
-    else ZipArchive fromURL url
-  }
+  def getURL(url: URL): AbstractFile =
+    if (url.getProtocol == "file") {
+      val f = new java.io.File(url.getPath)
+      if (f.isDirectory) getDirectory(f)
+      else getFile(f)
+    } else null
 
   def getResources(url: URL): AbstractFile = ZipArchive fromManifestURL url
 }
diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala
index 1c751c8a9f..8bef424e2b 100644
--- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala
+++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala
@@ -19,6 +19,7 @@ import scala.reflect.internal.util.{ BatchSourceFile, ScalaClassLoader }
 import ScalaClassLoader._
 import scala.reflect.io.{ File, Directory }
 import scala.tools.util._
+import io.AbstractFile
 import scala.collection.generic.Clearable
 import scala.concurrent.{ ExecutionContext, Await, Future, future }
 import ExecutionContext.Implicits._
@@ -221,7 +222,7 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter)
     nullary("power", "enable power user mode", powerCmd),
     nullary("quit", "exit the interpreter", () => Result(keepRunning = false, None)),
     cmd("replay", "[options]", "reset the repl and replay all previous commands", replayCommand),
-    //cmd("require", "<path>", "add a jar or directory to the classpath", require),  // TODO
+    cmd("require", "<path>", "add a jar to the classpath", require),
     cmd("reset", "[options]", "reset the repl to its initial state, forgetting all session entries", resetCommand),
     cmd("save", "<path>", "save replayable session to a file", saveCommand),
     shCommand,
@@ -620,13 +621,57 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter)
     val f = File(arg).normalize
     if (f.exists) {
       addedClasspath = ClassPath.join(addedClasspath, f.path)
-      val totalClasspath = ClassPath.join(settings.classpath.value, addedClasspath)
-      echo("Added '%s'.  Your new classpath is:\n\"%s\"".format(f.path, totalClasspath))
-      replay()
+      intp.addUrlsToClassPath(f.toURI.toURL)
+      echo("Added '%s' to classpath.".format(f.path, intp.global.classPath.asClasspathString))
+      repldbg("Added '%s'.  Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString))
     }
     else echo("The path '" + f + "' doesn't seem to exist.")
   }
 
+  /** Adds jar file to the current classpath. Jar will only be added if it
+   *  does not contain classes that already exist on the current classpath.
+   *
+   *  Importantly, `require` adds jars to the classpath ''without'' resetting
+   *  the state of the interpreter. This is in contrast to `replay` which can
+   *  be used to add jars to the classpath and which creates a new instance of
+   *  the interpreter and replays all interpreter expressions.
+   */
+  def require(arg: String): Unit = {
+    class InfoClassLoader extends java.lang.ClassLoader {
+      def classOf(arr: Array[Byte]): Class[_] =
+        super.defineClass(null, arr, 0, arr.length)
+    }
+
+    val f = File(arg).normalize
+
+    if (f.isDirectory) {
+      echo("Adding directories to the classpath is not supported. Add a jar instead.")
+      return
+    }
+
+    val jarFile = AbstractFile.getDirectory(new java.io.File(arg))
+
+    def flatten(f: AbstractFile): Iterator[AbstractFile] =
+      if (f.isClassContainer) f.iterator.flatMap(flatten)
+      else Iterator(f)
+
+    val entries = flatten(jarFile)
+    val cloader = new InfoClassLoader
+
+    def classNameOf(classFile: AbstractFile): String = cloader.classOf(classFile.toByteArray).getName
+    def alreadyDefined(clsName: String) = intp.classLoader.tryToLoadClass(clsName).isDefined
+    val exists = entries.filter(_.hasExtension("class")).map(classNameOf).exists(alreadyDefined)
+
+    if (!f.exists) echo(s"The path '$f' doesn't seem to exist.")
+    else if (exists) echo(s"The path '$f' cannot be loaded, because existing classpath entries conflict.") // TODO tell me which one
+    else {
+      addedClasspath = ClassPath.join(addedClasspath, f.path)
+      intp.addUrlsToClassPath(f.toURI.toURL)
+      echo("Added '%s' to classpath.".format(f.path, intp.global.classPath.asClasspathString))
+      repldbg("Added '%s'.  Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString))
+    }
+  }
+
   def powerCmd(): Result = {
     if (isReplPower) "Already in power mode."
     else enablePowerMode(isDuringInit = false)
diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala
index 3f4922a602..b990e401ec 100644
--- a/src/repl/scala/tools/nsc/interpreter/IMain.scala
+++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala
@@ -18,9 +18,13 @@ import scala.reflect.internal.util.{ BatchSourceFile, SourceFile }
 import scala.tools.util.PathResolver
 import scala.tools.nsc.io.AbstractFile
 import scala.tools.nsc.typechecker.{ TypeStrings, StructuredTypeStrings }
-import scala.tools.nsc.util.{ ScalaClassLoader, stringFromReader, stringFromWriter, StackTraceOps }
+import scala.tools.nsc.util.{ ScalaClassLoader, stringFromReader, stringFromWriter, StackTraceOps, ClassPath, MergedClassPath }
+import ScalaClassLoader.URLClassLoader
 import scala.tools.nsc.util.Exceptional.unwrap
+import scala.tools.nsc.backend.JavaPlatform
 import javax.script.{AbstractScriptEngine, Bindings, ScriptContext, ScriptEngine, ScriptEngineFactory, ScriptException, CompiledScript, Compilable}
+import java.net.URL
+import java.io.File
 
 /** An interpreter for Scala code.
  *
@@ -82,6 +86,8 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
   private var _classLoader: util.AbstractFileClassLoader = null                              // active classloader
   private val _compiler: ReplGlobal                 = newCompiler(settings, reporter)   // our private compiler
 
+  private var _runtimeClassLoader: URLClassLoader = null              // wrapper exposing addURL
+
   def compilerClasspath: Seq[java.net.URL] = (
     if (isInitializeComplete) global.classPath.asURLs
     else new PathResolver(settings).result.asURLs  // the compiler's classpath
@@ -237,6 +243,18 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     new Global(settings, reporter) with ReplGlobal { override def toString: String = "<global>" }
   }
 
+  /**
+   * Adds all specified jars to the compile and runtime classpaths.
+   *
+   * @note  Currently only supports jars, not directories.
+   * @param urls The list of items to add to the compile and runtime classpaths.
+   */
+  def addUrlsToClassPath(urls: URL*): Unit = {
+    new Run //  force some initialization
+    urls.foreach(_runtimeClassLoader.addURL) // Add jars to runtime classloader
+    global.extendCompilerClassPath(urls: _*) // Add jars to compile-time classpath
+  }
+
   /** Parent classloader.  Overridable. */
   protected def parentClassLoader: ClassLoader =
     settings.explicitParentLoader.getOrElse( this.getClass.getClassLoader() )
@@ -329,9 +347,9 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     }
   }
   private def makeClassLoader(): util.AbstractFileClassLoader =
-    new TranslatingClassLoader(parentClassLoader match {
-      case null   => ScalaClassLoader fromURLs compilerClasspath
-      case p      => new ScalaClassLoader.URLClassLoader(compilerClasspath, p)
+    new TranslatingClassLoader({
+      _runtimeClassLoader = new URLClassLoader(compilerClasspath, parentClassLoader)
+      _runtimeClassLoader
     })
 
   // Set the current Java "context" class loader to this interpreter's class loader
diff --git a/test/files/run/t6502.check b/test/files/run/t6502.check
new file mode 100644
index 0000000000..95d36ee221
--- /dev/null
+++ b/test/files/run/t6502.check
@@ -0,0 +1,8 @@
+test1 res1: true
+test1 res2: true
+test2 res1: true
+test2 res2: true
+test3 res1: true
+test3 res2: true
+test4 res1: true
+test4 res2: true
diff --git a/test/files/run/t6502.scala b/test/files/run/t6502.scala
new file mode 100644
index 0000000000..ced1b5812d
--- /dev/null
+++ b/test/files/run/t6502.scala
@@ -0,0 +1,101 @@
+import scala.tools.partest._
+import java.io.File
+import scala.tools.nsc.interpreter.ILoop
+
+object Test extends StoreReporterDirectTest {
+  def code = ???
+
+  def compileCode(code: String, jarFileName: String) = {
+    val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
+    compileString(newCompiler("-cp", classpath, "-d", s"${testOutput.path}/$jarFileName"))(code)
+  }
+
+  def app1 = """
+    package test
+
+    object Test extends App {
+      def test(): Unit = {
+        println("testing...")
+      }
+    }"""
+
+  def app2 = """
+    package test
+
+    object Test extends App {
+      def test(): Unit = {
+        println("testing differently...")
+      }
+    }"""
+
+  def app3 = """
+    package test
+
+    object Test3 extends App {
+      def test(): Unit = {
+        println("new object in existing package")
+      }
+    }"""
+
+  def test1(): Unit = {
+    val jar = "test1.jar"
+    compileCode(app1, jar)
+
+    val output = ILoop.run(List(s":require ${testOutput.path}/$jar", "test.Test.test()"))
+    val lines  = output.split("\n")
+    val res1   = lines(4).contains("Added") && lines(4).contains("test1.jar")
+    val res2   = lines(lines.length-3).contains("testing...")
+
+    println(s"test1 res1: $res1")
+    println(s"test1 res2: $res2")
+  }
+
+  def test2(): Unit = {
+    // should reject jars with conflicting entries
+    val jar1 = "test1.jar"
+    val jar2 = "test2.jar"
+    compileCode(app2, jar2)
+
+    val output = ILoop.run(List(s":require ${testOutput.path}/$jar1", s":require ${testOutput.path}/$jar2"))
+    val lines  = output.split("\n")
+    val res1   = lines(4).contains("Added") && lines(4).contains("test1.jar")
+    val res2   = lines(lines.length-3).contains("test2.jar") && lines(lines.length-3).contains("existing classpath entries conflict")
+
+    println(s"test2 res1: $res1")
+    println(s"test2 res2: $res2")
+  }
+
+  def test3(): Unit = {
+    // should accept jars with overlapping packages, but no conflicts
+    val jar1 = "test1.jar"
+    val jar3 = "test3.jar"
+    compileCode(app3, jar3)
+
+    val output = ILoop.run(List(s":require ${testOutput.path}/$jar1", s":require ${testOutput.path}/$jar3", "test.Test3.test()"))
+    val lines  = output.split("\n")
+    val res1   = lines(4).contains("Added") && lines(4).contains("test1.jar")
+    val res2   = lines(lines.length-3).contains("new object in existing package")
+
+    println(s"test3 res1: $res1")
+    println(s"test3 res2: $res2")
+  }
+
+  def test4(): Unit = {
+    // twice the same jar should be rejected
+    val jar1   = "test1.jar"
+    val output = ILoop.run(List(s":require ${testOutput.path}/$jar1", s":require ${testOutput.path}/$jar1"))
+    val lines  = output.split("\n")
+    val res1   = lines(4).contains("Added") && lines(4).contains("test1.jar")
+    val res2   = lines(lines.length-3).contains("test1.jar") && lines(lines.length-3).contains("existing classpath entries conflict")
+
+    println(s"test4 res1: $res1")
+    println(s"test4 res2: $res2")
+  }
+
+  def show(): Unit = {
+    test1()
+    test2()
+    test3()
+    test4()
+  }
+}