Merge pull request #7492 from adpi2/sbt2-todo-errorspec

Fix test in ErrorSpec

buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala

@@ -14,12 +14,10 @@ import java.io.File
 import java.nio.charset.StandardCharsets
 import java.util.concurrent.ConcurrentHashMap
 import sbt.internal.parser.SbtParser._
-import scala.compat.Platform.EOL
 import dotty.tools.dotc.ast.Trees.Lazy
-import dotty.tools.dotc.ast.untpd
+import dotty.tools.dotc.ast.untpd.*
-import dotty.tools.dotc.ast.untpd.Tree
 import dotty.tools.dotc.CompilationUnit
-import dotty.tools.dotc.core.Contexts.Context
+import dotty.tools.dotc.core.Contexts.*
 import dotty.tools.dotc.Driver
 import dotty.tools.dotc.util.NoSourcePosition
 import dotty.tools.dotc.util.SourceFile
@@ -41,7 +39,7 @@ private[sbt] object SbtParser:
   val END_OF_LINE_CHAR = '\n'
   val END_OF_LINE = String.valueOf(END_OF_LINE_CHAR)
   private[parser] val NOT_FOUND_INDEX = -1
-  private[sbt] val FAKE_FILE = VirtualFileRef.of("fake") // new File("fake")
+  private[sbt] val FAKE_FILE = VirtualFileRef.of("fake")
   private[parser] val XML_ERROR = "';' expected but 'val' found."
 
   private val XmlErrorMessage =
@@ -85,12 +83,12 @@ private[sbt] object SbtParser:
 
     override def doReport(dia: Diagnostic)(using Context): Unit =
       import scala.jdk.OptionConverters.*
-      val sourcePath = dia.position.asScala.getOrElse(sys.error("missing position")).source.path
+      val sourcePath = dia.position.toScala.getOrElse(sys.error("missing position")).source.path
       val reporter = getReporter(sourcePath)
       reporter.doReport(dia)
     override def report(dia: Diagnostic)(using Context): Unit =
       import scala.jdk.OptionConverters.*
-      val sourcePath = dia.position.asScala.getOrElse(sys.error("missing position")).source.path
+      val sourcePath = dia.position.toScala.getOrElse(sys.error("missing position")).source.path
       val reporter = getReporter(sourcePath)
       reporter.report(dia)
 
@@ -127,7 +125,7 @@ private[sbt] object SbtParser:
         val seq = reporter.pendingMessages.map { info =>
           s"""[$fileName]:${info.pos.line}: ${info.msg}"""
         }
-        val errorMessage = seq.mkString(EOL)
+        val errorMessage = seq.mkString(System.lineSeparator)
         val error: String =
           if (errorMessage.contains(XML_ERROR))
             s"$errorMessage\n${SbtParser.XmlErrorMessage}"
@@ -141,62 +139,42 @@ private[sbt] object SbtParser:
   private[sbt] val globalReporter = UniqueParserReporter()
   private[sbt] val defaultGlobalForParser = ParseDriver()
   private[sbt] final class ParseDriver extends Driver:
-    import dotty.tools.dotc.config.Settings.Setting._
+    override protected val sourcesRequired: Boolean = false
     val compileCtx0 = initCtx.fresh
-    val options = List("-classpath", s"$defaultClasspath", "dummy.scala")
+    val options = List("-classpath", s"$defaultClasspath")
     val compileCtx1 = setup(options.toArray, compileCtx0) match
       case Some((_, ctx)) => ctx
       case _              => sys.error(s"initialization failed for $options")
-    val outputDir = VirtualDirectory("output")
+    val compileCtx: Context = compileCtx1.fresh
-    val compileCtx2 = compileCtx1.fresh
+      .setSetting(compileCtx1.settings.outputDir, VirtualDirectory("output"))
-      .setSetting(
-        compileCtx1.settings.outputDir,
-        outputDir
-      )
       .setReporter(globalReporter)
-    val compileCtx = compileCtx2
     val compiler = newCompiler(using compileCtx)
   end ParseDriver
 
   /**
    * Parse code reusing the same [[Run]] instance.
    *
-   * @param code The code to be parsed.
    * @param filePath The file name where the code comes from.
-   * @param reporterId0 The reporter id is the key used to get the pertinent
+   * @param reporterId The reporter id is the key used to get the pertinent
    *                    reporter. Given that the parsing reuses a global
    *                    instance, this reporter id makes sure that every parsing
    *                    session gets its own errors in a concurrent setting.
    *                    The reporter id must be unique per parsing session.
-   * @return
+   * @return the parsed trees
    */
-  private[sbt] def parse(
+  private def parse(filePath: String, reporterId: String)(using Context): List[Tree] =
-      code: String,
-      filePath: String,
-      reporterId0: Option[String]
-  ): (List[untpd.Tree], String, SourceFile) =
     import defaultGlobalForParser.*
-    given ctx: Context = compileCtx
-    val reporterId = reporterId0.getOrElse(s"$filePath-${Random.nextInt}")
     val reporter = globalReporter.getOrCreateReporter(reporterId)
     reporter.removeBufferedMessages
-    val moduleName = "SyntheticModule"
+    val parser = Parsers.Parser(ctx.source)
-    val wrapCode = s"""object $moduleName {
-                      |$code
-                      |}""".stripMargin
-    val wrapperFile = SourceFile(
-      VirtualFile(reporterId, wrapCode.getBytes(StandardCharsets.UTF_8)),
-      scala.io.Codec.UTF8
-    )
-    val parser = Parsers.Parser(wrapperFile)
     val t = parser.parse()
     val parsedTrees = t match
-      case untpd.PackageDef(_, List(untpd.ModuleDef(_, untpd.Template(_, _, _, trees)))) =>
+      case PackageDef(_, List(ModuleDef(_, Template(_, _, _, trees)))) =>
         trees match
-          case ts: List[untpd.Tree]       => ts
+          case ts: List[Tree]       => ts
-          case ts: Lazy[List[untpd.Tree]] => ts.complete
+          case ts: Lazy[List[Tree]] => ts.complete
     globalReporter.throwParserErrorsIfAny(reporter, filePath)
-    (parsedTrees, reporterId, wrapperFile)
+    parsedTrees
 end SbtParser
 
 private class SbtParserInit {
@@ -257,24 +235,36 @@ private[sbt] case class SbtParser(path: VirtualFileRef, lines: Seq[String])
       lines: Seq[String]
   ): (Seq[(String, Int)], Seq[(String, LineRange)], Seq[(String, Tree)]) = {
     // import sbt.internal.parser.MissingBracketHandler.findMissingText
-    val indexedLines = lines.toIndexedSeq
+    val code = lines.toIndexedSeq.mkString(END_OF_LINE)
-    val content = indexedLines.mkString(END_OF_LINE)
+    val wrapCode = s"""object SyntheticModule {
+                      |$code
+                      |}""".stripMargin
     val fileName = path.id
-    val (parsedTrees, reporterId, sourceFile) = parse(content, fileName, None)
+    val reporterId = s"$fileName-${Random.nextInt}"
-    given ctx: Context = compileCtx
+    val sourceFile = SourceFile(
+      VirtualFile(reporterId, wrapCode.getBytes(StandardCharsets.UTF_8)),
+      scala.io.Codec.UTF8
+    )
+    given Context = compileCtx.fresh.setSource(sourceFile)
+    val parsedTrees = parse(fileName, reporterId)
 
-    val (imports: Seq[untpd.Tree], statements: Seq[untpd.Tree]) =
+    // Check No val (a,b) = foo *or* val a,b = foo as these are problematic to range positions and the WHOLE architecture.
+    parsedTrees.withFilter(_.isInstanceOf[PatDef]).foreach { badTree =>
+      throw new MessageOnlyException(
+        s"[${fileName}]:${badTree.line}: Pattern matching in val statements is not supported"
+      )
+    }
+
+    val (imports: Seq[Tree], statements: Seq[Tree]) =
       parsedTrees.partition {
-        case _: untpd.Import => true
+        case _: Import => true
-        case _               => false
+        case _         => false
       }
 
-    def convertStatement(tree: untpd.Tree)(using ctx: Context): Option[(String, Tree, LineRange)] =
+    def convertStatement(tree: Tree)(using Context): Option[(String, Tree, LineRange)] =
       if tree.span.exists then
-        // not sure why I need to reconstruct the position myself
+        val statement = String(tree.sourcePos.linesSlice).trim
-        val pos = SourcePosition(sourceFile, tree.span)
+        val lines = tree.sourcePos.lines
-        val statement = String(pos.linesSlice).trim()
-        val lines = pos.lines
         val wrapperLineOffset = 0
         Some(
           (

buildfile/src/test/scala/sbt/internal/parser/CheckIfParsedSpec.scala

@@ -16,18 +16,18 @@ abstract class CheckIfParsedSpec(
   test(s"${this.getClass.getName} should parse sbt file") {
     files foreach { case (content, description, nonEmptyImports, nonEmptyStatements) =>
       println(s"""${getClass.getSimpleName}: "$description" """)
-      val (imports, statements) = split(content)(splitter)
+      val (imports, statements) = splitter(content)
       assert(
         nonEmptyStatements == statements.nonEmpty,
-        s"""$description
+        s"""|$description
-             |***${shouldContains(nonEmptyStatements)} statements***
+            |***${shouldContains(nonEmptyStatements)} statements***
-             |$content """.stripMargin
+            |$content """.stripMargin
       )
       assert(
         nonEmptyImports == imports.nonEmpty,
-        s"""$description
+        s"""|$description
-             |***${shouldContains(nonEmptyImports)} imports***
+            |***${shouldContains(nonEmptyImports)} imports***
-             |$content """.stripMargin
+            |$content """.stripMargin
       )
     }
   }

buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala

@@ -1,96 +0,0 @@
-/*
- * sbt
- * Copyright 2011 - 2018, Lightbend, Inc.
- * Copyright 2008 - 2010, Mark Harrah
- * Licensed under Apache License 2.0 (see LICENSE)
- */
-
-package sbt
-package internal
-package parser
-
-import java.io.File
-import sbt.internal.util.MessageOnlyException
-import scala.io.Source
-import sbt.internal.inc.PlainVirtualFileConverter
-
-object ErrorSpec extends AbstractSpec {
-
-  val converter = PlainVirtualFileConverter.converter
-  // implicit val splitter: SplitExpressions.SplitExpression = EvaluateConfigurations.splitExpressions
-
-  test("Parser should contains file name and line number") {
-    val rootPath = getClass.getResource("/error-format/").getPath
-    println(s"Reading files from: $rootPath")
-    new File(rootPath).listFiles foreach { file =>
-      print(s"Processing ${file.getName}: ")
-
-      val vf = converter.toVirtualFile(file.toPath())
-      val buildSbt = Source.fromFile(file).getLines().mkString("\n")
-      try {
-        SbtParser(vf, buildSbt.linesIterator.toSeq)
-      } catch {
-        case exp: MessageOnlyException =>
-          val message = exp.getMessage
-          println(s"${exp.getMessage}")
-          assert(message.contains(file.getName))
-      }
-      // todo:
-      // containsLineNumber(buildSbt)
-    }
-  }
-
-  // test("it should handle wrong parsing") {
-  //   intercept[MessageOnlyException] {
-  //     val buildSbt =
-  //       """
-  //         |libraryDependencies ++= Seq("a" % "b" % "2") map {
-  //         |(dependency) =>{
-  //         | dependency
-  //         | } /* */ //
-  //         |}
-  //       """.stripMargin
-  //     MissingBracketHandler.findMissingText(
-  //       buildSbt,
-  //       buildSbt.length,
-  //       2,
-  //       "fake.txt",
-  //       new MessageOnlyException("fake")
-  //     )
-  //     ()
-  //   }
-  // }
-
-  test("it should handle xml error") {
-    try {
-      val buildSbt =
-        """
-          |val a = <a/><b/>
-          |val s = '
-        """.stripMargin
-      SbtParser(SbtParser.FAKE_FILE, buildSbt.linesIterator.toSeq)
-      // sys.error("not supposed to reach here")
-    } catch {
-      case exp: MessageOnlyException =>
-        val message = exp.getMessage
-        println(s"${exp.getMessage}")
-        assert(message.contains(SbtParser.FAKE_FILE.id()))
-    }
-  }
-
-  private def containsLineNumber(buildSbt: String) = {
-    try {
-      split(buildSbt)
-      throw new IllegalStateException(s"${classOf[MessageOnlyException].getName} expected")
-    } catch {
-      case exception: MessageOnlyException =>
-        val error = exception.getMessage
-        """(\d+)""".r.findFirstIn(error) match {
-          case Some(_) => true
-          case None =>
-            println(s"Number not found in $error")
-            false
-        }
-    }
-  }
-}

buildfile/src/test/scala/sbt/internal/parser/SbtParserErrorSpec.scala

@@ -0,0 +1,62 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt
+package internal
+package parser
+
+import sbt.internal.util.MessageOnlyException
+import scala.io.Source
+import sbt.internal.inc.PlainVirtualFileConverter
+
+import java.nio.file.Files
+import java.nio.file.Paths
+import verify.sourcecode.SourceLocation
+
+object SbtParserErrorSpec extends AbstractSpec {
+
+  val converter = PlainVirtualFileConverter.converter
+
+  test("errors should contain file name and line number") {
+    val rootPath = Paths.get(getClass.getResource("/error-format/").toURI)
+    println(s"Reading files from: $rootPath")
+
+    Files.list(rootPath).forEach { file =>
+      print(s"Processing ${file.getFileName}: ")
+
+      val vf = converter.toVirtualFile(file)
+      val buildSbt = Source.fromFile(file.toUri).getLines.toSeq
+      val message = interceptMessageException(SbtParser(vf, buildSbt))
+      println(message)
+      assert(message.contains(file.getFileName.toString))
+      containsLineNumber(message)
+    }
+  }
+
+  test("xml error") {
+    val buildSbt =
+      """
+          |val a = <a/><b/>
+          |val s = '
+        """.stripMargin
+    val message =
+      interceptMessageException(SbtParser(SbtParser.FAKE_FILE, buildSbt.linesIterator.toSeq))
+    println(message)
+    assert(message.contains(SbtParser.FAKE_FILE.id))
+  }
+
+  private def containsLineNumber(message: String) =
+    """\d+""".r.findFirstIn(message).getOrElse(fail(s"Line number not found in $message"))
+
+  private def interceptMessageException(callback: => Unit)(using
+      pos: SourceLocation
+  ): String =
+    try
+      callback
+      throw new AssertionError(s"$pos: expected a MessageOnlyException to be thrown")
+    catch case ex: MessageOnlyException => ex.getMessage
+}

buildfile/src/test/scala/sbt/internal/parser/SplitExpressionsBehavior.scala

@@ -15,9 +15,9 @@ import sbt.internal.util.LineRange
 import xsbti.VirtualFileRef
 
 trait SplitExpression {
-  def split(s: String, file: VirtualFileRef = VirtualFileRef.of("noFile"))(
+  extension (splitter: SplitExpressions.SplitExpression)
-      splitter: SplitExpressions.SplitExpression
+    def apply(s: String): (Seq[(String, Int)], Seq[(String, LineRange)]) =
-  ): (Seq[(String, Int)], Seq[(String, LineRange)]) = splitter(file, s.split("\n").toSeq)
+      splitter(VirtualFileRef.of("noFile"), s.split('\n').toSeq)
 }
 
 trait SplitExpressionsBehavior extends SplitExpression { this: verify.BasicTestSuite =>
@@ -25,29 +25,32 @@ trait SplitExpressionsBehavior extends SplitExpression { this: verify.BasicTestS
   def newExpressionsSplitter(splitter: SplitExpressions.SplitExpression) = {
 
     test("parse a two settings without intervening blank line") {
-      val (imports, settings) = split("""version := "1.0"
+      val (imports, settings) = splitter(
-scalaVersion := "2.10.4"""")(splitter)
+        """|version := "1.0"
+           |scalaVersion := "2.10.4"""".stripMargin
+      )
 
       assert(imports.isEmpty)
       assert(settings.size == 2)
     }
 
     test("parse a setting and val without intervening blank line") {
-      val (imports, settings) =
+      val (imports, settings) = splitter(
-        split("""version := "1.0"
+        """|version := "1.0"
-lazy val root = (project in file(".")).enablePlugins­(PlayScala)""")(splitter)
+           |lazy val root = (project in file(".")).enablePlugins(PlayScala)""".stripMargin
+      )
 
       assert(imports.isEmpty)
       assert(settings.size == 2)
     }
 
     test("parse a config containing two imports and a setting with no blank line") {
-      val (imports, settingsAndDefs) = split(
+      val (imports, settingsAndDefs) = splitter(
-        """import foo.Bar
+        """|import foo.Bar
-              import foo.Bar
+           |import foo.Bar
-             version := "1.0"
+           |version := "1.0"
-        """.stripMargin
+           |""".stripMargin
-      )(splitter)
+      )
       assert(imports.size == 2)
       assert(settingsAndDefs.size == 1)
     }