Skip to content
Draft
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
15 changes: 14 additions & 1 deletion modules/build/src/main/scala/scala/build/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import scala.build.compiler.{ScalaCompiler, ScalaCompilerMaker}
import scala.build.errors.*
import scala.build.input.*
import scala.build.internal.resource.ResourceMapper
import scala.build.internal.{Constants, MainClass, Name, Util}
import scala.build.internal.util.WarningMessages
import scala.build.internal.{Constants, MainClass, Name, ScriptUtils, Util}
import scala.build.internals.ConsoleUtils.ScalaCliConsole.warnPrefix
import scala.build.options.*
import scala.build.options.validation.ValidationException
Expand Down Expand Up @@ -1129,6 +1130,18 @@ object Build {

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

for shadowed <- ScriptUtils.findShadowedDependencyPackages(
sources = sources,
classPath = artifacts.compileClassPath,
logger = logger
)
do
logger.diagnostic(
message = WarningMessages.scriptNameShadowsDependencyPackage(shadowed),
severity = Severity.Warning,
positions = Seq(Position.File(Right(shadowed.filePath), (0, 0), (0, 0)))
)

value(validate(logger, options0))

val project = value {
Expand Down
11 changes: 10 additions & 1 deletion modules/build/src/main/scala/scala/build/Sources.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import coursier.util.Task
import java.nio.charset.StandardCharsets

import scala.build.input.Inputs
import scala.build.internal.{CodeWrapper, WrapperParams}
import scala.build.internal.ScriptUtils.ScriptDescriptor
import scala.build.internal.{AmmUtil, CodeWrapper, WrapperParams}
import scala.build.options.{BuildOptions, Scope}
import scala.build.preprocessing.*

Expand Down Expand Up @@ -76,6 +77,14 @@ final case class Sources(
lazy val hasScala =
(paths.iterator.map(_._1.last) ++ inMemory.iterator.map(_.generatedRelPath.last))
.exists(_.endsWith(".scala"))

def scriptTopLevelNames: Seq[ScriptDescriptor] =
inMemory.collect {
case Sources.InMemory(Right((subPath, filePath)), _, _, Some(_)) =>
val (pkg, wrapper) = AmmUtil.pathToPackageWrapper(subPath)
val topLevelName = pkg.headOption.map(_.raw).getOrElse(wrapper.raw)
ScriptDescriptor(topLevelName, subPath, filePath)
}
}

object Sources {
Expand Down
32 changes: 32 additions & 0 deletions modules/build/src/main/scala/scala/build/internal/JarUtils.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package scala.build.internal

import java.io.ByteArrayInputStream
import java.nio.file.NoSuchFileException

import scala.build.internal.zip.WrappedZipInputStream
import scala.build.{Logger, retry}

object JarUtils {

/** Walk `.class` entries in a JAR */
def walkClassEntries[A](jar: os.Path, logger: Logger)(
extract: (String, () => Array[Byte]) => Iterator[A]
): Iterator[A] =
try
retry()(logger) {
val content = os.read.bytes(jar)
val zip = WrappedZipInputStream.create(new ByteArrayInputStream(content))
zip.entries().flatMap { ent =>
if !ent.isDirectory && ent.getName.endsWith(".class") then
extract(ent.getName, () => zip.readAllBytes())
else Iterator.empty
}
}
catch {
case e: NoSuchFileException =>
logger.debugStackTrace(e)
logger.debug(s"JAR file $jar not found: $e, skipping.")
Iterator.empty
}

}
21 changes: 2 additions & 19 deletions modules/build/src/main/scala/scala/build/internal/MainClass.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import java.io.{ByteArrayInputStream, InputStream}
import java.nio.file.NoSuchFileException
import java.util.jar.{Attributes, JarFile}

import scala.build.internal.zip.WrappedZipInputStream
import scala.build.{Logger, retry}

object MainClass {
Expand Down Expand Up @@ -73,24 +72,8 @@ object MainClass {
finally is.close()

private def findInJar(path: os.Path, logger: Logger): Iterator[String] =
try retry()(logger) {
val content = os.read.bytes(path)
val jarInputStream = WrappedZipInputStream.create(new ByteArrayInputStream(content))
jarInputStream.entries().flatMap(ent =>
if !ent.isDirectory && ent.getName.endsWith(".class") then {
val content = jarInputStream.readAllBytes()
val inputStream = new ByteArrayInputStream(content)
findInClass(inputStream, logger)
}
else Iterator.empty
)
}
catch {
case e: NoSuchFileException =>
logger.debugStackTrace(e)
logger.log(s"JAR file $path not found: $e, trying to recover...")
logger.log("Are you trying to run too many builds at once? Trying to recover...")
Iterator.empty
JarUtils.walkClassEntries(path, logger) { (_, bytes) =>
findInClass(new ByteArrayInputStream(bytes()), logger)
}

def findInDependency(jar: os.Path): Option[String] =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package scala.build.internal

import scala.build.{Logger, Sources}

object ScriptUtils {
private val ignoredPackageRoots = Set("scala", "java", "javax", "META-INF")

final case class ScriptDescriptor(
name: String,
subPath: os.SubPath,
filePath: os.Path,
shadowedDependencyJars: Seq[String] = Nil
)

def findShadowedDependencyPackages(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we walking the classpath every time? Does it make running scripts much slower?

Or maybe we could only run it if any errors are present?

sources: Sources,
classPath: Seq[os.Path],
logger: Logger
): Seq[ScriptDescriptor] = {
val scripts = sources.scriptTopLevelNames.filterNot(s => ignoredPackageRoots(s.name))
if scripts.isEmpty then Nil
else
val packageRoots =
classPath
.filter(_.last.endsWith(".jar"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the same happen if we had locally package with the same name, is it limited to jars?

.flatMap(path =>
JarUtils.walkClassEntries(path, logger) { (name, _) =>
val slashIdx = name.indexOf('/')
if slashIdx > 0 then Iterator.single(name.take(slashIdx))
else Iterator.empty
}.toSet.map(_ -> path)
)
.groupMap((root, _) => root)((_, path) => path)
.view.mapValues(_.toSet).toMap
scripts.flatMap { script =>
packageRoots.get(script.name).map { jars =>
script.copy(shadowedDependencyJars = jars.toSeq.map(_.last).sorted)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package scala.build.internal.util

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

def scriptNameShadowsDependencyPackage(shadowed: ScriptDescriptor): String =
val name = shadowed.name
val depsClause = shadowed.shadowedDependencyJars match {
case Nil => "from an unknown dependency"
case Seq(j) => s"from dependency '$j'"
case js => s"from dependencies: ${js.map(j => s"'$j'").mkString(", ")}"
}
s"""Script '${shadowed.subPath}' generates a top-level symbol '$name' that shadows the '$name' package $depsClause.
|Imports of '$name.*' from the dependency will not resolve.
|Consider renaming the script (e.g. to '${name}1.sc') or moving it into a subdirectory.""".stripMargin

private val deprecationNote =
"Deprecated features may be removed in a future version."

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ case object ScriptPreprocessor extends Preprocessor {
inputArgPath.getOrElse(subPath.toString)
)

(pkg :+ wrapper).map(_.raw).mkString(".")
val relPath = os.rel / (subPath / os.up) / s"${subPath.last.stripSuffix(".sc")}.scala"

val file = PreprocessedSource.UnwrappedScript(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,56 @@ trait RunScriptTestDefinitions { this: RunTestDefinitions =>
}
}

test("warn when script name shadows a dependency top-level package") {
val inputs = TestInputs(
os.rel / "os.sc" ->
s"""//> using dep com.lihaoyi::os-lib:0.11.8
|println("hi")
|""".stripMargin
)
inputs.fromRoot { root =>
val res = os.proc(TestUtil.cli, extraOptions, "os.sc")
.call(cwd = root, check = false, mergeErrIntoOut = true)
val output = res.out.trim()
expect(output.contains("shadows the 'os' package"))
expect(res.exitCode == 0)
}
}

test("warn with multiple JARs when script name shadows a shared package root") {
val inputs = TestInputs(
os.rel / "cats.sc" ->
s"""//> using dep org.typelevel::cats-core:2.12.0
|println("hi")
|""".stripMargin
)
inputs.fromRoot { root =>
val res = os.proc(TestUtil.cli, extraOptions, "cats.sc")
.call(cwd = root, check = false, mergeErrIntoOut = true)
val output = res.out.trim()
expect(output.contains("shadows the 'cats' package from dependencies:"))
expect(output.contains("cats-core"))
expect(output.contains("cats-kernel"))
expect(res.exitCode == 0)
}
}

test("do not warn when script name does not shadow a dependency top-level package") {
val inputs = TestInputs(
os.rel / "safe.sc" ->
s"""//> using dep com.lihaoyi::os-lib:0.11.8
|println("hi")
|""".stripMargin
)
inputs.fromRoot { root =>
val res = os.proc(TestUtil.cli, extraOptions, "safe.sc")
.call(cwd = root, check = false, mergeErrIntoOut = true)
val output = res.out.trim()
expect(!output.contains("shadows the 'os' package"))
expect(res.exitCode == 0)
}
}

test("Directory") {
val message = "Hello"
val inputs = TestInputs(
Expand Down