Merge pull request #53 from lucidsoftware/zinc-logging-issues

Make error reporting match that of SBT's

rules/private/phases/phase_zinc_compile.bzl

@@ -37,15 +37,14 @@ def phase_zinc_compile(ctx, g):
     ]
 
     zincs = [dep[_ZincInfo] for dep in ctx.attr.deps if _ZincInfo in dep]
-    common_scalacopts = ctx.attr.scalacopts + g.semanticdb.scalacopts
+    common_scalacopts = scala_configuration.global_scalacopts + ctx.attr.scalacopts + g.semanticdb.scalacopts
 
     args = ctx.actions.args()
 
     args.add_all(depset(transitive = [zinc.deps for zinc in zincs]), map_each = _compile_analysis)
     args.add("--compiler_bridge", zinc_configuration.compiler_bridge)
     args.add_all("--compiler_classpath", g.classpaths.compiler)
     args.add_all("--classpath", g.classpaths.compile)
-    args.add_all(scala_configuration.global_scalacopts, format_each = "--compiler_option=%s")
     args.add_all(common_scalacopts, format_each = "--compiler_option=%s")
     args.add_all(javacopts, format_each = "--java_compiler_option=%s")
     args.add(ctx.label, format = "--label=%s")

rules/scala/private/provider.bzl

@@ -59,8 +59,8 @@ def configure_zinc_scala_implementation(ctx):
         ),
         _ScalaRulePhase(
             phases = [
-                ("+", "classpaths", "semanticdb", _phase_semanticdb),
                 ("=", "compile", "compile", _phase_zinc_compile),
+                ("-", "compile", "semanticdb", _phase_semanticdb),
                 ("+", "compile", "depscheck", _phase_zinc_depscheck),
             ],
         ),

rules/scala/workspace.bzl

@@ -12,7 +12,7 @@ filegroup(
 
 scala_2_13_version = "2.13.14"
 scala_3_version = "3.4.2"
-zinc_version = "1.10.0"
+zinc_version = "1.10.1"
 
 def scala_artifacts():
     return [
@@ -24,7 +24,7 @@ def scala_artifacts():
         "org.jacoco:org.jacoco.core:0.7.5.201505241946",
         "org.scala-sbt:test-interface:1.0",
         "org.scala-sbt:compiler-interface:{}".format(zinc_version),
-        "org.scala-sbt:io_2.13:{}".format(zinc_version),
+        "org.scala-sbt:io_2.13:1.10.0",
         "org.scala-sbt:util-interface:{}".format(zinc_version),
         "org.scala-sbt:util-logging_2.13:{}".format(zinc_version),
         "org.scala-sbt:zinc_2.13:{}".format(zinc_version),

src/main/scala/higherkindness/rules_scala/workers/common/Color.scala

@@ -5,26 +5,10 @@ import xsbti.Problem
 import Console.{GREEN => CG, RED => CR, RESET => CRESET, YELLOW => CY}
 
 object Color {
-  def Info(problem: Problem): String = colorProblem(problem, CG)
-
   def Info(message: String): String = colorString(message, CG, "Info")
-
-  def Warning(problem: Problem): String = colorProblem(problem, CY)
-
   def Warning(message: String): String = colorString(message, CY, "Warn")
-
-  def Error(problem: Problem): String = colorProblem(problem, CR)
-
   def Error(message: String): String = colorString(message, CR, "Error")
 
-  private def colorProblem(problem: Problem, color: String): String = {
-    val header = s"[$color${problem.severity}$CRESET]"
-    problem.position.toString.size match {
-      case 0 => s"$header " + problem.message.replaceAll("\n", s"\n$header ")
-      case _ => s"$header ${problem.position} " + problem.message.replaceAll("\n", s"\n$header ")
-    }
-  }
-
   private def colorString(message: String, color: String, severity: String): String = {
     val header = s"[$color$severity$CRESET]"
     s"$header " + message.replaceAll("\n", s"\n$header ")

src/main/scala/higherkindness/rules_scala/workers/common/CommonArguments.scala

@@ -49,6 +49,19 @@ object Analysis {
 }
 
 object CommonArguments {
+  private def adjustCompilerOptions(workDir: Path, options: List[String]) = {
+    def adjustStringPath(path: String) = SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString
+
+    options.flatMap {
+      case s"-P:semanticdb:targetroot:$path" =>
+        List(s"-P:semanticdb:sourceroot:${workDir.toString}", s"-P:semanticdb:targetroot:${adjustStringPath(path)}")
+
+      case s"-semanticdb-target:$path" =>
+        List(s"-semanticdb-target:${adjustStringPath(path)}", s"-sourceroot:${workDir.toString}")
+
+      case option => List(option)
+    }
+  }
 
   /**
    * Adds argument parsers for CommonArguments to the given ArgumentParser and then returns the mutated ArgumentParser.
@@ -178,8 +191,10 @@ object CommonArguments {
       analyses = analyses,
       compilerBridge = SandboxUtil.getSandboxPath(workDir, namespace.get[Path]("compiler_bridge")),
       compilerClasspath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("compiler_classpath")),
-      compilerOptions =
+      compilerOptions = adjustCompilerOptions(
+        workDir,
         Option(namespace.getList[String]("compiler_option")).map(_.asScala.toList).getOrElse(List.empty),
+      ),
       classpath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("classpath")),
       debug = namespace.getBoolean("debug"),
       javaCompilerOptions = namespace.getList[String]("java_compiler_option").asScala.toList,

src/main/scala/higherkindness/rules_scala/workers/common/LoggedReporter.scala

@@ -2,18 +2,18 @@ package higherkindness.rules_scala
 package workers.common
 
 import xsbti.{Logger, Problem}
-import sbt.internal.inc.{LoggedReporter => SbtLoggedReporter}
+import sbt.internal.inc.{LoggedReporter => SbtLoggedReporter, ProblemStringFormats}
 
 class LoggedReporter(logger: Logger, versionString: String) extends SbtLoggedReporter(0, logger) {
+  private val problemStringFormats = new ProblemStringFormats {}
+
   // Scala 3 has great error messages, let's leave those alone, but still add color to the Scala 2 messages
-  val shouldEnhanceMessage = if (versionString.startsWith("0.") || versionString.startsWith("3")) false else true
+  private val shouldEnhanceMessage = !versionString.startsWith("0.") && !versionString.startsWith("3")
+
+  private def doLog(problem: Problem, colorFunction: String => String): String = {
+    val formattedProblem = problemStringFormats.ProblemStringFormat.showLines(problem).mkString("\n")
 
-  def doLog(problem: Problem, colorFunc: Problem => String): String = {
-    if (shouldEnhanceMessage) {
-      colorFunc(problem)
-    } else {
-      problem.rendered().orElse(problem.toString)
-    }
+    if (shouldEnhanceMessage) colorFunction(formattedProblem) else formattedProblem
   }
 
   override protected def logError(problem: Problem): Unit = {

src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala

@@ -52,14 +52,12 @@ object ZincRunnerWorkerConfig {
   }
 }
 
-// The list in this docstring gets clobbered by the formatter, unfortunately.
-//format: off
 /**
  * <strong>Caching</strong>
  *
  * Zinc has two caches:
  *   1. a ClassLoaderCache which is a soft reference cache for classloaders of Scala compilers.
- *   2. a CompilerCache which is a hard reference cache for (I think) Scala compiler instances.
+ *   1. a CompilerCache which is a hard reference cache for (I think) Scala compiler instances.
  *
  * The CompilerCache has reproducibility issues, so it needs to be a no-op. The ClassLoaderCache needs to be reused else
  * JIT reuse (i.e. the point of the worker strategy) doesn't happen.
@@ -75,7 +73,6 @@ object ZincRunnerWorkerConfig {
  * that Zinc caches. We do so to prevent non-determinism in Zinc's analysis store files. Check the comments in
  * AnnexScalaInstance for more info.
  */
- //format: on
 object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {
 
   private[this] val classloaderCache = new ClassLoaderCache(new URLClassLoader(Array()))

tests/compile/logger/Example.scala

@@ -12,9 +12,9 @@ object Example {
     def main(args: Array[String]) {
         val logger = new AnnexLogger(LogLevel.Info, Paths.get(""), System.err)
         val reporter = new LoggedReporter(logger, "2.13.14")
-        val problem1 = problem("", new JavaPosition("Test Line", 100, "", 100), "Info Message 1", Severity.Info)
-        val problem2 = problem("", new JavaPosition("Test Line", 200, "", 200), "Warning Message 2", Severity.Warn)
-        val problem3 = problem("", new JavaPosition("Test Line", 300, "", 300), "Error Message 3", Severity.Error)
+        val problem1 = problem("", new JavaPosition("Test Line", 100, "", 99, ""), "Info Message 1", Severity.Info)
+        val problem2 = problem("", new JavaPosition("Test Line", 200, "", 199, ""), "Warning Message 2", Severity.Warn)
+        val problem3 = problem("", new JavaPosition("Test Line", 300, "", 299, ""), "Error Message 3", Severity.Error)
         // Test logger
         reporter.log(problem1)
         reporter.log(problem2)

tests/compile/logger/test

@@ -2,9 +2,9 @@
 . "$(dirname "$0")"/../../common.sh
 
 bazel run :lib > output 2>&1
-cat output | grep -q $'\[\e\[32mInfo\e\[0m\] Test Line:100:100 Info Message 1'
-cat output | grep -q $'\[\e\[33mWarn\e\[0m\] Test Line:200:200 Warning Message 2'
-cat output | grep -q $'\[\e\[31mError\e\[0m\] Test Line:300:300 Error Message 3'
+cat output | grep -q $'\[\e\[32mInfo\e\[0m\] Test Line:100:100: Info Message 1'
+cat output | grep -q $'\[\e\[33mWarn\e\[0m\] Test Line:200:200: Warning Message 2'
+cat output | grep -q $'\[\e\[31mError\e\[0m\] Test Line:300:300: Error Message 3'
 cat output | grep -q $'\[\e\[32mInfo\e\[0m\] This is an info'
 cat output | grep -q $'\[\e\[33mWarn\e\[0m\] This is a warning'
 cat output | grep -q $'\[\e\[31mError\e\[0m\] This is an error'

tests/zinc/BUILD

@@ -0,0 +1,22 @@
+load("@rules_scala_annex//rules:scala.bzl", "scala_library")
+
+scala_library(
+    name = "error_reporting_2_13",
+    srcs = ["ErrorReporting.scala"],
+    scala = "//scala:2_13",
+    tags = ["manual"],
+)
+
+scala_library(
+    name = "error_reporting_bootstrap_3",
+    srcs = ["ErrorReporting.scala"],
+    scala = "//scala:bootstrap_3",
+    tags = ["manual"],
+)
+
+scala_library(
+    name = "error_reporting_zinc_3",
+    srcs = ["ErrorReporting.scala"],
+    scala = "//scala:zinc_3",
+    tags = ["manual"],
+)

tests/zinc/ErrorReporting.scala

@@ -0,0 +1,5 @@
+object Main {
+  private def printZooAnimals(animals: List[String]): Unit = println(s"The zoo has ${animals.mkString(",")}")
+
+  def main(arguments: Array[String]): Unit = printZooAnimals(Seq("kangaroos", "giraffes"))
+}

tests/zinc/test

@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+
+"""
+This test is written in Python because Bash makes it unnecessarily difficult to check that the output of a command
+includes a multiline string.
+"""
+
+import os
+import subprocess
+
+os.chdir(os.path.dirname(os.path.realpath(__file__)))
+
+def output_from_failed_build(target: str) -> str:
+    result = subprocess.run(["bazel", "build", target], stderr=subprocess.PIPE, text=True)
+
+    assert result.returncode != 0
+
+    return result.stderr
+
+def string_must_contain(string: str, expected_substring: str):
+    if expected_substring not in string:
+        print(f'Expected """{string}""" to contain """{expected_substring}"""')
+
+        assert False
+
+string_must_contain(
+    output_from_failed_build(":error_reporting_2_13"),
+    """\
+[\x1b[31mError\x1b[0m] zinc/ErrorReporting.scala:4:65: type mismatch;
+[\x1b[31mError\x1b[0m]  found   : Seq[String]
+[\x1b[31mError\x1b[0m]  required: List[String]
+[\x1b[31mError\x1b[0m]   def main(arguments: Array[String]): Unit = printZooAnimals(Seq("kangaroos", "giraffes"))
+[\x1b[31mError\x1b[0m]                                                                 ^
+one error found
+"""
+)
+
+string_must_contain(
+    output_from_failed_build(":error_reporting_bootstrap_3"),
+    """\
+\x1b[31m\x1b[31m-- [E007] Type Mismatch Error: zinc/ErrorReporting.scala:4:64 ------------------\x1b[0m\x1b[0m
+\x1b[31m4 |\x1b[0m  \x1b[33mdef\x1b[0m \x1b[36mmain\x1b[0m(\x1b[36marguments\x1b[0m: \x1b[35mArray\x1b[0m[\x1b[35mString\x1b[0m]): \x1b[35mUnit\x1b[0m = printZooAnimals(Seq(\x1b[31m"kangaroos"\x1b[0m, \x1b[31m"giraffes"\x1b[0m))
+\x1b[31m\x1b[31m  |\x1b[0m                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^\x1b[0m
+\x1b[31m  |\x1b[0m                                                    Found:    \x1b[1m\x1b[31mSeq\x1b[0m[String]
+\x1b[31m  |\x1b[0m                                                    Required: \x1b[1m\x1b[32mList\x1b[0m[String]
+\x1b[31m  |\x1b[0m
+\x1b[31m  |\x1b[0m longer explanation available when compiling with `-explain`
+1 error found
+"""
+)
+
+string_must_contain(
+    output_from_failed_build(":error_reporting_zinc_3"),
+    """\
+\x1b[31m\x1b[31m-- [E007] Type Mismatch Error: zinc/ErrorReporting.scala:4:64 \x1b[0m\x1b[0m
+\x1b[31m4 |\x1b[0m  \x1b[33mdef\x1b[0m \x1b[36mmain\x1b[0m(\x1b[36marguments\x1b[0m: \x1b[35mArray\x1b[0m[\x1b[35mString\x1b[0m]): \x1b[35mUnit\x1b[0m = printZooAnimals(Seq(\x1b[31m"kangaroos"\x1b[0m, \x1b[31m"giraffes"\x1b[0m))
+\x1b[31m\x1b[31m  |\x1b[0m                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^\x1b[0m
+\x1b[31m  |\x1b[0m                                                    Found:    \x1b[1m\x1b[31mSeq\x1b[0m[String]
+\x1b[31m  |\x1b[0m                                                    Required: \x1b[1m\x1b[32mList\x1b[0m[String]
+\x1b[31m  |\x1b[0m
+\x1b[31m  |\x1b[0m longer explanation available when compiling with `-explain`
+one error found
+"""
+)