Skip to content

Fix #7635: Package REPL wrappers #12607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ object StdNames {
val OPS_PACKAGE: N = "<special-ops>"
val OVERLOADED: N = "<overloaded>"
val PACKAGE: N = "package"
val REPL_PACKAGE: N = "repl$"
val ROOT: N = "<root>"
val SPECIALIZED_SUFFIX: N = "$sp"
val SUPER_PREFIX: N = "super$"
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/repl/Rendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
None
else
string.map { s =>
if (s.startsWith(str.REPL_SESSION_LINE))
s.drop(str.REPL_SESSION_LINE.length).dropWhile(c => c.isDigit || c == '$')
if (s.startsWith(REPL_WRAPPER_NAME_PREFIX))
s.drop(REPL_WRAPPER_NAME_PREFIX.length).dropWhile(c => c.isDigit || c == '$')
else
s
}
Expand Down Expand Up @@ -180,6 +180,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
}

object Rendering {
final val REPL_WRAPPER_NAME_PREFIX = s"${nme.REPL_PACKAGE}.${str.REPL_SESSION_LINE}"

extension (s: Symbol)
def showUser(using Context): String = {
Expand Down
10 changes: 6 additions & 4 deletions compiler/src/dotty/tools/repl/ReplCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ReplCompiler extends Compiler {

def importPreviousRun(id: Int)(using Context) = {
// we first import the wrapper object id
val path = nme.EMPTY_PACKAGE ++ "." ++ objectNames(id)
val path = nme.REPL_PACKAGE ++ "." ++ objectNames(id)
val ctx0 = ctx.fresh
.setNewScope
.withRootImports(RootRef(() => requiredModuleRef(path)) :: Nil)
Expand All @@ -58,7 +58,9 @@ class ReplCompiler extends Compiler {
importContext(imp)(using ctx))
}

val rootCtx = super.rootContext.withRootImports
val rootCtx = super.rootContext
.withRootImports // default root imports
.withRootImports(RootRef(() => defn.EmptyPackageVal.termRef) :: Nil)
(1 to state.objectIndex).foldLeft(rootCtx)((ctx, id) =>
importPreviousRun(id)(using ctx))
}
Expand Down Expand Up @@ -130,7 +132,7 @@ class ReplCompiler extends Compiler {
val module = ModuleDef(objectTermName, tmpl)
.withSpan(span)

PackageDef(Ident(nme.EMPTY_PACKAGE), List(module))
PackageDef(Ident(nme.REPL_PACKAGE), List(module))
}

private def createUnit(defs: Definitions, span: Span)(using Context): CompilationUnit = {
Expand Down Expand Up @@ -231,7 +233,7 @@ class ReplCompiler extends Compiler {
val wrapper = TypeDef("$wrapper".toTypeName, tmpl)
.withMods(Modifiers(Final))
.withSpan(Span(0, expr.length))
PackageDef(Ident(nme.EMPTY_PACKAGE), List(wrapper))
PackageDef(Ident(nme.REPL_PACKAGE), List(wrapper))
}

ParseResult(sourceFile)(state) match {
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/repl/ScriptEngine.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package repl

import java.io.{Reader, StringWriter}
import javax.script.{AbstractScriptEngine, Bindings, ScriptContext, ScriptEngine => JScriptEngine, ScriptEngineFactory, ScriptException, SimpleBindings}
import dotc.core.StdNames.str
import dotc.core.StdNames.{nme, str}

/** A JSR 223 (Scripting API) compatible wrapper around the REPL for improved
* interoperability with software that supports it.
Expand Down Expand Up @@ -37,7 +37,7 @@ class ScriptEngine extends AbstractScriptEngine {
val vid = state.valIndex
state = driver.run(script)(state)
val oid = state.objectIndex
Class.forName(s"${str.REPL_SESSION_LINE}$oid", true, rendering.classLoader()(using state.context))
Class.forName(s"${nme.REPL_PACKAGE}.${str.REPL_SESSION_LINE}$oid", true, rendering.classLoader()(using state.context))
.getDeclaredMethods.find(_.getName == s"${str.REPL_RES_PREFIX}$vid")
.map(_.invoke(null))
.getOrElse(null)
Expand Down
9 changes: 0 additions & 9 deletions compiler/test-resources/repl/i7635

This file was deleted.

13 changes: 7 additions & 6 deletions compiler/test/dotty/tools/repl/ReplTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ extends ReplDriver(options, new PrintStream(out, true, StandardCharsets.UTF_8.na
extension [A](state: State)
def andThen(op: State => A): A = op(state)

def testFile(f: JFile): Unit = {
def testFile(f: JFile): Unit = testScript(f.toString, readLines(f))

def testScript(name: => String, lines: List[String]): Unit = {
val prompt = "scala>"

def evaluate(state: State, input: String) =
Expand All @@ -50,7 +52,7 @@ extends ReplDriver(options, new PrintStream(out, true, StandardCharsets.UTF_8.na
}
catch {
case ex: Throwable =>
System.err.println(s"failed while running script: $f, on:\n$input")
System.err.println(s"failed while running script: $name, on:\n$input")
throw ex
}

Expand All @@ -60,13 +62,12 @@ extends ReplDriver(options, new PrintStream(out, true, StandardCharsets.UTF_8.na
case nonEmptyLine => nonEmptyLine :: Nil
}

val expectedOutput = readLines(f).flatMap(filterEmpties)
val expectedOutput = lines.flatMap(filterEmpties)
val actualOutput = {
resetToInitial()

val lines = readLines(f)
assert(lines.head.startsWith(prompt),
s"""Each file has to start with the prompt: "$prompt"""")
s"""Each script must start with the prompt: "$prompt"""")
val inputRes = lines.filter(_.startsWith(prompt))

val buf = new ArrayBuffer[String]
Expand All @@ -88,7 +89,7 @@ extends ReplDriver(options, new PrintStream(out, true, StandardCharsets.UTF_8.na
println("actual ===========>")
println(actualOutput.mkString(EOL))

fail(s"Error in file $f, expected output did not match actual")
fail(s"Error in script $name, expected output did not match actual")
end if
}
}
Expand Down
141 changes: 141 additions & 0 deletions compiler/test/dotty/tools/repl/ShadowingTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package dotty.tools
package repl

import java.io.File
import java.nio.file.{Path, Files}
import java.util.Comparator

import org.junit.{Test, Ignore, BeforeClass, AfterClass}

import dotc.Driver
import dotc.reporting.TestReporter
import dotc.interfaces.Diagnostic.ERROR
import vulpix.{TestConfiguration, TestFlags}

/** Test that the REPL can shadow artifacts in the local filesystem on the classpath.
* Since the REPL launches with the current directory on the classpath, stray .class
* files containing definitions in the empty package will be in scope in the REPL.
* Additionally, any subdirectories will be treated as package names in scope.
* As this may come as a surprise to an unsuspecting user, we would like definitions
* from the REPL session to shadow these names.
*
* Provided here is a framework for creating the filesystem artifacts to be shadowed
* and running scripted REPL tests with them on the claspath.
*/
object ShadowingTests:
def classpath = TestConfiguration.basicClasspath + File.pathSeparator + shadowDir
def options = ReplTest.commonOptions ++ Array("-classpath", classpath)
def shadowDir = dir.toAbsolutePath.toString

def createSubDir(name: String): Path =
val subdir = dir.resolve(name)
try Files.createDirectory(subdir)
catch case _: java.nio.file.FileAlreadyExistsException =>
assert(Files.isDirectory(subdir), s"failed to create shadowed subdirectory $subdir")
subdir

// The directory on the classpath containing artifacts to be shadowed
private var dir: Path = null

@BeforeClass def setupDir: Unit =
dir = Files.createTempDirectory("repl-shadow")

@AfterClass def tearDownDir: Unit =
Files.walk(dir).sorted(Comparator.reverseOrder).forEach(Files.delete)
dir = null

class ShadowingTests extends ReplTest(options = ShadowingTests.options):
// delete contents of shadowDir after each test
override def cleanup: Unit =
super.cleanup
val dir = ShadowingTests.dir
Files.walk(dir)
.filter(_ != dir)
.sorted(Comparator.reverseOrder)
.forEach(Files.delete)

/** Run a scripted REPL test with the compilation artifacts of `shadowed` on the classpath */
def shadowedScriptedTest(name: String, shadowed: String, script: String): Unit =
compileShadowed(shadowed)
testScript(name, script.linesIterator.toList)

/** Compile the given source text and output to the shadow dir on the classpath */
private def compileShadowed(src: String): Unit =
val file: Path = Files.createTempFile("repl-shadow-test", ".scala")
Files.write(file, src.getBytes)

val flags =
TestFlags(TestConfiguration.basicClasspath, TestConfiguration.noCheckOptions)
.and("-d", ShadowingTests.shadowDir)
val driver = new Driver
val reporter = TestReporter.reporter(System.out, logLevel = ERROR)
driver.process(flags.all :+ file.toString, reporter)
assert(!reporter.hasErrors, s"compilation of $file failed")
Files.delete(file)
end compileShadowed

@Test def i7635 = shadowedScriptedTest(name = "<i7635>",
shadowed = "class C(val c: Int)",
script =
"""|scala> new C().c
|1 | new C().c
| | ^^^^^^^
| | missing argument for parameter c of constructor C in class C: (c: Int): C
|
|scala> new C(13).c
|val res0: Int = 13
|
|scala> class C { val c = 42 }
|// defined class C
|
|scala> new C().c
|val res1: Int = 42
|""".stripMargin
)

@Ignore("not yet fixed")
@Test def `shadow subdirectories on classpath` =
// NB: Tests of shadowing of subdirectories on the classpath are only valid
// when the subdirectories exist prior to initialization of the REPL driver.
// In the tests below this is enforced by the call to `testScript` which
// in turn invokes `ReplDriver#resetToInitial`. When testing interactively,
// the subdirectories may be created before launching the REPL, or during
// an existing session followed by the `:reset` command.

ShadowingTests.createSubDir("foo")
testScript(name = "<shadow-subdir-foo>",
"""|scala> val foo = 3
|val foo: Int = 3
|
|scala> foo
|val res0: Int = 3
|""".stripMargin.linesIterator.toList
)

ShadowingTests.createSubDir("x")
testScript(name = "<shadow-subdir-x>",
"""|scala> val (x, y) = (42, "foo")
|val x: Int = 42
|val y: String = foo
|
|scala> if (true) x else y
|val res0: Matchable = 42
|""".stripMargin.linesIterator.toList
)

ShadowingTests.createSubDir("util")
testScript(name = "<shadow-subdir-util>",
"""|scala> import util.Try
|1 | import util.Try
| | ^^^
| | value Try is not a member of util
|
|scala> object util { class Try { override def toString = "you've gotta try!" } }
|// defined object util
|
|scala> import util.Try
|scala> new Try
|val res0: util.Try = you've gotta try!
|""".stripMargin.linesIterator.toList
)
end ShadowingTests