Verify clashes post-error

Author: Gedochao

modules/build/src/main/scala/scala/build/BloopBuildClient.scala

@@ -7,6 +7,8 @@ import scala.build.options.Scope
 trait BloopBuildClient extends bsp4j.BuildClient {
   def setProjectParams(newParams: Seq[String]): Unit
   def setGeneratedSources(scope: Scope, newGeneratedSources: Seq[GeneratedSource]): Unit
+  def setSuspectedClashNames(names: Set[String]): Unit
+  def detectedShadowingCandidates: Set[String]
   def diagnostics: Option[Seq[(Either[String, os.Path], bsp4j.Diagnostic)]]
   def clear(): Unit
 }

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

@@ -1130,18 +1130,6 @@ 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 {
@@ -1236,12 +1224,32 @@ object Build {
     buildClient.clear()
     buildClient.setGeneratedSources(scope, generatedSources)
 
+    val scriptNames = sources.scriptTopLevelNames.iterator.map(_.name).toSet
+    buildClient.setSuspectedClashNames(scriptNames)
+
     val partial = partialOpt.getOrElse {
       options.notForBloopOptions.packageOptions.packageTypeOpt.exists(_.sourceBased)
     }
 
     val success = partial || compiler.compile(project, logger)
 
+    if !success && scriptNames.nonEmpty then
+      val matched = buildClient.detectedShadowingCandidates
+      if matched.nonEmpty then
+        for
+          shadowed <- ScriptUtils.findShadowingClashes(
+            sources = sources,
+            classPath = artifacts.compileClassPath,
+            logger = logger
+          )
+          if matched(shadowed.name)
+        do
+          logger.diagnostic(
+            message = WarningMessages.scriptShadowingClashHint(shadowed),
+            severity = Severity.Error,
+            positions = Seq(Position.File(Right(shadowed.filePath), (0, 0), (0, 0)))
+          )
+
     if success then
       Successful(
         inputs = inputs,

modules/build/src/main/scala/scala/build/ConsoleBloopBuildClient.scala

@@ -5,6 +5,7 @@ import ch.epfl.scala.bsp4j
 import java.io.File
 import java.net.URI
 import java.nio.file.{NoSuchFileException, Paths}
+import java.util.concurrent.atomic.AtomicReference
 
 import scala.build.errors.Severity
 import scala.build.internal.WrapperParams
@@ -32,6 +33,9 @@ class ConsoleBloopBuildClient(
 
   private val diagnostics0 = new mutable.ListBuffer[(Either[String, os.Path], bsp4j.Diagnostic)]
 
+  private val suspectedClashNames = new AtomicReference(Set.empty[String])
+  private val matchedClashNames   = new AtomicReference(Set.empty[String])
+
   def setGeneratedSources(scope: Scope, newGeneratedSources: Seq[GeneratedSource]) =
     generatedSources(scope) = newGeneratedSources
   def setProjectParams(newParams: Seq[String]): Unit = {
@@ -41,6 +45,20 @@ class ConsoleBloopBuildClient(
     if (keepDiagnostics) Some(diagnostics0.result())
     else None
 
+  def setSuspectedClashNames(names: Set[String]): Unit =
+    suspectedClashNames.set(names)
+
+  def detectedShadowingCandidates: Set[String] =
+    matchedClashNames.get()
+
+  private def namesMentionedInMessage(message: String, names: Set[String]): Set[String] =
+    names.filter { name =>
+      message.contains(name) ||
+      message.contains(s"$name$$_") ||
+      message.contains(s"object $name") ||
+      message.contains(s"`$name`")
+    }
+
   private def postProcessDiagnostic(
     path: os.Path,
     diag: bsp4j.Diagnostic,
@@ -81,6 +99,10 @@ class ConsoleBloopBuildClient(
       val path = os.Path(Paths.get(new URI(params.getTextDocument.getUri)).toAbsolutePath)
       val (updatedPath, updatedDiag) = postProcessDiagnostic(path, diag, diagnosticMappings)
         .getOrElse((Right(path), diag))
+      if updatedDiag.getSeverity == bsp4j.DiagnosticSeverity.ERROR then
+        val mentioned = namesMentionedInMessage(updatedDiag.getMessage, suspectedClashNames.get())
+        if mentioned.nonEmpty then
+          matchedClashNames.updateAndGet(_ ++ mentioned)
       if (keepDiagnostics)
         diagnostics0 += updatedPath -> updatedDiag
       ConsoleBloopBuildClient.printFileDiagnostic(logger, updatedPath, updatedDiag)
@@ -137,6 +159,8 @@ class ConsoleBloopBuildClient(
   def clear(): Unit = {
     generatedSources.clear()
     diagnostics0.clear()
+    suspectedClashNames.set(Set.empty)
+    matchedClashNames.set(Set.empty)
     printedStart = false
   }
 }

modules/build/src/main/scala/scala/build/bsp/BspClient.scala

@@ -115,6 +115,8 @@ class BspClient(
   }
 
   def setProjectParams(newParams: Seq[String]): Unit                    = {}
+  def setSuspectedClashNames(names: Set[String]): Unit                  = {}
+  def detectedShadowingCandidates: Set[String]                          = Set.empty
   def diagnostics: Option[Seq[(Either[String, os.Path], b.Diagnostic)]] = None
   def clear(): Unit                                                     = {}
 

modules/build/src/main/scala/scala/build/bsp/BspImpl.scala

@@ -720,6 +720,10 @@ object BspImpl {
       underlying.setProjectParams(newParams)
     def setGeneratedSources(scope: Scope, newGeneratedSources: Seq[GeneratedSource]) =
       underlying.setGeneratedSources(scope, newGeneratedSources)
+    def setSuspectedClashNames(names: Set[String]) =
+      underlying.setSuspectedClashNames(names)
+    def detectedShadowingCandidates =
+      underlying.detectedShadowingCandidates
   }
 
   private final case class PreBuildData(

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

@@ -9,33 +9,55 @@ object ScriptUtils {
     name: String,
     subPath: os.SubPath,
     filePath: os.Path,
-    shadowedDependencyJars: Seq[String] = Nil
+    shadowedDependencyJars: Seq[String] = Nil,
+    clashingLocalSources: Seq[os.SubPath] = Nil
   )
 
-  def findShadowedDependencyPackages(
+  def findShadowingClashes(
     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"))
-          .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
+      val localCandidates = localTopLevelCandidates(sources)
+      val packageRoots    = topLevelPackageRoots(classPath, logger)
       scripts.flatMap { script =>
-        packageRoots.get(script.name).map { jars =>
-          script.copy(shadowedDependencyJars = jars.toSeq.map(_.last).sorted)
+        val deps   = packageRoots.getOrElse(script.name, Set.empty).toSeq.map(_.last).sorted
+        val locals = localCandidates.getOrElse(script.name, Nil).filterNot(_ == script.subPath)
+        Option.when(deps.nonEmpty || locals.nonEmpty) {
+          script.copy(shadowedDependencyJars = deps, clashingLocalSources = locals)
         }
       }
   }
+
+  private def localTopLevelCandidates(sources: Sources): Map[String, Seq[os.SubPath]] =
+    (sources.paths.map((_, rel) => (baseName(rel.last), os.SubPath(rel.segments.toIndexedSeq))) ++
+      sources.inMemory.collect {
+        case Sources.InMemory(Right((subPath, _)), _, _, Some(_)) =>
+          (baseName(subPath.last), subPath)
+      })
+      .groupMap(_._1)(_._2)
+
+  private def baseName(fileName: String): String =
+    val dot = fileName.lastIndexOf('.')
+    if dot > 0 then fileName.take(dot) else fileName
+
+  private def topLevelPackageRoots(
+    classPath: Seq[os.Path],
+    logger: Logger
+  ): Map[String, Set[os.Path]] =
+    classPath
+      .filter(_.last.endsWith(".jar"))
+      .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
+
 }

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

@@ -131,15 +131,22 @@ 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(", ")}"
+  def scriptShadowingClashHint(shadowed: ScriptDescriptor): String =
+    val name      = shadowed.name
+    val depClause = shadowed.shadowedDependencyJars match {
+      case Nil    => None
+      case Seq(j) => Some(s"the '$name' package from dependency '$j'")
+      case js     =>
+        Some(s"the '$name' package 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.
+    val localClause = shadowed.clashingLocalSources match {
+      case Nil    => None
+      case Seq(p) => Some(s"a local source '$p'")
+      case ps     => Some(s"local sources: ${ps.map(p => s"'$p'").mkString(", ")}")
+    }
+    val clashes = Seq(depClause, localClause).flatten.mkString(" and ")
+    s"""Script '${shadowed.subPath}' generates a top-level symbol '$name' that shadows $clashes.
+       |This is likely the cause of compilation errors mentioning '$name'.
        |Consider renaming the script (e.g. to '${name}1.sc') or moving it into a subdirectory.""".stripMargin
 
   private val deprecationNote =

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

@@ -270,37 +270,75 @@ trait RunScriptTestDefinitions { this: RunTestDefinitions =>
       }
     }
 
-  test("warn when script name shadows a dependency top-level package") {
+  test("hint when script name shadows a dependency top-level package and import fails") {
     val inputs = TestInputs(
       os.rel / "os.sc" ->
         s"""//> using dep com.lihaoyi::os-lib:0.11.8
-           |println("hi")
+           |import os.Path
+           |println(Path("/tmp"))
            |""".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)
+      expect(res.exitCode != 0)
+      expect(output.contains("shadows the 'os' package from dependencies:"))
+      expect(output.contains("os-lib"))
+      expect(output.contains("is likely the cause of compilation errors mentioning 'os'"))
     }
   }
 
-  test("warn with multiple JARs when script name shadows a shared package root") {
+  test("hint 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")
+           |import cats.Show
+           |println(Show[Int])
            |""".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(res.exitCode != 0)
       expect(output.contains("shadows the 'cats' package from dependencies:"))
       expect(output.contains("cats-core"))
       expect(output.contains("cats-kernel"))
-      expect(res.exitCode == 0)
+    }
+  }
+
+  test("hint when script name clashes with a sibling .scala file") {
+    val inputs = TestInputs(
+      os.rel / "foo.sc" ->
+        """println("hi")""",
+      os.rel / "foo.scala" ->
+        """object foo { def hello = "world" }"""
+    )
+    inputs.fromRoot { root =>
+      val res = os.proc(TestUtil.cli, extraOptions, "foo.sc", "foo.scala")
+        .call(cwd = root, check = false, mergeErrIntoOut = true)
+      val output = res.out.trim()
+      expect(res.exitCode != 0)
+      expect(output.contains("foo.sc"))
+      expect(output.contains("local source 'foo.scala'") || output.contains("local sources:"))
+    }
+  }
+
+  test("do not hint when compile fails for an unrelated reason") {
+    val inputs = TestInputs(
+      os.rel / "safe.sc" ->
+        s"""//> using dep com.lihaoyi::os-lib:0.11.8
+           |val x: Int = "not an int"
+           |""".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(res.exitCode != 0)
+      expect(!output.contains("shadows"))
+      expect(!output.contains("is likely the cause"))
     }
   }
 
@@ -315,7 +353,8 @@ trait RunScriptTestDefinitions { this: RunTestDefinitions =>
       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(!output.contains("shadows"))
+      expect(!output.contains("is likely the cause"))
       expect(res.exitCode == 0)
     }
   }