From 5990775a433b9586da29db972b52000c3481301e Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Thu, 8 Feb 2024 13:12:35 +0100 Subject: [PATCH 1/4] Fix test in ErrorSpec --- .../scala/sbt/internal/parser/SbtParser.scala | 92 +++++++++---------- .../internal/parser/CheckIfParsedSpec.scala | 14 +-- .../scala/sbt/internal/parser/ErrorSpec.scala | 59 ++++++------ .../parser/SplitExpressionsBehavior.scala | 31 ++++--- 4 files changed, 91 insertions(+), 105 deletions(-) diff --git a/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala b/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala index 13b09421a..d9c0cb904 100644 --- a/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala +++ b/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.Tree +import dotty.tools.dotc.ast.untpd.* 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 @@ -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 compileCtx2 = compileCtx1.fresh - .setSetting( - compileCtx1.settings.outputDir, - outputDir - ) + val compileCtx: Context = compileCtx1.fresh + .setSetting(compileCtx1.settings.outputDir, VirtualDirectory("output")) .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( - code: String, - filePath: String, - reporterId0: Option[String] - ): (List[untpd.Tree], String, SourceFile) = + private def parse(filePath: String, reporterId: String)(using Context): List[Tree] = 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 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 parser = Parsers.Parser(ctx.source) 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: Lazy[List[untpd.Tree]] => ts.complete + case ts: List[Tree] => ts + 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 content = indexedLines.mkString(END_OF_LINE) + val code = lines.toIndexedSeq.mkString(END_OF_LINE) + val wrapCode = s"""object SyntheticModule { + |$code + |}""".stripMargin val fileName = path.id - val (parsedTrees, reporterId, sourceFile) = parse(content, fileName, None) - given ctx: Context = compileCtx + val reporterId = s"$fileName-${Random.nextInt}" + 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 _ => false + case _: Import => true + 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 pos = SourcePosition(sourceFile, tree.span) - val statement = String(pos.linesSlice).trim() - val lines = pos.lines + val statement = String(tree.sourcePos.linesSlice).trim + val lines = tree.sourcePos.lines val wrapperLineOffset = 0 Some( ( diff --git a/buildfile/src/test/scala/sbt/internal/parser/CheckIfParsedSpec.scala b/buildfile/src/test/scala/sbt/internal/parser/CheckIfParsedSpec.scala index 1561b5beb..f1f9f3d25 100644 --- a/buildfile/src/test/scala/sbt/internal/parser/CheckIfParsedSpec.scala +++ b/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 - |***${shouldContains(nonEmptyStatements)} statements*** - |$content """.stripMargin + s"""|$description + |***${shouldContains(nonEmptyStatements)} statements*** + |$content """.stripMargin ) assert( nonEmptyImports == imports.nonEmpty, - s"""$description - |***${shouldContains(nonEmptyImports)} imports*** - |$content """.stripMargin + s"""|$description + |***${shouldContains(nonEmptyImports)} imports*** + |$content """.stripMargin ) } } diff --git a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala b/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala index 9659d437e..6452e7243 100644 --- a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala +++ b/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala @@ -9,34 +9,32 @@ package sbt package internal package parser -import java.io.File 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 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 + test("errors should contain file name and line number") { + val rootPath = Paths.get(getClass.getResource("/error-format/").toURI) 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) + Files.list(rootPath).forEach { file => + print(s"Processing ${file.getFileName}: ") + + val vf = converter.toVirtualFile(file) + val buildSbt = Source.fromFile(file.toUri).getLines.mkString("\n") + val message = + interceptMessageException(SbtParser(vf, buildSbt.linesIterator.toSeq)).getMessage + println(message) + assert(message.contains(file.getFileName.toString)) + containsLineNumber(message) } } @@ -78,19 +76,14 @@ object ErrorSpec extends AbstractSpec { } } - 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 - } - } - } + 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 + ): MessageOnlyException = + try + callback + throw new AssertionError(s"$pos: expected a MessageOnlyException to be thrown") + catch case ex: MessageOnlyException => ex } diff --git a/buildfile/src/test/scala/sbt/internal/parser/SplitExpressionsBehavior.scala b/buildfile/src/test/scala/sbt/internal/parser/SplitExpressionsBehavior.scala index 69ce3b89c..df1b53b7d 100644 --- a/buildfile/src/test/scala/sbt/internal/parser/SplitExpressionsBehavior.scala +++ b/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"))( - splitter: SplitExpressions.SplitExpression - ): (Seq[(String, Int)], Seq[(String, LineRange)]) = splitter(file, s.split("\n").toSeq) + extension (splitter: SplitExpressions.SplitExpression) + def apply(s: String): (Seq[(String, Int)], Seq[(String, LineRange)]) = + 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" -scalaVersion := "2.10.4"""")(splitter) + val (imports, settings) = 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) = - split("""version := "1.0" -lazy val root = (project in file(".")).enablePlugins­(PlayScala)""")(splitter) + val (imports, settings) = splitter( + """|version := "1.0" + |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( - """import foo.Bar - import foo.Bar - version := "1.0" - """.stripMargin - )(splitter) + val (imports, settingsAndDefs) = splitter( + """|import foo.Bar + |import foo.Bar + |version := "1.0" + |""".stripMargin + ) assert(imports.size == 2) assert(settingsAndDefs.size == 1) } From 135ab16b5cfd2768e9364af47f1a0add02e235c9 Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Thu, 8 Feb 2024 13:21:04 +0100 Subject: [PATCH 2/4] Remove outdated test in ErrorSpec --- .../scala/sbt/internal/parser/ErrorSpec.scala | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala b/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala index 6452e7243..2e2d1f717 100644 --- a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala +++ b/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala @@ -38,27 +38,6 @@ object ErrorSpec extends AbstractSpec { } } - // 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 = From 245d13575a2b6e6af491e25f4288687ae63d67c4 Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Thu, 8 Feb 2024 13:28:19 +0100 Subject: [PATCH 3/4] Refactor xml test in ErrorSpec --- .../scala/sbt/internal/parser/SbtParser.scala | 2 +- .../scala/sbt/internal/parser/ErrorSpec.scala | 28 ++++++++----------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala b/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala index d9c0cb904..e96ee7844 100644 --- a/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala +++ b/buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala @@ -39,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 = diff --git a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala b/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala index 2e2d1f717..ac94032e1 100644 --- a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala +++ b/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala @@ -29,30 +29,24 @@ object ErrorSpec extends AbstractSpec { print(s"Processing ${file.getFileName}: ") val vf = converter.toVirtualFile(file) - val buildSbt = Source.fromFile(file.toUri).getLines.mkString("\n") - val message = - interceptMessageException(SbtParser(vf, buildSbt.linesIterator.toSeq)).getMessage + 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("it should handle xml error") { - try { - val buildSbt = - """ + test("xml error") { + val buildSbt = + """ |val a = |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())) - } + val message = + interceptMessageException(SbtParser(SbtParser.FAKE_FILE, buildSbt.linesIterator.toSeq)) + println(message) + assert(message.contains(SbtParser.FAKE_FILE.id)) } private def containsLineNumber(message: String) = @@ -60,9 +54,9 @@ object ErrorSpec extends AbstractSpec { private def interceptMessageException(callback: => Unit)(using pos: SourceLocation - ): MessageOnlyException = + ): String = try callback throw new AssertionError(s"$pos: expected a MessageOnlyException to be thrown") - catch case ex: MessageOnlyException => ex + catch case ex: MessageOnlyException => ex.getMessage } From 013194c21786d9d8b05e87138ad9d8e3110038ec Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Thu, 8 Feb 2024 13:29:15 +0100 Subject: [PATCH 4/4] Rename to SbtParserErrorSpec --- .../parser/{ErrorSpec.scala => SbtParserErrorSpec.scala} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename buildfile/src/test/scala/sbt/internal/parser/{ErrorSpec.scala => SbtParserErrorSpec.scala} (97%) diff --git a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala b/buildfile/src/test/scala/sbt/internal/parser/SbtParserErrorSpec.scala similarity index 97% rename from buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala rename to buildfile/src/test/scala/sbt/internal/parser/SbtParserErrorSpec.scala index ac94032e1..827260e8a 100644 --- a/buildfile/src/test/scala/sbt/internal/parser/ErrorSpec.scala +++ b/buildfile/src/test/scala/sbt/internal/parser/SbtParserErrorSpec.scala @@ -17,7 +17,7 @@ import java.nio.file.Files import java.nio.file.Paths import verify.sourcecode.SourceLocation -object ErrorSpec extends AbstractSpec { +object SbtParserErrorSpec extends AbstractSpec { val converter = PlainVirtualFileConverter.converter