Skip to content

Commit d85f06a

Browse files
committed
Fix REPL clashing with CWD artefacts
This reverts the change that moves all the REPL's artefacts into the "repl$" package, moving everything back to the empty package. Then it fixes the class name and package name shadowing issue, by simulating what the batch compiler does when it compiles a source file without a package statement: it considers the sources to be in the empty package. Then the imports for the previous runs and the user-defined imports will be in sub-contexts of the empty package. The reason the working directory artefacts were shadowing the REPL artefacts was because before the imports were being added to sub-contexts of the root package, afterwhich the REPL tree was being wrapped in the empty package. That meant that the classes found in the empty package (i.e. classfiles in the working directory, or even directories) had precedence over the wildcard imports for the previous runs. The tests now cascade up into contexts that don't have the current compilation unit stored on them, so I defaulted the InitialContext to a NoCompilationUnit, because that's the precedent and NPEs kill kittens. Also, I developed a variant of ShadowingTests, using the batch compiler, to automate the expectations of how the shadowing should and shouldn't work.
1 parent ca483f8 commit d85f06a

File tree

9 files changed

+86
-15
lines changed

9 files changed

+86
-15
lines changed

compiler/src/dotty/tools/dotc/CompilationUnit.scala

+4
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import transform.SymUtils._
1515
import core.Decorators._
1616
import config.SourceVersion
1717

18+
import scala.annotation.internal.sharable
19+
1820
class CompilationUnit protected (val source: SourceFile) {
1921

2022
override def toString: String = source.toString
@@ -142,3 +144,5 @@ object CompilationUnit {
142144
}
143145
}
144146
}
147+
148+
@sharable object NoCompilationUnit extends CompilationUnit(NoSource)

compiler/src/dotty/tools/dotc/core/Contexts.scala

+1
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,7 @@ object Contexts {
806806
source = NoSource
807807
store = initialStore
808808
.updated(settingsStateLoc, settingsGroup.defaultState)
809+
.updated(compilationUnitLoc, NoCompilationUnit)
809810
.updated(notNullInfosLoc, Nil)
810811
searchHistory = new SearchRoot
811812
gadt = EmptyGadtConstraint

compiler/src/dotty/tools/dotc/core/StdNames.scala

-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ object StdNames {
133133
val OPS_PACKAGE: N = "<special-ops>"
134134
val OVERLOADED: N = "<overloaded>"
135135
val PACKAGE: N = "package"
136-
val REPL_PACKAGE: N = "repl$"
137136
val ROOT: N = "<root>"
138137
val SPECIALIZED_SUFFIX: N = "$sp"
139138
val SUPER_PREFIX: N = "super$"

compiler/src/dotty/tools/repl/Rendering.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
158158
//at repl$.rs$line$2$.<clinit>(rs$line$2:1)
159159
//at repl$.rs$line$2.res1(rs$line$2)
160160
def isWrapperInitialization(ste: StackTraceElement) =
161-
ste.getClassName.startsWith(nme.REPL_PACKAGE.toString + ".") // d.symbol.owner.name.show is simple name
161+
ste.getClassName.startsWith(REPL_WRAPPER_NAME_PREFIX) // d.symbol.owner.name.show is simple name
162162
&& (ste.getMethodName == nme.STATIC_CONSTRUCTOR.show || ste.getMethodName == nme.CONSTRUCTOR.show)
163163

164164
cause.formatStackTracePrefix(!isWrapperInitialization(_))
@@ -170,7 +170,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
170170
}
171171

172172
object Rendering {
173-
final val REPL_WRAPPER_NAME_PREFIX = s"${nme.REPL_PACKAGE}.${str.REPL_SESSION_LINE}"
173+
final val REPL_WRAPPER_NAME_PREFIX = str.REPL_SESSION_LINE
174174

175175
extension (s: Symbol)
176176
def showUser(using Context): String = {

compiler/src/dotty/tools/repl/ReplCompiler.scala

+6-6
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class ReplCompiler extends Compiler {
4747

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

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

150-
PackageDef(Ident(nme.REPL_PACKAGE), List(module))
150+
PackageDef(Ident(nme.EMPTY_PACKAGE), List(module))
151151
}
152152

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

255255
ParseResult(sourceFile)(state) match {

compiler/src/dotty/tools/repl/ScriptEngine.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class ScriptEngine extends AbstractScriptEngine {
3737
val vid = state.valIndex
3838
state = driver.run(script)(state)
3939
val oid = state.objectIndex
40-
Class.forName(s"${nme.REPL_PACKAGE}.${str.REPL_SESSION_LINE}$oid", true, rendering.classLoader()(using state.context))
40+
Class.forName(s"${Rendering.REPL_WRAPPER_NAME_PREFIX}$oid", true, rendering.classLoader()(using state.context))
4141
.getDeclaredMethods.find(_.getName == s"${str.REPL_RES_PREFIX}$vid")
4242
.map(_.invoke(null))
4343
.getOrElse(null)

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTest.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import org.junit.Assert._
88

99
trait ErrorMessagesTest extends DottyTest {
1010

11-
private def newContext = {
11+
protected def newContext = {
1212
val rep = new StoreReporter(null)
1313
with UniqueMessagePositions with HideNonSensicalMessages
1414
initialCtx.setReporter(rep).setSetting(ctx.settings.color, "never")
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package dotty.tools
2+
package repl
3+
4+
import java.io.File
5+
import java.nio.file.Files
6+
7+
import org.junit.{ After, AfterClass, BeforeClass, Ignore, Test }
8+
import org.junit.Assert._
9+
import io.{ Directory, PlainDirectory }
10+
import dotc.core.Contexts._
11+
import dotc.reporting.{ ErrorMessagesTest, StoreReporter }
12+
import vulpix.TestConfiguration
13+
14+
object ShadowingBatchTests:
15+
val dir = Directory(Files.createTempDirectory("batch-shadow"))
16+
17+
@BeforeClass def suiteStarting: Unit = dir.createDirectory()
18+
@AfterClass def suiteFinished: Unit = dir.deleteRecursively()
19+
20+
class ShadowingBatchTests extends ErrorMessagesTest:
21+
import ShadowingBatchTests._
22+
23+
@After def testFinished: Unit = dir.list.foreach(_.deleteRecursively())
24+
25+
val compiler = new dotc.Compiler()
26+
27+
override def initializeCtx(ictx: FreshContext) = inContext(ictx) {
28+
super.initializeCtx(ictx)
29+
val settings = ictx.settings; import settings._
30+
ictx.setSetting(outputDir, new PlainDirectory(dir))
31+
ictx.setSetting(classpath, classpath.value + File.pathSeparator + dir.jpath.toAbsolutePath)
32+
}
33+
34+
@Test def file =
35+
checkMessages("class C(val c: Int)").expectNoErrors
36+
checkMessages("object rsline1 {\n def line1 = new C().c\n}").expect { (_, msgs) =>
37+
assertMessageCount(1, msgs)
38+
assertEquals("missing argument for parameter c of constructor C in class C: (c: Int): C", msgs.head.message)
39+
}
40+
checkMessages("object rsline2 {\n def line2 = new C(13).c\n}").expectNoErrors
41+
checkMessages("object rsline3 {\n class C { val c = 42 }\n}").expectNoErrors
42+
checkMessages("import rsline3._\nobject rsline4 {\n def line4 = new C().c\n}").expectNoErrors
43+
44+
@Test def directory =
45+
checkMessages("package foo\nclass C").expectNoErrors
46+
checkMessages("object rsline1 {\n def line1 = foo\n}").expect { (_, msgs) =>
47+
assertMessageCount(1, msgs)
48+
assertEquals("package foo is not a value", msgs.head.message)
49+
}
50+
checkMessages("object rsline2 {\n val foo = 2\n}").expectNoErrors
51+
checkMessages("import rsline2._\nobject rsline3 {\n def line3 = foo\n}").expectNoErrors
52+
53+
@Test def directoryJava =
54+
checkMessages("object rsline1 {\n def line1 = java\n}").expect { (_, msgs) =>
55+
assertMessageCount(1, msgs)
56+
assertEquals("package java is not a value", msgs.head.message)
57+
}
58+
checkMessages("object rsline2 {\n val java = 2\n}").expectNoErrors
59+
checkMessages("import rsline2._\nobject rsline3 {\n def line3 = java\n}").expectNoErrors
60+
61+
def checkMessages(source: String): Report =
62+
ctx = newContext
63+
val run = compiler.newRun(using ctx.fresh)
64+
run.compileFromStrings(List(source))
65+
val runCtx = run.runContext
66+
if runCtx.reporter.hasErrors then
67+
val rep = runCtx.reporter.asInstanceOf[StoreReporter]
68+
val msgs = rep.removeBufferedMessages(using runCtx).map(_.msg).reverse
69+
new Report(msgs, runCtx)
70+
else new EmptyReport
71+
end ShadowingBatchTests

compiler/test/dotty/tools/repl/ShadowingTests.scala

-4
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ class ShadowingTests extends ReplTest(options = ShadowingTests.options):
9595
|""".stripMargin
9696
)
9797

98-
@Ignore("not yet fixed")
9998
@Test def `shadow subdirectories on classpath` =
10099
// NB: Tests of shadowing of subdirectories on the classpath are only valid
101100
// when the subdirectories exist prior to initialization of the REPL driver.
@@ -128,9 +127,6 @@ class ShadowingTests extends ReplTest(options = ShadowingTests.options):
128127
ShadowingTests.createSubDir("util")
129128
testScript(name = "<shadow-subdir-util>",
130129
"""|scala> import util.Try
131-
|1 | import util.Try
132-
| | ^^^
133-
| | value Try is not a member of util
134130
|
135131
|scala> object util { class Try { override def toString = "you've gotta try!" } }
136132
|// defined object util

0 commit comments

Comments
 (0)