Skip to content

Commit

Permalink
Merge pull request #14021 from dwijnand/repl-shadows-empty
Browse files Browse the repository at this point in the history
Fix REPL clashing with CWD artefacts
  • Loading branch information
griggt authored Dec 13, 2021
2 parents 6e37dec + 83633a9 commit 8e1054e
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 16 deletions.
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ 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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ class Typer extends Namer
*/
def isDefinedInCurrentUnit(denot: Denotation)(using Context): Boolean = denot match {
case MultiDenotation(d1, d2) => isDefinedInCurrentUnit(d1) || isDefinedInCurrentUnit(d2)
case denot: SingleDenotation => denot.symbol.source == ctx.compilationUnit.source
case denot: SingleDenotation => ctx.compilationUnit != null && denot.symbol.source == ctx.compilationUnit.source
}

/** Is `denot` the denotation of a self symbol? */
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/repl/Rendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
//at repl$.rs$line$2$.<clinit>(rs$line$2:1)
//at repl$.rs$line$2.res1(rs$line$2)
def isWrapperInitialization(ste: StackTraceElement) =
ste.getClassName.startsWith(nme.REPL_PACKAGE.toString + ".") // d.symbol.owner.name.show is simple name
ste.getClassName.startsWith(REPL_WRAPPER_NAME_PREFIX) // d.symbol.owner.name.show is simple name
&& (ste.getMethodName == nme.STATIC_CONSTRUCTOR.show || ste.getMethodName == nme.CONSTRUCTOR.show)

cause.formatStackTracePrefix(!isWrapperInitialization(_))
Expand All @@ -170,7 +170,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}"
final val REPL_WRAPPER_NAME_PREFIX = str.REPL_SESSION_LINE

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

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

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

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

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

ParseResult(sourceFile)(state) match {
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/repl/ScriptEngine.scala
Original file line number Diff line number Diff line change
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"${nme.REPL_PACKAGE}.${str.REPL_SESSION_LINE}$oid", true, rendering.classLoader()(using state.context))
Class.forName(s"${Rendering.REPL_WRAPPER_NAME_PREFIX}$oid", true, rendering.classLoader()(using state.context))
.getDeclaredMethods.find(_.getName == s"${str.REPL_RES_PREFIX}$vid")
.map(_.invoke(null))
.getOrElse(null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import org.junit.Assert._

trait ErrorMessagesTest extends DottyTest {

private def newContext = {
protected def newContext = {
val rep = new StoreReporter(null)
with UniqueMessagePositions with HideNonSensicalMessages
initialCtx.setReporter(rep).setSetting(ctx.settings.color, "never")
Expand Down
71 changes: 71 additions & 0 deletions compiler/test/dotty/tools/repl/ShadowingBatchTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package dotty.tools
package repl

import java.io.File
import java.nio.file.Files

import org.junit.{ After, AfterClass, BeforeClass, Ignore, Test }
import org.junit.Assert._
import io.{ Directory, PlainDirectory }
import dotc.core.Contexts._
import dotc.reporting.{ ErrorMessagesTest, StoreReporter }
import vulpix.TestConfiguration

object ShadowingBatchTests:
val dir = Directory(Files.createTempDirectory("batch-shadow"))

@BeforeClass def suiteStarting: Unit = dir.createDirectory()
@AfterClass def suiteFinished: Unit = dir.deleteRecursively()

class ShadowingBatchTests extends ErrorMessagesTest:
import ShadowingBatchTests._

@After def testFinished: Unit = dir.list.foreach(_.deleteRecursively())

val compiler = new dotc.Compiler()

override def initializeCtx(ictx: FreshContext) = inContext(ictx) {
super.initializeCtx(ictx)
val settings = ictx.settings; import settings._
ictx.setSetting(outputDir, new PlainDirectory(dir))
ictx.setSetting(classpath, classpath.value + File.pathSeparator + dir.jpath.toAbsolutePath)
}

@Test def file =
checkMessages("class C(val c: Int)").expectNoErrors
checkMessages("object rsline1 {\n def line1 = new C().c\n}").expect { (_, msgs) =>
assertMessageCount(1, msgs)
assertEquals("missing argument for parameter c of constructor C in class C: (c: Int): C", msgs.head.message)
}
checkMessages("object rsline2 {\n def line2 = new C(13).c\n}").expectNoErrors
checkMessages("object rsline3 {\n class C { val c = 42 }\n}").expectNoErrors
checkMessages("import rsline3._\nobject rsline4 {\n def line4 = new C().c\n}").expectNoErrors

@Test def directory =
checkMessages("package foo\nclass C").expectNoErrors
checkMessages("object rsline1 {\n def line1 = foo\n}").expect { (_, msgs) =>
assertMessageCount(1, msgs)
assertEquals("package foo is not a value", msgs.head.message)
}
checkMessages("object rsline2 {\n val foo = 2\n}").expectNoErrors
checkMessages("import rsline2._\nobject rsline3 {\n def line3 = foo\n}").expectNoErrors

@Test def directoryJava =
checkMessages("object rsline1 {\n def line1 = java\n}").expect { (_, msgs) =>
assertMessageCount(1, msgs)
assertEquals("package java is not a value", msgs.head.message)
}
checkMessages("object rsline2 {\n val java = 2\n}").expectNoErrors
checkMessages("import rsline2._\nobject rsline3 {\n def line3 = java\n}").expectNoErrors

def checkMessages(source: String): Report =
ctx = newContext
val run = compiler.newRun(using ctx.fresh)
run.compileFromStrings(List(source))
val runCtx = run.runContext
if runCtx.reporter.hasErrors then
val rep = runCtx.reporter.asInstanceOf[StoreReporter]
val msgs = rep.removeBufferedMessages(using runCtx).map(_.msg).reverse
new Report(msgs, runCtx)
else new EmptyReport
end ShadowingBatchTests
4 changes: 0 additions & 4 deletions compiler/test/dotty/tools/repl/ShadowingTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ class ShadowingTests extends ReplTest(options = ShadowingTests.options):
|""".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.
Expand Down Expand Up @@ -128,9 +127,6 @@ class ShadowingTests extends ReplTest(options = ShadowingTests.options):
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
Expand Down

0 comments on commit 8e1054e

Please sign in to comment.