Skip to content

Commit e442879

Browse files
Cleanups and source compatibility improvements for repl.AbstractFileClassLoader (#24514)
REPL and AbstractFileClassLoader is used by multiple downstream dependencies (eg. scalameta/mdoc), this PR is oriented on improving source compatibility of this class : - Share common parts of implementation between `io.AbstractFileClassLoader` and `repl.AbstractFileClassLoader` - Added auxilary conststructor with default `interruptInstrumentation` argument - Made `InterruptInstrumentation` an enum - Fixed warnings present in the repl projects
1 parent ae54faa commit e442879

File tree

6 files changed

+52
-43
lines changed

6 files changed

+52
-43
lines changed

repl/src/dotty/tools/repl/AbstractFileClassLoader.scala

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,30 +15,37 @@ package repl
1515

1616
import scala.language.unsafeNulls
1717

18+
import dotty.tools.dotc.config.ScalaSettings
19+
1820
import io.AbstractFile
1921

2022
import java.net.{URL, URLConnection, URLStreamHandler}
2123
import java.util.Collections
2224

23-
class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader, interruptInstrumentation: String) extends ClassLoader(parent):
24-
private def findAbstractFile(name: String) = root.lookupPath(name.split('/').toIndexedSeq, directory = false)
25-
26-
// on JDK 20 the URL constructor we're using is deprecated,
27-
// but the recommended replacement, URL.of, doesn't exist on JDK 8
28-
@annotation.nowarn("cat=deprecation")
29-
override protected def findResource(name: String): URL | Null =
30-
findAbstractFile(name) match
31-
case null => null
32-
case file => new URL(null, s"memory:${file.path}", new URLStreamHandler {
33-
override def openConnection(url: URL): URLConnection = new URLConnection(url) {
34-
override def connect() = ()
35-
override def getInputStream = file.input
36-
}
37-
})
38-
override protected def findResources(name: String): java.util.Enumeration[URL] =
39-
findResource(name) match
40-
case null => Collections.enumeration(Collections.emptyList[URL]) //Collections.emptyEnumeration[URL]
41-
case url => Collections.enumeration(Collections.singleton(url))
25+
import AbstractFileClassLoader.InterruptInstrumentation
26+
27+
28+
object AbstractFileClassLoader:
29+
enum InterruptInstrumentation(val stringValue: String):
30+
case Disabled extends InterruptInstrumentation("false")
31+
case Enabled extends InterruptInstrumentation("true")
32+
case Local extends InterruptInstrumentation("local")
33+
34+
def is(value: InterruptInstrumentation): Boolean = this == value
35+
def isOneOf(others: InterruptInstrumentation*): Boolean = others.contains(this)
36+
37+
object InterruptInstrumentation:
38+
def fromString(string: String): InterruptInstrumentation = string match {
39+
case "false" => Disabled
40+
case "true" => Enabled
41+
case "local" => Local
42+
case _ => throw new IllegalArgumentException(s"Invalid interrupt instrumentation value: $string")
43+
}
44+
45+
class AbstractFileClassLoader(root: AbstractFile, parent: ClassLoader, interruptInstrumentation: InterruptInstrumentation)
46+
extends io.AbstractFileClassLoader(root, parent):
47+
48+
def this(root: AbstractFile, parent: ClassLoader) = this(root, parent, InterruptInstrumentation.fromString(ScalaSettings.XreplInterruptInstrumentation.default))
4249

4350
override def findClass(name: String): Class[?] = {
4451
var file: AbstractFile | Null = root
@@ -52,23 +59,23 @@ class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader, inter
5259

5360
val bytes = file.toByteArray
5461

55-
if interruptInstrumentation != "false" then defineClassInstrumented(name, bytes)
62+
if !interruptInstrumentation.is(InterruptInstrumentation.Enabled) then defineClassInstrumented(name, bytes)
5663
else defineClass(name, bytes, 0, bytes.length)
5764
}
5865

59-
def defineClassInstrumented(name: String, originalBytes: Array[Byte]) = {
66+
private def defineClassInstrumented(name: String, originalBytes: Array[Byte]) = {
6067
val instrumentedBytes = ReplBytecodeInstrumentation.instrument(originalBytes)
6168
defineClass(name, instrumentedBytes, 0, instrumentedBytes.length)
6269
}
6370

6471
override def loadClass(name: String): Class[?] =
65-
if interruptInstrumentation == "false" || interruptInstrumentation == "local"
66-
then return super.loadClass(name)
72+
if interruptInstrumentation.isOneOf(InterruptInstrumentation.Disabled, InterruptInstrumentation.Local) then
73+
return super.loadClass(name)
6774

6875
val loaded = findLoadedClass(name) // Check if already loaded
6976
if loaded != null then return loaded
7077

71-
name match {
78+
name match {
7279
// Don't instrument JDK classes or StopRepl. These are often restricted to load from a single classloader
7380
// due to the JDK module system, and so instrumenting them and loading the modified copy of the class
7481
// results in runtime exceptions

repl/src/dotty/tools/repl/DependencyResolver.scala

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import java.net.{URL, URLClassLoader}
77
import scala.jdk.CollectionConverters.*
88
import scala.util.control.NonFatal
99

10+
import dotty.tools.repl.AbstractFileClassLoader
11+
1012
import coursierapi.{Repository, Dependency, MavenRepository}
1113
import com.virtuslab.using_directives.UsingDirectivesProcessor
1214
import com.virtuslab.using_directives.custom.model.{Path, StringValue, Value}
@@ -90,7 +92,7 @@ object DependencyResolver:
9092
import dotty.tools.dotc.classpath.ClassPathFactory
9193
import dotty.tools.dotc.core.SymbolLoaders
9294
import dotty.tools.dotc.core.Symbols.defn
93-
import dotty.tools.io.*
95+
import dotty.tools.io.{AbstractFile, ClassPath}
9496
import dotty.tools.repl.ScalaClassLoader.fromURLsParallelCapable
9597

9698
// Create a classloader with all the resolved JAR files
@@ -106,10 +108,10 @@ object DependencyResolver:
106108
SymbolLoaders.mergeNewEntries(defn.RootClass, ClassPath.RootPackage, jarClassPath, ctx.platform.classPath)
107109

108110
// Create new classloader with previous output dir and resolved dependencies
109-
new dotty.tools.repl.AbstractFileClassLoader(
111+
new AbstractFileClassLoader(
110112
prevOutputDir,
111113
depsClassLoader,
112-
ctx.settings.XreplInterruptInstrumentation.value
114+
AbstractFileClassLoader.InterruptInstrumentation.fromString(ctx.settings.XreplInterruptInstrumentation.value)
113115
)
114116

115117
end DependencyResolver

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None):
8686
myClassLoader = new AbstractFileClassLoader(
8787
ctx.settings.outputDir.value,
8888
parent,
89-
ctx.settings.XreplInterruptInstrumentation.value
89+
AbstractFileClassLoader.InterruptInstrumentation.fromString(ctx.settings.XreplInterruptInstrumentation.value)
9090
)
9191
myClassLoader
9292
}

repl/src/dotty/tools/repl/ReplBytecodeInstrumentation.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import scala.language.unsafeNulls
66
import scala.tools.asm.*
77
import scala.tools.asm.Opcodes.*
88
import scala.tools.asm.tree.*
9-
import scala.collection.JavaConverters.*
9+
import scala.jdk.CollectionConverters.*
1010
import java.util.concurrent.atomic.AtomicBoolean
11-
11+
1212
object ReplBytecodeInstrumentation:
1313
/** Instrument bytecode to add checks to throw an exception if the REPL command is cancelled
1414
*/

repl/src/dotty/tools/repl/ReplDriver.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ class ReplDriver(settings: Array[String],
607607
rendering.myClassLoader = new AbstractFileClassLoader(
608608
prevOutputDir,
609609
jarClassLoader,
610-
ctx.settings.XreplInterruptInstrumentation.value
610+
AbstractFileClassLoader.InterruptInstrumentation.fromString(ctx.settings.XreplInterruptInstrumentation.value)
611611
)
612612

613613
out.println(s"Added '$path' to classpath.")

repl/test/dotty/tools/repl/AbstractFileClassLoaderTest.scala

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ class AbstractFileClassLoaderTest:
5151
@Test def afclGetsParent(): Unit =
5252
val p = new URLClassLoader(Array.empty[URL])
5353
val d = new VirtualDirectory("vd", None)
54-
val x = new AbstractFileClassLoader(d, p, "false")
54+
val x = new AbstractFileClassLoader(d, p)
5555
assertSame(p, x.getParent)
5656

5757
@Test def afclGetsResource(): Unit =
5858
val (fuzz, booz) = fuzzBuzzBooz
5959
booz.writeContent("hello, world")
60-
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
60+
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader)
6161
val res = sut.getResource("buzz/booz.class")
6262
assertNotNull("Find buzz/booz.class", res)
6363
assertEquals("hello, world", slurp(res))
@@ -67,8 +67,8 @@ class AbstractFileClassLoaderTest:
6767
val (fuzz_, booz_) = fuzzBuzzBooz
6868
booz.writeContent("hello, world")
6969
booz_.writeContent("hello, world_")
70-
val p = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
71-
val sut = new AbstractFileClassLoader(fuzz_, p, "false")
70+
val p = new AbstractFileClassLoader(fuzz, NoClassLoader)
71+
val sut = new AbstractFileClassLoader(fuzz_, p)
7272
val res = sut.getResource("buzz/booz.class")
7373
assertNotNull("Find buzz/booz.class", res)
7474
assertEquals("hello, world", slurp(res))
@@ -79,7 +79,7 @@ class AbstractFileClassLoaderTest:
7979
val bass = fuzz.fileNamed("bass")
8080
booz.writeContent("hello, world")
8181
bass.writeContent("lo tone")
82-
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
82+
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader)
8383
val res = sut.getResource("booz.class")
8484
assertNotNull(res)
8585
assertEquals("hello, world", slurp(res))
@@ -89,7 +89,7 @@ class AbstractFileClassLoaderTest:
8989
@Test def afclGetsResources(): Unit =
9090
val (fuzz, booz) = fuzzBuzzBooz
9191
booz.writeContent("hello, world")
92-
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
92+
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader)
9393
val e = sut.getResources("buzz/booz.class")
9494
assertTrue("At least one buzz/booz.class", e.hasMoreElements)
9595
assertEquals("hello, world", slurp(e.nextElement))
@@ -100,8 +100,8 @@ class AbstractFileClassLoaderTest:
100100
val (fuzz_, booz_) = fuzzBuzzBooz
101101
booz.writeContent("hello, world")
102102
booz_.writeContent("hello, world_")
103-
val p = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
104-
val x = new AbstractFileClassLoader(fuzz_, p, "false")
103+
val p = new AbstractFileClassLoader(fuzz, NoClassLoader)
104+
val x = new AbstractFileClassLoader(fuzz_, p)
105105
val e = x.getResources("buzz/booz.class")
106106
assertTrue(e.hasMoreElements)
107107
assertEquals("hello, world", slurp(e.nextElement))
@@ -112,15 +112,15 @@ class AbstractFileClassLoaderTest:
112112
@Test def afclGetsResourceAsStream(): Unit =
113113
val (fuzz, booz) = fuzzBuzzBooz
114114
booz.writeContent("hello, world")
115-
val x = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
115+
val x = new AbstractFileClassLoader(fuzz, NoClassLoader)
116116
val r = x.getResourceAsStream("buzz/booz.class")
117117
assertNotNull(r)
118118
assertEquals("hello, world", closing(r)(is => Source.fromInputStream(is).mkString))
119119

120120
@Test def afclGetsClassBytes(): Unit =
121121
val (fuzz, booz) = fuzzBuzzBooz
122122
booz.writeContent("hello, world")
123-
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
123+
val sut = new AbstractFileClassLoader(fuzz, NoClassLoader)
124124
val b = sut.classBytes("buzz/booz.class")
125125
assertEquals("hello, world", new String(b, UTF8.charSet))
126126

@@ -130,8 +130,8 @@ class AbstractFileClassLoaderTest:
130130
booz.writeContent("hello, world")
131131
booz_.writeContent("hello, world_")
132132

133-
val p = new AbstractFileClassLoader(fuzz, NoClassLoader, "false")
134-
val sut = new AbstractFileClassLoader(fuzz_, p, "false")
133+
val p = new AbstractFileClassLoader(fuzz, NoClassLoader)
134+
val sut = new AbstractFileClassLoader(fuzz_, p)
135135
val b = sut.classBytes("buzz/booz.class")
136136
assertEquals("hello, world", new String(b, UTF8.charSet))
137137
end AbstractFileClassLoaderTest

0 commit comments

Comments
 (0)