Merge pull request #1323 from Gedochao/improve-error-messages

Improve the error message for when a build's main class is ambiguous
VirtusLab · Sep 6, 2022 · 5dc16d1 · 5dc16d1
2 parents 6b35e2c + 5fa98ff
commit 5dc16d1
Show file tree

Hide file tree

Showing 11 changed files with 130 additions and 75 deletions.
diff --git a/modules/build/src/main/scala/scala/build/Build.scala b/modules/build/src/main/scala/scala/build/Build.scala
@@ -59,6 +59,7 @@ object Build {
     def foundMainClasses(): Seq[String]   = MainClass.find(output)
     def retainedMainClass(
       mainClasses: Seq[String],
+      commandString: String,
       logger: Logger
     ): Either[MainClassError, String] = {
       val defaultMainClassOpt = sources.defaultMainClass
@@ -75,8 +76,9 @@ object Build {
                   mainClasses.toList
                 )
                 .toRight {
-                  new SeveralMainClassesFoundError(
+                  SeveralMainClassesFoundError(
                     ::(mainClasses.head, mainClasses.tail.toList),
+                    commandString,
                     Nil
                   )
                 }

diff --git a/modules/cli/src/main/scala/scala/cli/commands/Default.scala b/modules/cli/src/main/scala/scala/cli/commands/Default.scala
@@ -15,8 +15,6 @@ class Default(
   private def defaultHelp: String     = actualHelp.help(ScalaCliHelp.helpFormat)
   private def defaultFullHelp: String = actualHelp.help(ScalaCliHelp.helpFormat, showHidden = true)
 
-  override protected def commandLength = 0
-
   override def group                                                         = "Main"
   override def sharedOptions(options: DefaultOptions): Option[SharedOptions] = Some(options.shared)
   private[cli] var rawArgs                                                   = Array.empty[String]

diff --git a/modules/cli/src/main/scala/scala/cli/commands/Package.scala b/modules/cli/src/main/scala/scala/cli/commands/Package.scala
@@ -1,8 +1,8 @@
 package scala.cli.commands
 
-import caseapp._
-import coursier.launcher._
-import packager.config._
+import caseapp.*
+import coursier.launcher.*
+import packager.config.*
 import packager.deb.DebianPackage
 import packager.docker.DockerPackage
 import packager.mac.dmg.DmgPackage
@@ -14,35 +14,29 @@ import java.io.{ByteArrayOutputStream, OutputStream}
 import java.nio.charset.StandardCharsets
 import java.nio.file.attribute.FileTime
 import java.util.zip.{ZipEntry, ZipOutputStream}
-
+import scala.build.*
 import scala.build.EitherCps.{either, value}
-import scala.build.Ops._
-import scala.build._
-import scala.build.errors.{
-  BuildException,
-  CompositeBuildException,
-  MalformedCliInputError,
-  NoMainClassFoundError,
-  ScalaNativeBuildError
-}
+import scala.build.Ops.*
+import scala.build.errors.*
 import scala.build.interactive.InteractiveFileOps
-import scala.build.internal.Util._
+import scala.build.internal.Util.*
 import scala.build.internal.{Runner, ScalaJsLinkerConfig}
 import scala.build.options.{PackageType, Platform}
 import scala.cli.CurrentParams
-import scala.cli.commands.OptionsHelper._
+import scala.cli.commands.OptionsHelper.*
 import scala.cli.commands.packaging.Spark
-import scala.cli.commands.util.MainClassOptionsUtil._
-import scala.cli.commands.util.PackageOptionsUtil._
-import scala.cli.commands.util.SharedOptionsUtil._
+import scala.cli.commands.util.BuildCommandHelpers
+import scala.cli.commands.util.CommonOps.SharedDirectoriesOptionsOps
+import scala.cli.commands.util.MainClassOptionsUtil.*
+import scala.cli.commands.util.PackageOptionsUtil.*
+import scala.cli.commands.util.SharedOptionsUtil.*
+import scala.cli.config.{ConfigDb, Keys}
 import scala.cli.errors.ScalaJsLinkingError
 import scala.cli.internal.{CachedBinary, ProcUtil, ScalaJsLinker}
 import scala.cli.packaging.{Library, NativeImage}
 import scala.util.Properties
-import scala.cli.config.{ConfigDb, Keys}
-import scala.cli.commands.util.CommonOps.SharedDirectoriesOptionsOps
 
-object Package extends ScalaCommand[PackageOptions] {
+object Package extends ScalaCommand[PackageOptions] with BuildCommandHelpers {
   override def name                                                          = "package"
   override def group                                                         = "Main"
   override def inSipScala                                                    = false
@@ -259,7 +253,7 @@ object Package extends ScalaCommand[PackageOptions] {
             .map(_.stripSuffix("_sc"))
             .map(_ + extension)
         }
-        .orElse(build.retainedMainClass(build.foundMainClasses(), logger).map(
+        .orElse(build.retainedMainClass(logger).map(
           _.stripSuffix("_sc") + extension
         ).toOption)
         .orElse(build.sources.paths.collectFirst(_._1.baseName + extension))
@@ -286,7 +280,7 @@ object Package extends ScalaCommand[PackageOptions] {
       def mainClass: Either[BuildException, String] =
         build.options.mainClass match {
           case Some(cls) => Right(cls)
-          case None      => build.retainedMainClass(build.foundMainClasses(), logger)
+          case None      => build.retainedMainClass(logger)
         }
 
       def mainClassOpt: Option[String] =

diff --git a/modules/cli/src/main/scala/scala/cli/commands/Run.scala b/modules/cli/src/main/scala/scala/cli/commands/Run.scala
@@ -1,25 +1,24 @@
 package scala.cli.commands
 
-import caseapp._
+import caseapp.*
 
 import java.util.concurrent.CompletableFuture
-
 import scala.build.EitherCps.{either, value}
 import scala.build.errors.BuildException
 import scala.build.internal.{Constants, Runner, ScalaJsLinkerConfig}
 import scala.build.options.{BuildOptions, JavaOpt, Platform}
-import scala.build.{Build, BuildThreads, Inputs, Logger, Positioned}
+import scala.build.*
 import scala.cli.CurrentParams
 import scala.cli.commands.run.RunMode
-import scala.cli.commands.util.MainClassOptionsUtil._
-import scala.cli.commands.util.SharedOptionsUtil._
+import scala.cli.commands.util.CommonOps.SharedDirectoriesOptionsOps
+import scala.cli.commands.util.MainClassOptionsUtil.*
+import scala.cli.commands.util.SharedOptionsUtil.*
+import scala.cli.commands.util.{BuildCommandHelpers, RunHadoop, RunSpark}
+import scala.cli.config.{ConfigDb, Keys}
 import scala.cli.internal.ProcUtil
 import scala.util.Properties
-import scala.cli.config.{ConfigDb, Keys}
-import scala.cli.commands.util.CommonOps.SharedDirectoriesOptionsOps
-import scala.cli.commands.util.{RunHadoop, RunSpark}
 
-object Run extends ScalaCommand[RunOptions] {
+object Run extends ScalaCommand[RunOptions] with BuildCommandHelpers {
   override def group = "Main"
 
   override def sharedOptions(options: RunOptions): Option[SharedOptions] = Some(options.shared)
@@ -53,8 +52,8 @@ object Run extends ScalaCommand[RunOptions] {
   }
 
   def buildOptions(options: RunOptions): BuildOptions = {
-    import options._
-    import options.sharedRun._
+    import options.*
+    import options.sharedRun.*
     val baseOptions = shared.buildOptions(
       enableJmh = benchmarking.jmh.contains(true),
       jmhVersion = benchmarking.jmhVersion
@@ -283,7 +282,7 @@ object Run extends ScalaCommand[RunOptions] {
       }
     val mainClass = mainClassOpt match {
       case Some(cls) => cls
-      case None      => value(build.retainedMainClass(potentialMainClasses, logger))
+      case None      => value(build.retainedMainClass(logger, mainClasses = potentialMainClasses))
     }
     val verbosity = build.options.internal.verbosity.getOrElse(0).toString
 

diff --git a/modules/cli/src/main/scala/scala/cli/commands/ScalaCommand.scala b/modules/cli/src/main/scala/scala/cli/commands/ScalaCommand.scala
@@ -8,11 +8,13 @@ import caseapp.core.parser.Parser
 import caseapp.core.util.Formatter
 import caseapp.core.{Arg, Error}
 
+import scala.annotation.tailrec
 import scala.build.compiler.SimpleScalaCompiler
 import scala.build.internal.Constants
 import scala.build.options.{BuildOptions, Scope}
+import scala.cli.ScalaCli
 import scala.cli.commands.util.CommandHelpers
-import scala.cli.commands.util.SharedOptionsUtil._
+import scala.cli.commands.util.SharedOptionsUtil.*
 import scala.util.{Properties, Try}
 
 abstract class ScalaCommand[T](implicit parser: Parser[T], help: Help[T])
@@ -28,20 +30,39 @@ abstract class ScalaCommand[T](implicit parser: Parser[T], help: Help[T])
     argvOpt = Some(argv)
   }
 
+  /** @return the actual Scala CLI program name which was run */
+  protected def progName: String = ScalaCli.progName
+
   // TODO Manage to have case-app give use the exact command name that was used instead
-  protected def commandLength: Int = names.headOption.fold(1)(_.length)
+  /** The actual sub-command name that was used. If the sub-command name is a list of strings, space
+    * is used as the separator. If [[argvOpt]] hasn't been defined, it defaults to [[name]].
+    */
+  protected def actualCommandName: String =
+    argvOpt.map { argv =>
+      @tailrec
+      def validCommand(potentialCommandName: List[String]): Option[List[String]] =
+        if potentialCommandName.isEmpty then None
+        else
+          names.find(_ == potentialCommandName) match {
+            case cmd @ Some(_) => cmd
+            case _             => validCommand(potentialCommandName.dropRight(1))
+          }
 
-  override def error(message: Error): Nothing = {
-    System.err.println(message.message)
+      val maxCommandLength: Int    = names.map(_.length).max max 1
+      val maxPotentialCommandNames = argv.slice(1, maxCommandLength + 1).toList
+      validCommand(maxPotentialCommandNames).getOrElse(List(""))
+    }.getOrElse(List(name)).mkString(" ")
 
-    for (argv <- argvOpt if argv.length >= 1 + commandLength) {
-      System.err.println()
-      System.err.println("To list all available options, run")
-      System.err.println(
-        s"  ${Console.BOLD}${argv.take(1 + commandLength).mkString(" ")} --help${Console.RESET}"
-      )
-    }
+  protected def actualFullCommand: String =
+    if actualCommandName.nonEmpty then s"$progName $actualCommandName" else progName
 
+  override def error(message: Error): Nothing = {
+    System.err.println(
+      s"""${message.message}
+         |
+         |To list all available options, run
+         |  ${Console.BOLD}$actualFullCommand --help${Console.RESET}""".stripMargin
+    )
     sys.exit(1)
   }
 

diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala
@@ -21,30 +21,30 @@ import java.nio.file.Paths
 import java.time.{Instant, LocalDateTime, ZoneOffset}
 import java.util.concurrent.Executors
 import java.util.function.Supplier
-
+import scala.build.*
 import scala.build.EitherCps.{either, value}
-import scala.build.Ops._
-import scala.build._
+import scala.build.Ops.*
 import scala.build.compiler.ScalaCompilerMaker
 import scala.build.errors.{BuildException, CompositeBuildException, NoMainClassFoundError}
 import scala.build.internal.Util
 import scala.build.internal.Util.ScalaDependencyOps
-import scala.build.options.publish.{ComputeVersion, Developer, License, Signer => PSigner, Vcs}
+import scala.build.options.publish.{ComputeVersion, Developer, License, Vcs, Signer as PSigner}
 import scala.build.options.{BuildOptions, ConfigMonoid, PublishContextualOptions, Scope}
 import scala.cli.CurrentParams
 import scala.cli.commands.pgp.PgpExternalCommand
 import scala.cli.commands.publish.{PublishParamsOptions, PublishRepositoryOptions}
 import scala.cli.commands.util.CommonOps.SharedDirectoriesOptionsOps
-import scala.cli.commands.util.MainClassOptionsUtil._
-import scala.cli.commands.util.ScalaCliSttpBackend
-import scala.cli.commands.util.SharedOptionsUtil._
-import scala.cli.commands.util.PublishUtils._
+import scala.cli.commands.util.MainClassOptionsUtil.*
+import scala.cli.commands.util.SharedOptionsUtil.*
+import scala.cli.commands.util.{BuildCommandHelpers, ScalaCliSttpBackend}
+import scala.cli.commands.util.SharedOptionsUtil.*
+import scala.cli.commands.util.PublishUtils.*
 import scala.cli.commands.{
   MainClassOptions,
-  Package => PackageCmd,
   ScalaCommand,
   SharedOptions,
-  WatchUtil
+  WatchUtil,
+  Package as PackageCmd
 }
 import scala.cli.config.{ConfigDb, Keys}
 import scala.cli.errors.{
@@ -55,9 +55,9 @@ import scala.cli.errors.{
 }
 import scala.cli.packaging.Library
 import scala.cli.publish.BouncycastleSignerMaker
-import scala.cli.util.ConfigPasswordOptionHelpers._
+import scala.cli.util.ConfigPasswordOptionHelpers.*
 
-object Publish extends ScalaCommand[PublishOptions] {
+object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers {
 
   override def group: String       = "Main"
   override def inSipScala: Boolean = false
@@ -472,8 +472,7 @@ object Publish extends ScalaCommand[PublishOptions] {
 
     val mainJar = {
       val mainClassOpt = build.options.mainClass.orElse {
-        val potentialMainClasses = build.foundMainClasses()
-        build.retainedMainClass(potentialMainClasses, logger) match {
+        build.retainedMainClass(logger) match {
           case Left(_: NoMainClassFoundError) => None
           case Left(err) =>
             logger.debug(s"Error while looking for main class: $err")

diff --git a/modules/cli/src/main/scala/scala/cli/commands/util/BuildCommandHelpers.scala b/modules/cli/src/main/scala/scala/cli/commands/util/BuildCommandHelpers.scala
@@ -0,0 +1,19 @@
+package scala.cli.commands.util
+
+import scala.build.{Build, Logger}
+import scala.build.errors.MainClassError
+import scala.cli.commands.ScalaCommand
+
+trait BuildCommandHelpers { self: ScalaCommand[_] =>
+  extension (successfulBuild: Build.Successful) {
+    def retainedMainClass(
+      logger: Logger,
+      mainClasses: Seq[String] = successfulBuild.foundMainClasses()
+    ): Either[MainClassError, String] =
+      successfulBuild.retainedMainClass(
+        mainClasses,
+        self.argvOpt.map(_.mkString(" ")).getOrElse(actualFullCommand),
+        logger
+      )
+  }
+}
diff --git a/modules/core/src/main/scala/scala/build/errors/SeveralMainClassesFoundError.scala b/modules/core/src/main/scala/scala/build/errors/SeveralMainClassesFoundError.scala
@@ -4,8 +4,20 @@ import scala.build.Position
 
 final class SeveralMainClassesFoundError(
   mainClasses: ::[String],
+  commandString: String,
   positions: Seq[Position]
 ) extends MainClassError(
-      s"Found several main classes: ${mainClasses.mkString(", ")}",
+      {
+        val sortedMainClasses = mainClasses.sorted
+        val mainClassesString = sortedMainClasses.mkString(", ")
+        s"""Found several main classes: $mainClassesString
+           |${sortedMainClasses.headOption.map(mc =>
+            s"""You can run one of them by passing it with the --main-class option, e.g.
+               |  ${Console.BOLD}$commandString --main-class $mc${Console.RESET}
+               |""".stripMargin
+          ).getOrElse("")}
+           |You can pick the main class interactively by passing the --interactive option.
+           |  ${Console.BOLD}$commandString --interactive${Console.RESET}""".stripMargin
+      },
       positions = positions
     )
diff --git a/modules/integration/src/test/scala/scala/cli/integration/DefaultTests.scala b/modules/integration/src/test/scala/scala/cli/integration/DefaultTests.scala
@@ -94,13 +94,11 @@ class DefaultTests extends ScalaCliSuite {
     }
   }
 
-  private def unrecognizedArgMessage(argName: String) = {
-    val scalaCli = if (TestUtil.isNativeCli) TestUtil.cliPath else "scala-cli"
+  private def unrecognizedArgMessage(argName: String) =
     s"""
        |Unrecognized argument: $argName
        |
        |To list all available options, run
-       |  ${Console.BOLD}$scalaCli --help${Console.RESET}
+       |  ${Console.BOLD}${TestUtil.detectCliPath} --help${Console.RESET}
        |""".stripMargin.trim
-  }
 }
diff --git a/modules/integration/src/test/scala/scala/cli/integration/RunTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/RunTestDefinitions.scala
@@ -1900,7 +1900,7 @@ abstract class RunTestDefinitions(val scalaVersionOpt: Option[String])
 
   test("return relevant error if multiple .scala main classes are present") {
     val (scalaFile1, scalaFile2, scriptName) = ("ScalaMainClass1", "ScalaMainClass2", "ScalaScript")
-    val scriptsDir                           = "scritps"
+    val scriptsDir                           = "scripts"
     val inputs = TestInputs(
       os.rel / s"$scalaFile1.scala"           -> s"object $scalaFile1 extends App { println() }",
       os.rel / s"$scalaFile2.scala"           -> s"object $scalaFile2 extends App { println() }",
@@ -1910,15 +1910,27 @@ abstract class RunTestDefinitions(val scalaVersionOpt: Option[String])
       val res = os.proc(
         TestUtil.cli,
         "run",
-        extraOptions,
-        "."
+        ".",
+        extraOptions
       )
         .call(cwd = root, mergeErrIntoOut = true, check = false)
       expect(res.exitCode == 1)
-      val output          = res.out.text().trim
-      val Some(errorLine) = output.linesIterator.find(_.contains("Found several main classes"))
-      val mainClasses     = errorLine.split(":").last.trim.split(", ").toSet
-      expect(mainClasses == Set(scalaFile1, scalaFile2, s"$scriptsDir.${scriptName}_sc"))
+      val output = res.out.text().trim
+      val errorMessage =
+        output.linesWithSeparators.toSeq.takeRight(6).mkString // dropping compilation logs
+      val extraOptionsString = extraOptions.mkString(" ")
+      val expectedMainClassNames =
+        Seq(scalaFile1, scalaFile2, s"$scriptsDir.${scriptName}_sc").sorted
+      val expectedErrorMessage =
+        s"""[${Console.RED}error${Console.RESET}]  Found several main classes: ${expectedMainClassNames.mkString(
+            ", "
+          )}
+           |You can run one of them by passing it with the --main-class option, e.g.
+           |  ${Console.BOLD}${TestUtil.detectCliPath} run . $extraOptionsString --main-class ${expectedMainClassNames.head}${Console.RESET}
+           |
+           |You can pick the main class interactively by passing the --interactive option.
+           |  ${Console.BOLD}${TestUtil.detectCliPath} run . $extraOptionsString --interactive${Console.RESET}""".stripMargin
+      expect(errorMessage == expectedErrorMessage)
     }
   }
 

diff --git a/modules/integration/src/test/scala/scala/cli/integration/TestUtil.scala b/modules/integration/src/test/scala/scala/cli/integration/TestUtil.scala
@@ -16,6 +16,7 @@ object TestUtil {
   val isNativeCli: Boolean = cliKind.startsWith("native")
   val isCI: Boolean        = System.getenv("CI") != null
   val cliPath: String      = sys.props("test.scala-cli.path")
+  val detectCliPath        = if (TestUtil.isNativeCli) TestUtil.cliPath else "scala-cli"
   val cli: Seq[String]     = cliCommand(cliPath)
 
   def cliCommand(cliPath: String): Seq[String] =