From 92c5a941c9fd38502c7a0d38c02452a126fc9bcc Mon Sep 17 00:00:00 2001 From: pawelsadlo <106806258+pawelsadlo@users.noreply.github.com> Date: Thu, 19 Sep 2024 11:26:33 +0200 Subject: [PATCH] Fix expecty macro (#307) trying to fix #305 It seems that AST that our macro produces is spliced into AST of the expression provided to Expecty.expect Then it tries to evaluate this composed AST, but it fails, I'm not exactly sure why, maybe some local vals in macro arent simply available in the scope where expecty evaluates them. I think Expecty.expect should expand only AST of expression provided to it (and do not follow macros and splice ASTs generated by them) This PR rewrites macro a little bit not to use values calculated in macro itself (it also removes potentially problematic Expr.apply call). As a consequence we have to make a segmentsFromString public, because call to it is also inlined. This change fixes compilation of expecty macros, but due to the fact that expecty expands AST too much, its assertion clues will be malformed anyway, but I don't think we would be able to solve this in os-lib. Note: Using scala 2 there is no bug. --- build.sc | 2 ++ os/src-3/Macros.scala | 8 +++--- os/src/Path.scala | 4 +-- os/test/src-jvm/ExpectyIntegration.scala | 33 ++++++++++++++++++++++++ os/test/src/PathTests.scala | 1 + 5 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 os/test/src-jvm/ExpectyIntegration.scala diff --git a/build.sc b/build.sc index cd8aa492..02005bf9 100644 --- a/build.sc +++ b/build.sc @@ -24,6 +24,7 @@ object Deps { val geny = ivy"com.lihaoyi::geny::1.1.1" val sourcecode = ivy"com.lihaoyi::sourcecode::0.4.2" val utest = ivy"com.lihaoyi::utest::0.8.4" + val expecty = ivy"com.eed3si9n.expecty::expecty::0.16.0" def scalaReflect(scalaVersion: String) = ivy"org.scala-lang:scala-reflect:$scalaVersion" def scalaLibrary(version: String) = ivy"org.scala-lang:scala-library:${version}" } @@ -164,6 +165,7 @@ object os extends Module { object jvm extends Cross[OsJvmModule](scalaVersions) trait OsJvmModule extends OsModule with MiMaChecks { object test extends ScalaTests with OsLibTestModule { + override def ivyDeps = T{super.ivyDeps() ++ Agg(Deps.expecty)} // we check the textual output of system commands and expect it in english def forkEnv = super.forkEnv() ++ Map( diff --git a/os/src-3/Macros.scala b/os/src-3/Macros.scala index edcea44b..0dca663c 100644 --- a/os/src-3/Macros.scala +++ b/os/src-3/Macros.scala @@ -1,6 +1,6 @@ package os -import os.PathChunk.{RelPathChunk, StringPathChunk, segmentsFromStringLiteralValidation} +import os.PathChunk.{RelPathChunk, StringPathChunk, segmentsFromString, segmentsFromStringLiteralValidation} import os.RelPath.fromStringSegments import scala.quoted.{Expr, Quotes} @@ -20,11 +20,9 @@ object Macros { s.asTerm match { case Inlined(_, _, Literal(StringConstant(literal))) => - val stringSegments = segmentsFromStringLiteralValidation(literal) + segmentsFromStringLiteralValidation(literal) '{ - new RelPathChunk(fromStringSegments(${ - Expr(stringSegments) - })) + new RelPathChunk(fromStringSegments(segmentsFromString($s))) } case _ => '{ diff --git a/os/src/Path.scala b/os/src/Path.scala index 96dded27..35631cb3 100644 --- a/os/src/Path.scala +++ b/os/src/Path.scala @@ -20,14 +20,14 @@ trait StringPathChunkConversion { } object PathChunk extends PathChunkMacros { - private[os] def segmentsFromString(s: String): Array[String] = { + def segmentsFromString(s: String): Array[String] = { val trailingSeparatorsCount = s.reverseIterator.takeWhile(_ == '/').length val strNoTrailingSeps = s.dropRight(trailingSeparatorsCount) val splitted = strNoTrailingSeps.split('/') splitted ++ Array.fill(trailingSeparatorsCount)("") } - private[os] def segmentsFromStringLiteralValidation(literal: String) = { + private[os] def segmentsFromStringLiteralValidation(literal: String): Array[String] = { val stringSegments = segmentsFromString(literal) val validSegmnts = validLiteralSegments(stringSegments) val sanitizedLiteral = validSegmnts.mkString("/") diff --git a/os/test/src-jvm/ExpectyIntegration.scala b/os/test/src-jvm/ExpectyIntegration.scala new file mode 100644 index 00000000..da6add3b --- /dev/null +++ b/os/test/src-jvm/ExpectyIntegration.scala @@ -0,0 +1,33 @@ +package test.os + +import os._ +import utest._ +import com.eed3si9n.expecty.Expecty.expect + +object ExpectyIntegration extends TestSuite { + val tests = Tests { + test("Literals") { + test("Basic") { + expect(rel / "src" / "Main/.scala" == rel / "src" / "Main" / ".scala") + expect(root / "core/src/test" == root / "core" / "src" / "test") + expect(root / "core/src/test" == root / "core" / "src/test") + } + test("literals with [..]") { + expect(rel / "src" / ".." == rel / "src" / os.up) + expect(root / "src/.." == root / "src" / os.up) + expect(root / "src" / ".." == root / "src" / os.up) + expect(root / "hello" / ".." / "world" == root / "hello" / os.up / "world") + expect(root / "hello" / "../world" == root / "hello" / os.up / "world") + expect(root / "hello/../world" == root / "hello" / os.up / "world") + } + test("from issue") { + expect(Seq(os.pwd / "foo") == Seq(os.pwd / "foo")) + val path = os.Path("/") / "tmp" / "foo" + expect(path.startsWith(os.Path("/") / "tmp")) + } + test("multiple args") { + expect(rel / "src" / ".." == rel / "src" / os.up, root / "src/.." == root / "src" / os.up) + } + } + } +} diff --git a/os/test/src/PathTests.scala b/os/test/src/PathTests.scala index 608a765a..a12519b3 100644 --- a/os/test/src/PathTests.scala +++ b/os/test/src/PathTests.scala @@ -21,6 +21,7 @@ object PathTests extends TestSuite { assert(root / "core/src/test" == root / "core" / "src/test") } test("literals with [..]") { + assert(rel / "src" / ".." == rel / "src" / os.up) assert(root / "src/.." == root / "src" / os.up) assert(root / "src" / ".." == root / "src" / os.up)