Skip to content

Commit a3a3fb8

Browse files
authored
Fix "REPL dependency on scala-parallel-collections doesn't work (in 3.8.2-RC1)" (#25064)
Fixes #25058, vibe coded Prompt: > Help me fix #25058. > > - Reproduce the problem locally manually or in the unit test harness > - Analyze the code related to the problem and the `git blame` of that code to understand the history > - Investigate the root cause using `println`s and other debugging statements > - Propose a theory for why the problem is happening > - Implement a fix > - Verify your fix using the unit tests > - Review up your change by running `git diff 3.8.2-RC1` and analyzing the diff to find unnecessary changes that can be cleaned up or areas where your implementation can be simplified or improved > > Repeat as many times as necessary until the issue is fixed with an acceptable outcome _Almost_ one-shot by claude, except it initially jumped to coding a solution blind without testing, against my explicit instructions, which of course was wrong. After reminding it to reproduce the problem and write a test and make sure it's red/green, it came up with this PR
1 parent c5418c8 commit a3a3fb8

File tree

2 files changed

+152
-3
lines changed

2 files changed

+152
-3
lines changed

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,20 +335,34 @@ object SymbolLoaders {
335335
packageClass: ClassSymbol, fullPackageName: String,
336336
jarClasspath: ClassPath, fullClasspath: ClassPath,
337337
)(using Context): Unit =
338-
if jarClasspath.classes(fullPackageName).nonEmpty then
338+
val hasClasses = jarClasspath.classes(fullPackageName).nonEmpty
339+
val hasPackages = jarClasspath.packages(fullPackageName).nonEmpty
340+
341+
if hasClasses then
339342
// if the package contains classes in jarClasspath, the package is invalidated (or removed if there are no more classes in it)
340343
val packageVal = packageClass.sourceModule.asInstanceOf[TermSymbol]
341344
if packageClass.isRoot then
342345
val loader = new PackageLoader(packageVal, fullClasspath)
343346
loader.enterClasses(defn.EmptyPackageClass, fullPackageName, flat = false)
344347
loader.enterClasses(defn.EmptyPackageClass, fullPackageName, flat = true)
345348
else if packageClass.ownersIterator.contains(defn.ScalaPackageClass) then
346-
() // skip
349+
// For scala packages, enter new classes into the existing scope without
350+
// replacing the package info (which would cause cyclic references).
351+
// Use jarClasspath (not fullClasspath) to only enter new classes from the JAR.
352+
// This allows libraries like scala-parallel-collections to work with :dep/:jar
353+
val loader = new PackageLoader(packageVal, jarClasspath)
354+
loader.enterClasses(packageClass, fullPackageName, flat = false)
355+
loader.enterClasses(packageClass, fullPackageName, flat = true)
347356
else if fullClasspath.hasPackage(fullPackageName) then
348357
packageClass.info = new PackageLoader(packageVal, fullClasspath)
349358
else
350359
packageClass.owner.info.decls.openForMutations.unlink(packageVal)
351-
else
360+
361+
// Always process sub-packages, even when hasClasses is true.
362+
// This is needed when a package has BOTH classes AND sub-packages,
363+
// e.g. scala-parallel-collections adds both classes to scala.collection
364+
// and the new scala.collection.parallel sub-package.
365+
if hasPackages then
352366
for p <- jarClasspath.packages(fullPackageName) do
353367
val subPackageName = PackageNameUtils.separatePkgAndClassNames(p.name)._2.toTermName
354368
val subPackage = packageClass.info.decl(subPackageName).orElse:
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package dotty.tools
2+
package repl
3+
4+
import scala.language.unsafeNulls
5+
6+
import java.io.{File, FileOutputStream}
7+
import java.nio.file.{Files, Path}
8+
import java.util.Comparator
9+
import java.util.jar.{JarEntry, JarOutputStream, Manifest}
10+
import javax.tools.ToolProvider
11+
12+
import org.junit.{Test, Assume}
13+
import org.junit.Assert.{assertEquals, assertTrue, assertFalse}
14+
15+
/** Tests for loading JARs that contain classes in scala.* packages.
16+
* This tests the fix for https://github.com/scala/scala3/issues/25058
17+
*/
18+
class ScalaPackageJarTests extends ReplTest:
19+
import ScalaPackageJarTests.*
20+
21+
@Test def `i25058 load JAR with scala collection parallel subpackage` =
22+
// Skip if javac is not available
23+
Assume.assumeTrue("javac not available", ToolProvider.getSystemJavaCompiler != null)
24+
25+
// Create a JAR that simulates scala-parallel-collections:
26+
// - Classes in scala.collection (e.g., ParIterable)
27+
// - Classes in scala.collection.parallel subpackage (e.g., CollectionConverters)
28+
// The bug was that when a package has BOTH classes AND subpackages,
29+
// only the classes were processed and subpackages were skipped.
30+
val jarPath = createScalaCollectionParallelJar()
31+
try
32+
initially {
33+
// First, access scala.collection to ensure it's loaded in the symbol table
34+
val state = run("scala.collection.mutable.ArrayBuffer.empty[Int]")
35+
storedOutput() // discard output
36+
state
37+
} andThen {
38+
// Load the JAR with classes in scala.collection AND scala.collection.parallel
39+
val state = run(s":jar $jarPath")
40+
val output = storedOutput()
41+
assertTrue(s"Expected success message, got: $output", output.contains("Added") && output.contains("to classpath"))
42+
state
43+
} andThen {
44+
// Import from scala.collection.parallel - this is the key test for #25058
45+
val state = run("import scala.collection.parallel.TestParallel")
46+
val importOutput = storedOutput()
47+
// Should not have an error
48+
assertFalse(s"Import should succeed, got: $importOutput",
49+
importOutput.contains("Not Found Error") || importOutput.contains("not a member"))
50+
state
51+
} andThen {
52+
// Use the imported class
53+
run("TestParallel.getValue()")
54+
val output = storedOutput()
55+
assertTrue(s"Expected value 42, got: $output", output.contains("42"))
56+
}
57+
finally
58+
Files.deleteIfExists(Path.of(jarPath))
59+
60+
object ScalaPackageJarTests:
61+
62+
/** Creates a JAR file simulating scala-parallel-collections structure:
63+
* - A class in scala.collection (TestParIterable)
64+
* - A class in scala.collection.parallel (TestParallel)
65+
*
66+
* This is critical for testing #25058: the bug only manifests when
67+
* a JAR adds BOTH classes to an existing package (scala.collection)
68+
* AND a new subpackage (scala.collection.parallel).
69+
*/
70+
def createScalaCollectionParallelJar(): String =
71+
val tempDir = Files.createTempDirectory("scala-pkg-test")
72+
73+
// Create package directory structures
74+
val collectionDir = tempDir.resolve("scala/collection")
75+
val parallelDir = tempDir.resolve("scala/collection/parallel")
76+
Files.createDirectories(parallelDir)
77+
78+
// Write Java source file in scala.collection (simulates ParIterable etc.)
79+
val collectionSource = collectionDir.resolve("TestParIterable.java")
80+
Files.writeString(collectionSource,
81+
"""|package scala.collection;
82+
|public class TestParIterable {
83+
| public static int getCount() { return 100; }
84+
|}
85+
|""".stripMargin)
86+
87+
// Write Java source file in scala.collection.parallel (simulates CollectionConverters etc.)
88+
val parallelSource = parallelDir.resolve("TestParallel.java")
89+
Files.writeString(parallelSource,
90+
"""|package scala.collection.parallel;
91+
|public class TestParallel {
92+
| public static int getValue() { return 42; }
93+
|}
94+
|""".stripMargin)
95+
96+
// Compile with javac
97+
val compiler = ToolProvider.getSystemJavaCompiler
98+
val fileManager = compiler.getStandardFileManager(null, null, null)
99+
val compilationUnits = fileManager.getJavaFileObjects(collectionSource.toFile, parallelSource.toFile)
100+
val task = compiler.getTask(null, fileManager, null,
101+
java.util.Arrays.asList("-d", tempDir.toString), null, compilationUnits)
102+
val success = task.call()
103+
fileManager.close()
104+
105+
if !success then
106+
throw new RuntimeException("Failed to compile test classes")
107+
108+
// Create JAR file
109+
val jarFile = tempDir.resolve("scala-collection-parallel.jar").toFile
110+
val manifest = new Manifest()
111+
manifest.getMainAttributes.putValue("Manifest-Version", "1.0")
112+
113+
val jos = new JarOutputStream(new FileOutputStream(jarFile), manifest)
114+
try
115+
// Add class in scala.collection
116+
val collectionClass = collectionDir.resolve("TestParIterable.class")
117+
jos.putNextEntry(new JarEntry("scala/collection/TestParIterable.class"))
118+
jos.write(Files.readAllBytes(collectionClass))
119+
jos.closeEntry()
120+
121+
// Add class in scala.collection.parallel
122+
val parallelClass = parallelDir.resolve("TestParallel.class")
123+
jos.putNextEntry(new JarEntry("scala/collection/parallel/TestParallel.class"))
124+
jos.write(Files.readAllBytes(parallelClass))
125+
jos.closeEntry()
126+
finally
127+
jos.close()
128+
129+
// Clean up source and class files (keep only the JAR)
130+
Files.walk(tempDir)
131+
.sorted(Comparator.reverseOrder[Path]())
132+
.filter(p => !p.equals(jarFile.toPath) && !p.equals(tempDir))
133+
.forEach(Files.delete)
134+
135+
jarFile.getAbsolutePath

0 commit comments

Comments
 (0)