From eace0e04e50ef91f182ea72321bfe0a4aac65bd1 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 25 Jul 2023 12:38:55 +0200 Subject: [PATCH] Improve positions of using directives, add code offset as extra field --- .../scala/scala/build/bsp/BspClient.scala | 6 +- .../preprocessing/ScalaPreprocessor.scala | 4 +- .../preprocessing/ScriptPreprocessor.scala | 8 +- .../preprocessing/UsingDirectivesOps.scala | 33 +++++++- .../scala/build/tests/DirectiveTests.scala | 4 +- .../build/tests/ScalaPreprocessorTests.scala | 76 ++++++++++++++----- .../src/main/scala/scala/build/Position.scala | 3 +- 7 files changed, 99 insertions(+), 35 deletions(-) diff --git a/modules/build/src/main/scala/scala/build/bsp/BspClient.scala b/modules/build/src/main/scala/scala/build/bsp/BspClient.scala index 59346d8add..1302a774bd 100644 --- a/modules/build/src/main/scala/scala/build/bsp/BspClient.scala +++ b/modules/build/src/main/scala/scala/build/bsp/BspClient.scala @@ -202,8 +202,8 @@ class BspClient( } } .groupBy(_.positions.headOption match - case Some(File(Right(path), _, _)) => Some(path) - case _ => None + case Some(File(Right(path), _, _, _)) => Some(path) + case _ => None ) .filter(_._1.isDefined) .values @@ -222,7 +222,7 @@ class BspClient( reset: Boolean = false )(diag: Diagnostic): Seq[os.Path] = diag.positions.flatMap { - case File(Right(path), (startLine, startC), (endL, endC)) => + case File(Right(path), (startLine, startC), (endL, endC), _) => val id = new b.TextDocumentIdentifier(path.toNIO.toUri.toASCIIString) val startPos = new b.Position(startLine, startC) val endPos = new b.Position(endL, endC) diff --git a/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala b/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala index a46e12c212..3e4ced4fd1 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala @@ -240,7 +240,9 @@ case object ScalaPreprocessor extends Preprocessor { val summedOptions = allOptions.foldLeft(BuildOptions())(_ orElse _) val lastContentOpt = preprocessedDirectives.strippedContent .orElse(if (isSheBang) Some(content0) else None) - val directivesPositions = preprocessedDirectives.directivesPositions + val directivesPositions = preprocessedDirectives.directivesPositions.map { pos => + if (isSheBang) pos.copy(endPos = pos.endPos._1 + 1 -> pos.endPos._2) else pos + } val scopedRequirements = preprocessedDirectives.scopedReqs Some(ProcessingOutput( diff --git a/modules/build/src/main/scala/scala/build/preprocessing/ScriptPreprocessor.scala b/modules/build/src/main/scala/scala/build/preprocessing/ScriptPreprocessor.scala index 3cb7bf3f59..89bddc4e72 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/ScriptPreprocessor.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/ScriptPreprocessor.scala @@ -85,14 +85,11 @@ case object ScriptPreprocessor extends Preprocessor { suppressWarningOptions: SuppressWarningOptions )(using ScalaCliInvokeData): Either[BuildException, List[PreprocessedSource.UnwrappedScript]] = either { - - val (contentIgnoredSheBangLines, _) = SheBang.ignoreSheBangLines(content) - val (pkg, wrapper) = AmmUtil.pathToPackageWrapper(subPath) val processingOutput: ProcessingOutput = value(ScalaPreprocessor.process( - contentIgnoredSheBangLines, + content, reportingPath, scopePath / os.up, logger, @@ -102,7 +99,8 @@ case object ScriptPreprocessor extends Preprocessor { )) .getOrElse(ProcessingOutput.empty) - val scriptCode = processingOutput.updatedContent.getOrElse(contentIgnoredSheBangLines) + val scriptCode = + processingOutput.updatedContent.getOrElse(SheBang.ignoreSheBangLines(content)._1) // try to match in multiline mode, don't match comment lines starting with '//' val containsMainAnnot = "(?m)^(?!//).*@main.*".r.findFirstIn(scriptCode).isDefined diff --git a/modules/build/src/main/scala/scala/build/preprocessing/UsingDirectivesOps.scala b/modules/build/src/main/scala/scala/build/preprocessing/UsingDirectivesOps.scala index 23d7bfb76a..c8d8b0e2de 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/UsingDirectivesOps.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/UsingDirectivesOps.scala @@ -1,8 +1,9 @@ package scala.build.preprocessing import com.virtuslab.using_directives.custom.model.UsingDirectives -import com.virtuslab.using_directives.custom.utils.ast.UsingDefs +import com.virtuslab.using_directives.custom.utils.ast.* +import scala.annotation.tailrec import scala.build.Position import scala.jdk.CollectionConverters.* @@ -12,9 +13,33 @@ object UsingDirectivesOps { def containsTargetDirectives: Boolean = ud.keySet.exists(_.startsWith("target.")) def getPosition(path: Either[String, os.Path]): Position.File = - val line = ud.getAst().getPosition().getLine() - val column = ud.getAst().getPosition().getColumn() - Position.File(path, (0, 0), (line, column)) + extension (pos: Positioned) { + def getLine = pos.getPosition.getLine + def getColumn = pos.getPosition.getColumn + } + + @tailrec + def getEndPostion(ast: UsingTree): (Int, Int) = ast match { + case uds: UsingDefs => uds.getUsingDefs.asScala match { + case _ :+ lastUsingDef => getEndPostion(lastUsingDef) + case _ => (uds.getLine, uds.getColumn) + } + case ud: UsingDef => getEndPostion(ud.getValue) + case uvs: UsingValues => uvs.getValues.asScala match { + case _ :+ lastUsingValue => getEndPostion(lastUsingValue) + case _ => (uvs.getLine, uvs.getColumn) + } + case sl: StringLiteral => ( + sl.getLine, + sl.getColumn + sl.getValue.length + { if sl.getIsWrappedDoubleQuotes then 2 else 0 } + ) + case bl: BooleanLiteral => (bl.getLine, bl.getColumn + bl.getValue.toString.length) + case el: EmptyLiteral => (el.getLine, el.getColumn) + } + + val (line, column) = getEndPostion(ud.getAst) + + Position.File(path, (0, 0), (line, column), ud.getCodeOffset) def getDirectives = ud.getAst match { diff --git a/modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala b/modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala index e6b28e80b6..d69f92ba17 100644 --- a/modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala +++ b/modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala @@ -60,8 +60,8 @@ class DirectiveTests extends munit.FunSuite { assert(position.nonEmpty) val (startPos, endPos) = position.get match { - case Position.File(_, startPos, endPos) => (startPos, endPos) - case _ => sys.error("cannot happen") + case Position.File(_, startPos, endPos, _) => (startPos, endPos) + case _ => sys.error("cannot happen") } expect(startPos == (0, 15)) diff --git a/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala b/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala index 8996117fae..87f5230799 100644 --- a/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala +++ b/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala @@ -2,44 +2,49 @@ package scala.build.tests import com.eed3si9n.expecty.Expecty.expect -import scala.build.input.{ScalaCliInvokeData, SourceScalaFile} +import scala.build.input.{ScalaCliInvokeData, Script, SourceScalaFile} import scala.build.options.SuppressWarningOptions -import scala.build.preprocessing.{PreprocessedSource, ScalaPreprocessor} +import scala.build.preprocessing.{PreprocessedSource, ScalaPreprocessor, ScriptPreprocessor} class ScalaPreprocessorTests extends munit.FunSuite { test("should respect using directives in a .scala file with the shebang line") { + val lastUsingLine = + "//> using dep \"com.lihaoyi::os-lib::0.8.1\" \"com.lihaoyi::os-lib::0.8.1\"" TestInputs(os.rel / "Main.scala" -> - """#!/usr/bin/env -S scala-cli shebang - |//> using dep "com.lihaoyi::os-lib::0.8.1" - | - |object Main { - | def main(args: Array[String]): Unit = { - | println(os.pwd) - | } - |}""".stripMargin).fromRoot { root => + s"""#!/usr/bin/env -S scala-cli shebang + |//> using jvm 11 + |$lastUsingLine + | + |object Main { + | def main(args: Array[String]): Unit = { + | println(os.pwd) + | } + |}""".stripMargin).fromRoot { root => val scalaFile = SourceScalaFile(root, os.sub / "Main.scala") val Some(Right(result)) = ScalaPreprocessor.preprocess( scalaFile, logger = TestLogger(), - allowRestrictedFeatures = false, + allowRestrictedFeatures = true, suppressWarningOptions = SuppressWarningOptions() )(using ScalaCliInvokeData.dummy) expect(result.nonEmpty) val Some(directivesPositions) = result.head.directivesPositions expect(directivesPositions.startPos == 0 -> 0) - expect(directivesPositions.endPos == 3 -> 0) + expect(directivesPositions.endPos == 3 -> lastUsingLine.length) } } test("should respect using directives in a .sc file with the shebang line") { + val depLine = "//> using dep com.lihaoyi::os-lib::0.8.1" + TestInputs(os.rel / "sample.sc" -> - """#!/usr/bin/env -S scala-cli shebang - |//> using dep "com.lihaoyi::os-lib::0.8.1" - |println(os.pwd) - |""".stripMargin).fromRoot { root => - val scalaFile = SourceScalaFile(root, os.sub / "sample.sc") - val Some(Right(result)) = ScalaPreprocessor.preprocess( + s"""#!/usr/bin/env -S scala-cli shebang + |$depLine + |println(os.pwd) + |""".stripMargin).fromRoot { root => + val scalaFile = Script(root, os.sub / "sample.sc", None) + val Some(Right(result)) = ScriptPreprocessor.preprocess( scalaFile, logger = TestLogger(), allowRestrictedFeatures = false, @@ -48,7 +53,40 @@ class ScalaPreprocessorTests extends munit.FunSuite { expect(result.nonEmpty) val Some(directivesPositions) = result.head.directivesPositions expect(directivesPositions.startPos == 0 -> 0) - expect(directivesPositions.endPos == 2 -> 0) + expect(directivesPositions.endPos == 2 -> depLine.length) } } + + val lastUsingLines = Seq( + "//> using dep \"com.lihaoyi::os-lib::0.8.1\" \"com.lihaoyi::os-lib::0.8.1\"" -> "string literal", + "//> using scala 2.13.7" -> "numerical string", + "//> using objectWrapper true" -> "boolean literal", + "//> using objectWrapper" -> "empty value literal" + ) + + for ((lastUsingLine, typeName) <- lastUsingLines) do + test(s"correct directive positions with $typeName") { + TestInputs(os.rel / "Main.scala" -> + s"""#!/usr/bin/env -S scala-cli shebang + |//> using jvm 11 + |$lastUsingLine + | + |object Main { + | def main(args: Array[String]): Unit = { + | println(os.pwd) + | } + |}""".stripMargin).fromRoot { root => + val scalaFile = SourceScalaFile(root, os.sub / "Main.scala") + val Some(Right(result)) = ScalaPreprocessor.preprocess( + scalaFile, + logger = TestLogger(), + allowRestrictedFeatures = true, + suppressWarningOptions = SuppressWarningOptions() + )(using ScalaCliInvokeData.dummy) + expect(result.nonEmpty) + val Some(directivesPositions) = result.head.directivesPositions + expect(directivesPositions.startPos == 0 -> 0) + expect(directivesPositions.endPos == 3 -> lastUsingLine.length) + } + } } diff --git a/modules/core/src/main/scala/scala/build/Position.scala b/modules/core/src/main/scala/scala/build/Position.scala index 250e20873e..e7a13984e8 100644 --- a/modules/core/src/main/scala/scala/build/Position.scala +++ b/modules/core/src/main/scala/scala/build/Position.scala @@ -15,7 +15,8 @@ object Position { final case class File( path: Either[String, os.Path], startPos: (Int, Int), - endPos: (Int, Int) + endPos: (Int, Int), + offset: Int = 0 ) extends Position { def render(cwd: os.Path, sep: String): String = { val p = path match {