Skip to content

Commit 034a8e4

Browse files
committed
Add a diagnostic for when a script's wrapper name shadows a dependency package
1 parent 1ca0d55 commit 034a8e4

7 files changed

Lines changed: 161 additions & 21 deletions

File tree

modules/build/src/main/scala/scala/build/Build.scala

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import scala.build.compiler.{ScalaCompiler, ScalaCompilerMaker}
1616
import scala.build.errors.*
1717
import scala.build.input.*
1818
import scala.build.internal.resource.ResourceMapper
19-
import scala.build.internal.{Constants, MainClass, Name, Util}
19+
import scala.build.internal.util.WarningMessages
20+
import scala.build.internal.{Constants, MainClass, Name, ScriptUtils, Util}
2021
import scala.build.internals.ConsoleUtils.ScalaCliConsole.warnPrefix
2122
import scala.build.options.*
2223
import scala.build.options.validation.ValidationException
@@ -1129,6 +1130,18 @@ object Build {
11291130

11301131
val artifacts = value(options0.artifacts(logger, scope, maybeRecoverOnError))
11311132

1133+
for shadowed <- ScriptUtils.findShadowedDependencyPackages(
1134+
sources = sources,
1135+
classPath = artifacts.compileClassPath,
1136+
logger = logger
1137+
)
1138+
do
1139+
logger.diagnostic(
1140+
message = WarningMessages.scriptNameShadowsDependencyPackage(shadowed),
1141+
severity = Severity.Warning,
1142+
positions = Seq(Position.File(Right(shadowed.filePath), (0, 0), (0, 0)))
1143+
)
1144+
11321145
value(validate(logger, options0))
11331146

11341147
val project = value {

modules/build/src/main/scala/scala/build/Sources.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import coursier.util.Task
66
import java.nio.charset.StandardCharsets
77

88
import scala.build.input.Inputs
9-
import scala.build.internal.{CodeWrapper, WrapperParams}
9+
import scala.build.internal.ScriptUtils.ScriptDescriptor
10+
import scala.build.internal.{AmmUtil, CodeWrapper, WrapperParams}
1011
import scala.build.options.{BuildOptions, Scope}
1112
import scala.build.preprocessing.*
1213

@@ -76,6 +77,14 @@ final case class Sources(
7677
lazy val hasScala =
7778
(paths.iterator.map(_._1.last) ++ inMemory.iterator.map(_.generatedRelPath.last))
7879
.exists(_.endsWith(".scala"))
80+
81+
def scriptTopLevelNames: Seq[ScriptDescriptor] =
82+
inMemory.collect {
83+
case Sources.InMemory(Right((subPath, filePath)), _, _, Some(_)) =>
84+
val (pkg, wrapper) = AmmUtil.pathToPackageWrapper(subPath)
85+
val topLevelName = pkg.headOption.map(_.raw).getOrElse(wrapper.raw)
86+
ScriptDescriptor(topLevelName, subPath, filePath)
87+
}
7988
}
8089

8190
object Sources {
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package scala.build.internal
2+
3+
import java.io.ByteArrayInputStream
4+
import java.nio.file.NoSuchFileException
5+
6+
import scala.build.internal.zip.WrappedZipInputStream
7+
import scala.build.{Logger, retry}
8+
9+
object JarUtils {
10+
11+
/** Walk `.class` entries in a JAR */
12+
def walkClassEntries[A](jar: os.Path, logger: Logger)(
13+
extract: (String, () => Array[Byte]) => Iterator[A]
14+
): Iterator[A] =
15+
try
16+
retry()(logger) {
17+
val content = os.read.bytes(jar)
18+
val zip = WrappedZipInputStream.create(new ByteArrayInputStream(content))
19+
zip.entries().flatMap { ent =>
20+
if !ent.isDirectory && ent.getName.endsWith(".class") then
21+
extract(ent.getName, () => zip.readAllBytes())
22+
else Iterator.empty
23+
}
24+
}
25+
catch {
26+
case e: NoSuchFileException =>
27+
logger.debugStackTrace(e)
28+
logger.debug(s"JAR file $jar not found: $e, skipping.")
29+
Iterator.empty
30+
}
31+
32+
}

modules/build/src/main/scala/scala/build/internal/MainClass.scala

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import java.io.{ByteArrayInputStream, InputStream}
77
import java.nio.file.NoSuchFileException
88
import java.util.jar.{Attributes, JarFile}
99

10-
import scala.build.internal.zip.WrappedZipInputStream
1110
import scala.build.{Logger, retry}
1211

1312
object MainClass {
@@ -73,24 +72,8 @@ object MainClass {
7372
finally is.close()
7473

7574
private def findInJar(path: os.Path, logger: Logger): Iterator[String] =
76-
try retry()(logger) {
77-
val content = os.read.bytes(path)
78-
val jarInputStream = WrappedZipInputStream.create(new ByteArrayInputStream(content))
79-
jarInputStream.entries().flatMap(ent =>
80-
if !ent.isDirectory && ent.getName.endsWith(".class") then {
81-
val content = jarInputStream.readAllBytes()
82-
val inputStream = new ByteArrayInputStream(content)
83-
findInClass(inputStream, logger)
84-
}
85-
else Iterator.empty
86-
)
87-
}
88-
catch {
89-
case e: NoSuchFileException =>
90-
logger.debugStackTrace(e)
91-
logger.log(s"JAR file $path not found: $e, trying to recover...")
92-
logger.log("Are you trying to run too many builds at once? Trying to recover...")
93-
Iterator.empty
75+
JarUtils.walkClassEntries(path, logger) { (_, bytes) =>
76+
findInClass(new ByteArrayInputStream(bytes()), logger)
9477
}
9578

9679
def findInDependency(jar: os.Path): Option[String] =
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package scala.build.internal
2+
3+
import scala.build.{Logger, Sources}
4+
5+
object ScriptUtils {
6+
private val ignoredPackageRoots = Set("scala", "java", "javax", "META-INF")
7+
8+
final case class ScriptDescriptor(
9+
name: String,
10+
subPath: os.SubPath,
11+
filePath: os.Path,
12+
shadowedDependencyJars: Seq[String] = Nil
13+
)
14+
15+
def findShadowedDependencyPackages(
16+
sources: Sources,
17+
classPath: Seq[os.Path],
18+
logger: Logger
19+
): Seq[ScriptDescriptor] = {
20+
val scripts = sources.scriptTopLevelNames.filterNot(s => ignoredPackageRoots(s.name))
21+
if scripts.isEmpty then Nil
22+
else
23+
val packageRoots =
24+
classPath
25+
.filter(_.last.endsWith(".jar"))
26+
.flatMap(path =>
27+
JarUtils.walkClassEntries(path, logger) { (name, _) =>
28+
val slashIdx = name.indexOf('/')
29+
if slashIdx > 0 then Iterator.single(name.take(slashIdx))
30+
else Iterator.empty
31+
}.toSet.map(_ -> path)
32+
)
33+
.groupMap((root, _) => root)((_, path) => path)
34+
.view.mapValues(_.toSet).toMap
35+
scripts.flatMap { script =>
36+
packageRoots.get(script.name).map { jars =>
37+
script.copy(shadowedDependencyJars = jars.toSeq.map(_.last).sorted)
38+
}
39+
}
40+
}
41+
}

modules/build/src/main/scala/scala/build/internal/util/WarningMessages.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package scala.build.internal.util
22

33
import scala.build.input.ScalaCliInvokeData
44
import scala.build.internal.Constants
5+
import scala.build.internal.ScriptUtils.ScriptDescriptor
56
import scala.build.internals.FeatureType
67
import scala.build.preprocessing.directives.{DirectiveHandler, ScopedDirective}
78
import scala.cli.commands.SpecificationLevel
@@ -130,6 +131,17 @@ object WarningMessages {
130131
val mainScriptNameClashesWithAppWrapper =
131132
"Script file named 'main.sc' detected, keep in mind that accessing it from other scripts is impossible due to a clash of `main` symbols"
132133

134+
def scriptNameShadowsDependencyPackage(shadowed: ScriptDescriptor): String =
135+
val name = shadowed.name
136+
val depsClause = shadowed.shadowedDependencyJars match {
137+
case Nil => "from an unknown dependency"
138+
case Seq(j) => s"from dependency '$j'"
139+
case js => s"from dependencies: ${js.map(j => s"'$j'").mkString(", ")}"
140+
}
141+
s"""Script '${shadowed.subPath}' generates a top-level symbol '$name' that shadows the '$name' package $depsClause.
142+
|Imports of '$name.*' from the dependency will not resolve.
143+
|Consider renaming the script (e.g. to '${name}1.sc') or moving it into a subdirectory.""".stripMargin
144+
133145
private val deprecationNote =
134146
"Deprecated features may be removed in a future version."
135147

modules/integration/src/test/scala/scala/cli/integration/RunScriptTestDefinitions.scala

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,56 @@ trait RunScriptTestDefinitions { this: RunTestDefinitions =>
270270
}
271271
}
272272

273+
test("warn when script name shadows a dependency top-level package") {
274+
val inputs = TestInputs(
275+
os.rel / "os.sc" ->
276+
s"""//> using dep com.lihaoyi::os-lib:0.11.8
277+
|println("hi")
278+
|""".stripMargin
279+
)
280+
inputs.fromRoot { root =>
281+
val res = os.proc(TestUtil.cli, extraOptions, "os.sc")
282+
.call(cwd = root, check = false, mergeErrIntoOut = true)
283+
val output = res.out.trim()
284+
expect(output.contains("shadows the 'os' package"))
285+
expect(res.exitCode == 0)
286+
}
287+
}
288+
289+
test("warn with multiple JARs when script name shadows a shared package root") {
290+
val inputs = TestInputs(
291+
os.rel / "cats.sc" ->
292+
s"""//> using dep org.typelevel::cats-core:2.12.0
293+
|println("hi")
294+
|""".stripMargin
295+
)
296+
inputs.fromRoot { root =>
297+
val res = os.proc(TestUtil.cli, extraOptions, "cats.sc")
298+
.call(cwd = root, check = false, mergeErrIntoOut = true)
299+
val output = res.out.trim()
300+
expect(output.contains("shadows the 'cats' package from dependencies:"))
301+
expect(output.contains("cats-core"))
302+
expect(output.contains("cats-kernel"))
303+
expect(res.exitCode == 0)
304+
}
305+
}
306+
307+
test("do not warn when script name does not shadow a dependency top-level package") {
308+
val inputs = TestInputs(
309+
os.rel / "safe.sc" ->
310+
s"""//> using dep com.lihaoyi::os-lib:0.11.8
311+
|println("hi")
312+
|""".stripMargin
313+
)
314+
inputs.fromRoot { root =>
315+
val res = os.proc(TestUtil.cli, extraOptions, "safe.sc")
316+
.call(cwd = root, check = false, mergeErrIntoOut = true)
317+
val output = res.out.trim()
318+
expect(!output.contains("shadows the 'os' package"))
319+
expect(res.exitCode == 0)
320+
}
321+
}
322+
273323
test("Directory") {
274324
val message = "Hello"
275325
val inputs = TestInputs(

0 commit comments

Comments
 (0)