Skip to content
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

Fix REPL clashing with CWD artefacts #14021

Merged
merged 1 commit into from
Dec 13, 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: 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)
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
.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