From 6972f8e4eae12d6f82dd27c56f7a9bbdb8ebff95 Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Mon, 24 May 2021 21:46:02 -0700 Subject: [PATCH 1/2] Adapt REPL scripted test framework for in-memory scripts --- compiler/test/dotty/tools/repl/ReplTest.scala | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/test/dotty/tools/repl/ReplTest.scala b/compiler/test/dotty/tools/repl/ReplTest.scala index 0c2517dd0219..f338c7991e6b 100644 --- a/compiler/test/dotty/tools/repl/ReplTest.scala +++ b/compiler/test/dotty/tools/repl/ReplTest.scala @@ -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) = @@ -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 } @@ -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] @@ -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 } } From 655b7d961472847b3d28c19286be6aaa6e2425dc Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Sun, 2 May 2021 20:28:22 -0700 Subject: [PATCH 2/2] Put REPL wrappers in a (special) package Prior to this commit, the REPL wrapper objects were placed directly in the empty package. This prevented definitions in the REPL session from shadowing definitions in the empty package on the classpath (e.g. from stray .class files in the current directory). --- .../src/dotty/tools/dotc/core/StdNames.scala | 1 + compiler/src/dotty/tools/repl/Rendering.scala | 5 +- .../src/dotty/tools/repl/ReplCompiler.scala | 10 +- .../src/dotty/tools/repl/ScriptEngine.scala | 4 +- compiler/test-resources/repl/i7635 | 9 -- .../dotty/tools/repl/ShadowingTests.scala | 141 ++++++++++++++++++ 6 files changed, 153 insertions(+), 17 deletions(-) delete mode 100644 compiler/test-resources/repl/i7635 create mode 100644 compiler/test/dotty/tools/repl/ShadowingTests.scala diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index 23555a58abdd..5c718d4af0da 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -133,6 +133,7 @@ object StdNames { val OPS_PACKAGE: N = "" val OVERLOADED: N = "" val PACKAGE: N = "package" + val REPL_PACKAGE: N = "repl$" val ROOT: N = "" val SPECIALIZED_SUFFIX: N = "$sp" val SUPER_PREFIX: N = "super$" diff --git a/compiler/src/dotty/tools/repl/Rendering.scala b/compiler/src/dotty/tools/repl/Rendering.scala index a09ad8d962e1..17c876661e18 100644 --- a/compiler/src/dotty/tools/repl/Rendering.scala +++ b/compiler/src/dotty/tools/repl/Rendering.scala @@ -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 } @@ -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 = { diff --git a/compiler/src/dotty/tools/repl/ReplCompiler.scala b/compiler/src/dotty/tools/repl/ReplCompiler.scala index e81ea47cfff9..c11019fc0bc4 100644 --- a/compiler/src/dotty/tools/repl/ReplCompiler.scala +++ b/compiler/src/dotty/tools/repl/ReplCompiler.scala @@ -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) @@ -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)) } @@ -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 = { @@ -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 { diff --git a/compiler/src/dotty/tools/repl/ScriptEngine.scala b/compiler/src/dotty/tools/repl/ScriptEngine.scala index 8a1d3e7c2148..9c0ced8e5fa0 100644 --- a/compiler/src/dotty/tools/repl/ScriptEngine.scala +++ b/compiler/src/dotty/tools/repl/ScriptEngine.scala @@ -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. @@ -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) diff --git a/compiler/test-resources/repl/i7635 b/compiler/test-resources/repl/i7635 deleted file mode 100644 index 1c9e6474a768..000000000000 --- a/compiler/test-resources/repl/i7635 +++ /dev/null @@ -1,9 +0,0 @@ -scala> class C { protected val c = 42; override def toString() = s"C($c)" } -// defined class C -scala> val x = C() -val x: C = C(42) -scala> def foo = 3 -def foo: Int -scala> foo -val res0: Int = 3 -scala> import util.Try diff --git a/compiler/test/dotty/tools/repl/ShadowingTests.scala b/compiler/test/dotty/tools/repl/ShadowingTests.scala new file mode 100644 index 000000000000..037887602a44 --- /dev/null +++ b/compiler/test/dotty/tools/repl/ShadowingTests.scala @@ -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 = "", + 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 = "", + """|scala> val foo = 3 + |val foo: Int = 3 + | + |scala> foo + |val res0: Int = 3 + |""".stripMargin.linesIterator.toList + ) + + ShadowingTests.createSubDir("x") + testScript(name = "", + """|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 = "", + """|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