Fix line and path separators in test (#23534)

Fixes #23458 

Splitting on NL leaves the CR if present. Just use `linesIterator`.

Introduce a template method for rendering paths. The test reporter
replaces windows convention when run on windows, so that check files are
correct; `FileDiff.matches` currently accounts for comparing lines of
text when run on windows.

Previously, a `TestReporter` with empty console output would contribute
a NL because `"".split("\n")` is not empty. (There is a test reporter
per compilation group.)

Author: mbovel

compiler/src/dotty/tools/dotc/report.scala

@@ -144,8 +144,6 @@ object report:
       case Nil => pos
     recur(pos.sourcePos, tpd.enclosingInlineds)
 
-  private object messageRendering extends MessageRendering
-
   // Should only be called from Run#enrichErrorMessage.
   def enrichErrorMessage(errorMessage: String)(using Context): String =
     if ctx.settings.XnoEnrichErrorMessages.value then errorMessage

compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala

@@ -8,6 +8,7 @@ import java.lang.System.{lineSeparator => EOL}
 
 import core.Contexts.*
 import core.Decorators.*
+import io.AbstractFile
 import printing.Highlighting.{Blue, Red, Yellow}
 import printing.SyntaxHighlighting
 import Diagnostic.*
@@ -158,9 +159,12 @@ trait MessageRendering {
       .mkString(EOL)
   }
 
+  // file.path or munge it to normalize for testing
+  protected def renderPath(file: AbstractFile): String = file.path
+
   /** The source file path, line and column numbers from the given SourcePosition */
   protected def posFileStr(pos: SourcePosition): String =
-    val path = pos.source.file.path
+    val path = renderPath(pos.source.file)
     if pos.exists then s"$path:${pos.line + 1}:${pos.column}" else path
 
   /** The separator between errors containing the source file and error type

compiler/test/dotty/tools/dotc/reporting/TestReporter.scala

@@ -1,23 +1,25 @@
-package dotty.tools
+package dotty
+package tools
 package dotc
 package reporting
 
-import scala.language.unsafeNulls
-import java.io.{BufferedReader, FileInputStream, FileOutputStream, FileReader, PrintStream, PrintWriter, StringReader, StringWriter, File as JFile}
+import java.io.{File as JFile, *}
+import java.nio.file.Files.readAllLines
 import java.text.SimpleDateFormat
 import java.util.Date
-import core.Decorators.*
 
-import scala.collection.mutable
-import scala.jdk.CollectionConverters.*
-import util.SourcePosition
 import core.Contexts.*
-import Diagnostic.*
-import dotty.Properties
+import core.Decorators.*
 import interfaces.Diagnostic.{ERROR, WARNING}
+import io.AbstractFile
+import util.SourcePosition
+import Diagnostic.*
 
-import scala.io.Codec
+import scala.collection.mutable
 import scala.compiletime.uninitialized
+import scala.io.Codec
+import scala.jdk.CollectionConverters.*
+import scala.language.unsafeNulls
 
 class TestReporter protected (outWriter: PrintWriter, logLevel: Int)
 extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with MessageRendering {
@@ -30,13 +32,17 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
   final def messages: Iterator[String] = _messageBuf.iterator
 
   protected final val _consoleBuf = new StringWriter
-  protected final val _consoleReporter = new ConsoleReporter(null, new PrintWriter(_consoleBuf))
+  protected final val _consoleReporter = new ConsoleReporter(null, new PrintWriter(_consoleBuf)):
+    override protected def renderPath(file: AbstractFile): String = TestReporter.renderPath(file)
+
   final def consoleOutput: String = _consoleBuf.toString
 
   private var _skip: Boolean = false
   final def setSkip(): Unit = _skip = true
   final def skipped: Boolean = _skip
 
+  override protected def renderPath(file: AbstractFile): String = TestReporter.renderPath(file)
+
   protected final def inlineInfo(pos: SourcePosition)(using Context): String =
     if (pos.exists) {
       if (pos.outer.exists)
@@ -152,10 +158,16 @@ object TestReporter {
       Properties.rerunFailed &&
         failedTestsFile.exists() &&
         failedTestsFile.isFile
-    )(java.nio.file.Files.readAllLines(failedTestsFile.toPath).asScala.toList)
+    )(readAllLines(failedTestsFile.toPath).asScala.toList)
 
   def writeFailedTests(tests: List[String]): Unit =
     initLog()
     tests.foreach(failed => failedTestsWriter.println(failed))
     failedTestsWriter.flush()
+
+  def renderPath(file: AbstractFile): String =
+    if JFile.separatorChar == '\\' then
+      file.path.replace('\\', '/')
+    else
+      file.path
 }

compiler/test/dotty/tools/vulpix/ParallelTesting.scala

@@ -730,7 +730,7 @@ trait ParallelTesting extends RunnerOrchestration:
       testSource.checkFile.foreach(diffTest(testSource, _, reporterOutputLines(reporters), reporters, logger))
 
     private def reporterOutputLines(reporters: Seq[TestReporter]): List[String] =
-      reporters.flatMap(_.consoleOutput.split("\n")).toList
+      reporters.flatMap(_.consoleOutput.linesIterator).toList
 
     private[ParallelTesting] def executeTestSuite(): this.type = {
       assert(testSourcesCompleted == 0, "not allowed to re-use a `CompileRun`")
@@ -1043,7 +1043,7 @@ trait ParallelTesting extends RunnerOrchestration:
             testReporter
         }
       if !failedBestEffortCompilation.isEmpty then
-        Some(failedBestEffortCompilation.flatMap(_.consoleOutput.split("\n")).mkString("\n"))
+        Some(failedBestEffortCompilation.flatMap(_.consoleOutput.linesIterator).mkString("\n"))
       else
         None
   }

tests/neg-custom-args/captures/cc-this2.check

@@ -1,4 +1,3 @@
-
 -- Error: tests/neg-custom-args/captures/cc-this2/D_2.scala:3:8 --------------------------------------------------------
 3 |  this: D^ => // error
   |        ^^

tests/neg-macros/annot-crash.check

@@ -1,4 +1,3 @@
-
 -- Error: tests/neg-macros/annot-crash/Test_2.scala:1:0 ----------------------------------------------------------------
 1 |@crash // error
   |^^^^^^

tests/neg-macros/annot-empty-result.check

@@ -1,4 +1,3 @@
-
 -- Error: tests/neg-macros/annot-empty-result/Test_2.scala:5:2 ---------------------------------------------------------
 5 |  @nilAnnot // error
   |  ^^^^^^^^^

tests/neg-macros/annot-error-annot.check

@@ -1,4 +1,3 @@
-
 -- Error: tests/neg-macros/annot-error-annot/Test_2.scala:17:6 ---------------------------------------------------------
 16 |@error
 17 |class cGlobal // error

tests/neg-macros/annot-ill-abort.check

@@ -1,4 +1,3 @@
-
 -- Error: tests/neg-macros/annot-ill-abort/Test_2.scala:1:0 ------------------------------------------------------------
 1 |@crash // error
   |^^^^^^

tests/neg-macros/annot-mod-class-add-top-method.check

@@ -1,4 +1,3 @@
-
 -- Error: tests/neg-macros/annot-mod-class-add-top-method/Test_2.scala:1:0 ---------------------------------------------
 1 |@addTopLevelMethod // error
   |^^^^^^^^^^^^^^^^^^