Fix directive error handling

Author: alexarchambault

modules/build/src/main/scala/scala/build/errors/MalformedDirectiveError.scala

@@ -0,0 +1,6 @@
+package scala.build.errors
+
+import scala.build.Position
+
+final class MalformedDirectiveError(message: String, positions: Seq[Position])
+    extends BuildException(message, positions)

modules/build/src/main/scala/scala/build/preprocessing/CustomDirectivesReporter.scala

@@ -0,0 +1,50 @@
+package scala.build.preprocessing
+
+import com.virtuslab.using_directives.custom.utils.{Position => DirectivePosition}
+import com.virtuslab.using_directives.reporter.Reporter
+
+import scala.build.Position
+import scala.build.errors.{Diagnostic, Severity}
+
+class CustomDirectivesReporter(path: Either[String, os.Path], onDiagnostic: Diagnostic => Unit)
+    extends Reporter {
+
+  private var errorCount = 0
+
+  private def toScalaCliPosition(position: DirectivePosition): Position = {
+    val coords = (position.getLine, position.getColumn)
+    Position.File(path, coords, coords)
+  }
+
+  override def error(msg: String): Unit =
+    onDiagnostic {
+      errorCount += 1
+      Diagnostic(msg, Severity.Error)
+    }
+  override def error(position: DirectivePosition, msg: String): Unit =
+    onDiagnostic {
+      errorCount += 1
+      Diagnostic(msg, Severity.Error, Seq(toScalaCliPosition(position)))
+    }
+  override def warning(msg: String): Unit =
+    onDiagnostic {
+      Diagnostic(msg, Severity.Warning)
+    }
+  override def warning(position: DirectivePosition, msg: String): Unit =
+    onDiagnostic {
+      Diagnostic(msg, Severity.Warning, Seq(toScalaCliPosition(position)))
+    }
+
+  override def hasErrors(): Boolean =
+    errorCount != 0
+
+  override def reset(): Unit = {
+    errorCount = 0
+  }
+}
+
+object CustomDirectivesReporter {
+  def create(path: Either[String, os.Path])(onDiagnostic: Diagnostic => Unit)
+    : CustomDirectivesReporter =
+    new CustomDirectivesReporter(path, onDiagnostic)
+}

modules/build/src/main/scala/scala/build/preprocessing/DirectivesOutputStreamReporter.scala

@@ -15,16 +15,12 @@ import java.nio.charset.StandardCharsets
 
 import scala.build.EitherCps.{either, value}
 import scala.build.Ops._
-import scala.build.errors.{
-  BuildException,
-  CompositeBuildException,
-  DependencyFormatError,
-  UnusedDirectiveError
-}
+import scala.build.errors._
 import scala.build.internal.{AmmUtil, Util}
 import scala.build.options.{BuildOptions, BuildRequirements, ClassPathOptions, ShadowingSeq}
 import scala.build.preprocessing.directives._
 import scala.build.{Inputs, Logger, Position, Positioned}
+import scala.collection.mutable
 import scala.jdk.CollectionConverters._
 
 case object ScalaPreprocessor extends Preprocessor {
@@ -362,76 +358,88 @@ case object ScalaPreprocessor extends Preprocessor {
     path: Either[String, os.Path],
     logger: Logger
   ): Either[BuildException, ExtractedDirectives] = {
+    val errors = new mutable.ListBuffer[Diagnostic]
+    val reporter = CustomDirectivesReporter.create(path) { diag =>
+      if (diag.severity == Severity.Warning)
+        logger.log(Seq(diag))
+      else
+        errors += diag
+    }
     val processor = {
-      val reporter = new DirectivesOutputStreamReporter(System.err) // TODO Get that via a logger
       val settings = new Settings
       settings.setAllowStartWithoutAt(true)
       settings.setAllowRequire(false)
       val context = new Context(reporter, settings)
       new UsingDirectivesProcessor(context)
     }
     val all = processor.extract(contentChars, true, true).asScala
+    if (errors.isEmpty) {
 
-    def byKind(kind: UsingDirectiveKind) = all.find(_.getKind == kind).get
+      def byKind(kind: UsingDirectiveKind) = all.find(_.getKind == kind).get
 
-    def getDirectives(directives: UsingDirectives) =
-      directives.getAst() match {
-        case ud: UsingDefs =>
-          ud.getUsingDefs().asScala
-        case _ =>
-          Nil
-      }
+      def getDirectives(directives: UsingDirectives) =
+        directives.getAst() match {
+          case ud: UsingDefs =>
+            ud.getUsingDefs().asScala
+          case _ =>
+            Nil
+        }
 
-    val codeDirectives           = byKind(UsingDirectiveKind.Code)
-    val specialCommentDirectives = byKind(UsingDirectiveKind.SpecialComment)
-    val plainCommentDirectives   = byKind(UsingDirectiveKind.PlainComment)
-
-    def reportWarning(msg: String, values: Seq[UsingDef], before: Boolean = true): Unit =
-      values.foreach { v =>
-        val astPos = v.getPosition()
-        val (start, end) =
-          if (before) (0, astPos.getColumn())
-          else (astPos.getColumn(), astPos.getColumn() + v.getSyntax.getKeyword.size)
-        val position = Position.File(path, (astPos.getLine(), start), (astPos.getLine(), end))
-        logger.diagnostic(msg, positions = Seq(position))
-      }
+      val codeDirectives           = byKind(UsingDirectiveKind.Code)
+      val specialCommentDirectives = byKind(UsingDirectiveKind.SpecialComment)
+      val plainCommentDirectives   = byKind(UsingDirectiveKind.PlainComment)
+
+      def reportWarning(msg: String, values: Seq[UsingDef], before: Boolean = true): Unit =
+        values.foreach { v =>
+          val astPos = v.getPosition()
+          val (start, end) =
+            if (before) (0, astPos.getColumn())
+            else (astPos.getColumn(), astPos.getColumn() + v.getSyntax.getKeyword.size)
+          val position = Position.File(path, (astPos.getLine(), start), (astPos.getLine(), end))
+          logger.diagnostic(msg, positions = Seq(position))
+        }
 
-    val usedDirectives =
-      if (!codeDirectives.getFlattenedMap().isEmpty()) {
-        val msg =
-          "This using directive is ignored. File contains directives outside comments and those have higher precedence."
-        reportWarning(
-          msg,
-          getDirectives(plainCommentDirectives) ++ getDirectives(specialCommentDirectives)
-        )
-        codeDirectives
-      }
-      else if (!specialCommentDirectives.getFlattenedMap().isEmpty()) {
-        val msg =
-          s"This using directive is ignored. $changeToSpecialCommentMsg"
-        reportWarning(msg, getDirectives(plainCommentDirectives))
-        specialCommentDirectives
-      }
-      else {
-        reportWarning(changeToSpecialCommentMsg, getDirectives(plainCommentDirectives))
-        plainCommentDirectives
-      }
+      val usedDirectives =
+        if (!codeDirectives.getFlattenedMap().isEmpty()) {
+          val msg =
+            "This using directive is ignored. File contains directives outside comments and those have higher precedence."
+          reportWarning(
+            msg,
+            getDirectives(plainCommentDirectives) ++ getDirectives(specialCommentDirectives)
+          )
+          codeDirectives
+        }
+        else if (!specialCommentDirectives.getFlattenedMap().isEmpty()) {
+          val msg =
+            s"This using directive is ignored. $changeToSpecialCommentMsg"
+          reportWarning(msg, getDirectives(plainCommentDirectives))
+          specialCommentDirectives
+        }
+        else {
+          reportWarning(changeToSpecialCommentMsg, getDirectives(plainCommentDirectives))
+          plainCommentDirectives
+        }
+
+      // All using directives should use just `using` keyword, no @using or require
+      reportWarning(
+        "Deprecated using directive syntax, please use keyword `using`.",
+        getDirectives(usedDirectives).filter(_.getSyntax() != UsingDirectiveSyntax.Using),
+        before = false
+      )
+
+      val flattened = usedDirectives.getFlattenedMap.asScala.toSeq
+      val strictDirectives =
+        flattened.map { case (k, l) => StrictDirective(k.getPath.asScala.mkString("."), l.asScala) }
 
-    // All using directives should use just `using` keyword, no @using or require
-    reportWarning(
-      "Deprecated using directive syntax, please use keyword `using`.",
-      getDirectives(usedDirectives).filter(_.getSyntax() != UsingDirectiveSyntax.Using),
-      before = false
-    )
-
-    val flattened = usedDirectives.getFlattenedMap.asScala.toSeq
-    val strictDirectives =
-      flattened.map { case (k, l) => StrictDirective(k.getPath.asScala.mkString("."), l.asScala) }
-
-    val offset =
-      if (usedDirectives.getKind() != UsingDirectiveKind.Code) 0
-      else usedDirectives.getCodeOffset()
-    Right(ExtractedDirectives(offset, strictDirectives))
+      val offset =
+        if (usedDirectives.getKind() != UsingDirectiveKind.Code) 0
+        else usedDirectives.getCodeOffset()
+      Right(ExtractedDirectives(offset, strictDirectives))
+    }
+    else {
+      val errors0 = errors.map(diag => new MalformedDirectiveError(diag.message, diag.positions))
+      Left(CompositeBuildException(errors0))
+    }
   }
 
   private def parseDependency(str: String, pos: Position): Either[BuildException, AnyDependency] =

modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala

@@ -334,14 +334,6 @@ class BuildTests extends munit.FunSuite {
           |pprint.log(g)
           |""".stripMargin,
       os.rel / "simple2.sc" ->
-        """//> using
-          |//  lib "com.lihaoyi::geny:0.6.5"
-          |//  lib "com.lihaoyi::pprint:0.6.6"
-          |import geny.Generator
-          |val g = Generator("Hel", "lo")
-          |pprint.log(g)
-          |""".stripMargin,
-      os.rel / "simple3.sc" ->
         """//> using lib "com.lihaoyi::geny:0.6.5", "com.lihaoyi::pprint:0.6.6"
           |import geny.Generator
           |val g = Generator("Hel", "lo")
@@ -357,11 +349,7 @@ class BuildTests extends munit.FunSuite {
         "simple2.class",
         "simple2_sc.class",
         "simple2$.class",
-        "simple2_sc$.class",
-        "simple3.class",
-        "simple3_sc.class",
-        "simple3$.class",
-        "simple3_sc$.class"
+        "simple2_sc$.class"
       )
       maybeBuild.orThrow.assertNoDiagnostics
     }
@@ -595,17 +583,6 @@ class BuildTests extends munit.FunSuite {
           |for {
           |  (x, y) <- getCounts
           |} yield x + y
-          |""".stripMargin,
-      os.rel / "p2.sc" ->
-        """//> using
-          |//  scala "2.13"
-          |//  plugins "com.olegpy::better-monadic-for:0.3.1"
-          |
-          |def getCounts: Either[String, (Int, Int)] = ???
-          |
-          |for {
-          |  (x, y) <- getCounts
-          |} yield x + y
           |""".stripMargin
     )
     val buildOptions = defaultOptions.copy(