Skip to content

Commit

Permalink
Allow multiple segments in Stringliterals (#297)
Browse files Browse the repository at this point in the history
This PR adds support for multi-segment literal strings.
for example
```scala
val path = root / "foo/bar" 
val qux = "qux"
val path = root / "foo/bar" / "baz" / qux
val path = root / "foo/bar" / "baz/qux"
```
it also parses `..` segments from literal as `os.up` enabling syntax
like:
```scala
val path = root / "foo" / ".." / "bar" // equivalent to `root / "foo" / os.up / "bar"`
val path = root / "foo" /  "../bar" // equivalent to `root / "foo" / os.up / "bar"`
```
non-canonical paths used in literals result in compile-time errors,
suggesting usage of canonical paths or removing path segment, eg.
```scala
val path = root / "foo/./bar" //suggests "foo/bar"
val path = root / "foo//bar" //suggests "foo/bar"
val path = root / "//foo//bar/./baz" //suggests "foo/bar/baz"

val path = root / ""  //suggests removing the segment
val path = root / "." //suggests removing the segment
val path = root / "/" //suggests removing the segment
val path = root / "//" //suggests removing the segment
val path = root / "/./" //suggests removing the segment

```

Note:
Its not usable in os-Lib itself, due to the fact that it would lead to
macro expansion in the same compilation unit as its definition.

@lihaoyi 
there is a little bit of hacking involved:

1. There is a default implicit conversion not being a macro to be used
within os library, without this we would have to add a submodule and
split the whole project, I'm not even sure if its doable.
4. Needed to turn off acyclic in `Path` and particular `Macro` files
(also needed to mock `acyclic.skipped` in case of `scala 3`).
5. Needed to provide another implicit conversion in `ViewBoundImplicit`
trait because macros turn out to be not avaliable as implicit views,
this is needed for `ArrayPathChunk` and `SeqPathChunk` to work.
  • Loading branch information
pawelsadlo authored Sep 10, 2024
1 parent 5561ad6 commit 4262d5c
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 15 deletions.
7 changes: 7 additions & 0 deletions build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
def scalaReflect(scalaVersion: String) = ivy"org.scala-lang:scala-reflect:$scalaVersion"
def scalaLibrary(version: String) = ivy"org.scala-lang:scala-library:${version}"
}

Expand Down Expand Up @@ -94,6 +95,12 @@ trait OsLibModule

trait OsModule extends OsLibModule { outer =>
def ivyDeps = Agg(Deps.geny)
override def compileIvyDeps = T{
val scalaReflectOpt = Option.when(!ZincWorkerUtil.isDottyOrScala3(scalaVersion())) (
Deps.scalaReflect(scalaVersion())
)
super.compileIvyDeps() ++ scalaReflectOpt
}

def artifactName = "os-lib"

Expand Down
33 changes: 33 additions & 0 deletions os/src-2/Macros.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package os

import os.PathChunk.segmentsFromStringLiteralValidation

import scala.language.experimental.macros
import scala.reflect.macros.blackbox
import acyclic.skipped

// StringPathChunkConversion is a fallback to non-macro String => PathChunk implicit conversion in case eta expansion is needed, this is required for ArrayPathChunk and SeqPathChunk
trait PathChunkMacros extends StringPathChunkConversion {
implicit def stringPathChunkValidated(s: String): PathChunk =
macro Macros.stringPathChunkValidatedImpl
}

object Macros {

def stringPathChunkValidatedImpl(c: blackbox.Context)(s: c.Expr[String]): c.Expr[PathChunk] = {
import c.universe.{Try => _, _}

s match {
case Expr(Literal(Constant(literal: String))) =>
val stringSegments = segmentsFromStringLiteralValidation(literal)

c.Expr(
q"""new _root_.os.PathChunk.RelPathChunk(_root_.os.RelPath.fromStringSegments($stringSegments))"""
)
case nonLiteral =>
c.Expr(
q"new _root_.os.PathChunk.StringPathChunk($nonLiteral)"
)
}
}
}
37 changes: 37 additions & 0 deletions os/src-3/Macros.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package os

import os.PathChunk.{RelPathChunk, StringPathChunk, segmentsFromStringLiteralValidation}
import os.RelPath.fromStringSegments

import scala.quoted.{Expr, Quotes}
import acyclic.skipped

// StringPathChunkConversion is a fallback to non-macro String => PathChunk implicit conversion in case eta expansion is needed, this is required for ArrayPathChunk and SeqPathChunk
trait PathChunkMacros extends StringPathChunkConversion {
inline implicit def stringPathChunkValidated(s: String): PathChunk =
${
Macros.stringPathChunkValidatedImpl('s)
}
}

object Macros {
def stringPathChunkValidatedImpl(s: Expr[String])(using quotes: Quotes): Expr[PathChunk] = {
import quotes.reflect.*

s.asTerm match {
case Inlined(_, _, Literal(StringConstant(literal))) =>
val stringSegments = segmentsFromStringLiteralValidation(literal)
'{
new RelPathChunk(fromStringSegments(${
Expr(stringSegments)
}))
}
case _ =>
'{
{
new StringPathChunk($s)
}
}
}
}
}
6 changes: 6 additions & 0 deletions os/src-3/acyclic.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package os
private[os] object acyclic {

/** Mocks [[\\import acyclic.skipped]] for scala 3 */
private[os] type skipped
}
63 changes: 59 additions & 4 deletions os/src/Path.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,76 @@ package os

import java.net.URI
import java.nio.file.Paths

import collection.JavaConverters._
import scala.language.implicitConversions
import java.nio.file
import acyclic.skipped
import os.PathError.{InvalidSegment, NonCanonicalLiteral}

import scala.util.Try //needed for cross-version defined macros

trait PathChunk {
def segments: Seq[String]
def ups: Int
}
object PathChunk {
trait StringPathChunkConversion {

implicit def stringToPathChunk(s: String): PathChunk =
new PathChunk.StringPathChunkInternal(s)
}

object PathChunk extends PathChunkMacros {
private[os] 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) = {
val stringSegments = segmentsFromString(literal)
val validSegmnts = validLiteralSegments(stringSegments)
val sanitizedLiteral = validSegmnts.mkString("/")
if (validSegmnts.isEmpty) throw InvalidSegment(
literal,
s"Literal path sequence [$literal] doesn't affect path being formed, please remove it"
)
if (literal != sanitizedLiteral) throw NonCanonicalLiteral(literal, sanitizedLiteral)
stringSegments
}
private def validLiteralSegments(segments: Array[String]): Array[String] = {
val AllowedLiteralSegment = ".."
segments.collect {
case AllowedLiteralSegment => AllowedLiteralSegment
case segment if Try(BasePath.checkSegment(segment)).isSuccess => segment
}
}

implicit class RelPathChunk(r: RelPath) extends PathChunk {
def segments = r.segments
def ups = r.ups
override def toString() = r.toString
}

implicit class SubPathChunk(r: SubPath) extends PathChunk {
def segments = r.segments
def ups = 0
override def toString() = r.toString
}
implicit class StringPathChunk(s: String) extends PathChunk {

// Implicit String => PathChunk conversion used inside os-lib, prevents macro expansion in same compilation unit
private[os] implicit class StringPathChunkInternal(s: String) extends PathChunk {
BasePath.checkSegment(s)
def segments = Seq(s)
def ups = 0
override def toString() = s
}

// binary compatibility shim
class StringPathChunk(s: String) extends StringPathChunkInternal(s)

// binary compatibility shim
def StringPathChunk(s: String): StringPathChunk = new StringPathChunk(s)

implicit class SymbolPathChunk(s: Symbol) extends PathChunk {
BasePath.checkSegment(s.name)
def segments = Seq(s.name)
Expand Down Expand Up @@ -227,6 +271,11 @@ object PathError {

case class LastOnEmptyPath()
extends IAE("empty path has no last segment")

case class NonCanonicalLiteral(providedLiteral: String, sanitizedLiteral: String)
extends IAE(
s"Literal path sequence [$providedLiteral] used in OS-Lib must be in a canonical form, please use [$sanitizedLiteral] instead"
)
}

/**
Expand Down Expand Up @@ -297,6 +346,7 @@ class RelPath private[os] (segments0: Array[String], val ups: Int)
}

object RelPath {

def apply[T: PathConvertible](f0: T): RelPath = {
val f = implicitly[PathConvertible[T]].apply(f0)

Expand All @@ -319,6 +369,10 @@ object RelPath {
val up: RelPath = new RelPath(Internals.emptyStringArray, 1)
val rel: RelPath = new RelPath(Internals.emptyStringArray, 0)
implicit def SubRelPath(p: SubPath): RelPath = new RelPath(p.segments0, 0)
def fromStringSegments(segments: Array[String]): RelPath = segments.foldLeft(RelPath.rel) {
case (agg, "..") => agg / up
case (agg, seg) => agg / seg
}
}

/**
Expand Down Expand Up @@ -473,6 +527,7 @@ object Path {

trait ReadablePath {
def toSource: os.Source

def getInputStream: java.io.InputStream
}

Expand Down
76 changes: 65 additions & 11 deletions os/test/src/PathTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,68 @@ package test.os

import java.nio.file.Paths
import java.io.File

import os._
import os.Path.{driveRoot}
import os.Path.driveRoot
import utest.{assert => _, _}

import java.net.URI
object PathTests extends TestSuite {
private def nonCanonicalLiteral(providedLiteral: String, sanitizedLiteral: String) =
s"Literal path sequence [$providedLiteral] used in OS-Lib must be in a canonical form, please use [$sanitizedLiteral] instead"
private def removeLiteralErr(literal: String) =
s"Literal path sequence [$literal] doesn't affect path being formed, please remove it"

val tests = Tests {
test("Literals") {
test("Basic") {
assert(rel / "src" / "Main/.scala" == rel / "src" / "Main" / ".scala")
assert(root / "core/src/test" == root / "core" / "src" / "test")
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)
assert(root / "hello" / ".." / "world" == root / "hello" / os.up / "world")
assert(root / "hello" / "../world" == root / "hello" / os.up / "world")
assert(root / "hello/../world" == root / "hello" / os.up / "world")
}

test("Compile errors") {
compileError("""root / "/" """).check("", removeLiteralErr("/"))
compileError("""root / "/ " """).check("", nonCanonicalLiteral("/ ", " "))
compileError("""root / " /" """).check("", nonCanonicalLiteral(" /", " "))
compileError("""root / "//" """).check("", removeLiteralErr("//"))

compileError("""root / "foo/" """).check("", nonCanonicalLiteral("foo/", "foo"))
compileError("""root / "foo//" """).check("", nonCanonicalLiteral("foo//", "foo"))

compileError("""root / "foo/bar/" """).check("", nonCanonicalLiteral("foo/bar/", "foo/bar"))
compileError("""root / "foo/bar//" """).check(
"",
nonCanonicalLiteral("foo/bar//", "foo/bar")
)

compileError("""root / "/foo" """).check("", nonCanonicalLiteral("/foo", "foo"))
compileError("""root / "//foo" """).check("", nonCanonicalLiteral("//foo", "foo"))

compileError("""root / "//foo/" """).check("", nonCanonicalLiteral("//foo/", "foo"))

compileError(""" rel / "src" / "" """).check("", removeLiteralErr(""))
compileError(""" rel / "src" / "." """).check("", removeLiteralErr("."))

compileError(""" root / "src/" """).check("", nonCanonicalLiteral("src/", "src"))
compileError(""" root / "src/." """).check("", nonCanonicalLiteral("src/.", "src"))

compileError(""" root / "" """).check("", removeLiteralErr(""))
compileError(""" root / "." """).check("", removeLiteralErr("."))

}
}
test("Basic") {
val base = rel / "src" / "main" / "scala"
val subBase = sub / "src" / "main" / "scala"

test("Transform posix paths") {
// verify posix string format of driveRelative path
assert(posix(root / "omg") == posix(Paths.get("/omg").toAbsolutePath))
Expand Down Expand Up @@ -279,29 +331,31 @@ object PathTests extends TestSuite {
}
}
test("Errors") {
def nonLiteral(s: String) = s

test("InvalidChars") {
val ex = intercept[PathError.InvalidSegment](rel / "src" / "Main/.scala")
val ex = intercept[PathError.InvalidSegment](rel / "src" / nonLiteral("Main/.scala"))

val PathError.InvalidSegment("Main/.scala", msg1) = ex

assert(msg1.contains("[/] is not a valid character to appear in a path segment"))

val ex2 = intercept[PathError.InvalidSegment](root / "hello" / ".." / "world")
val ex2 = intercept[PathError.InvalidSegment](root / "hello" / nonLiteral("..") / "world")

val PathError.InvalidSegment("..", msg2) = ex2

assert(msg2.contains("use the `up` segment from `os.up`"))
}
test("InvalidSegments") {
intercept[PathError.InvalidSegment] { root / "core/src/test" }
intercept[PathError.InvalidSegment] { root / "" }
intercept[PathError.InvalidSegment] { root / "." }
intercept[PathError.InvalidSegment] { root / ".." }
intercept[PathError.InvalidSegment] { root / nonLiteral("core/src/test") }
intercept[PathError.InvalidSegment] { root / nonLiteral("") }
intercept[PathError.InvalidSegment] { root / nonLiteral(".") }
intercept[PathError.InvalidSegment] { root / nonLiteral("..") }
}
test("EmptySegment") {
intercept[PathError.InvalidSegment](rel / "src" / "")
intercept[PathError.InvalidSegment](rel / "src" / ".")
intercept[PathError.InvalidSegment](rel / "src" / "..")
intercept[PathError.InvalidSegment](rel / "src" / nonLiteral(""))
intercept[PathError.InvalidSegment](rel / "src" / nonLiteral("."))
intercept[PathError.InvalidSegment](rel / "src" / nonLiteral(".."))
}
test("CannotRelativizeAbsAndRel") {
val abs = pwd
Expand Down
39 changes: 39 additions & 0 deletions os/test/src/SegmentsFromStringTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package os

import os.PathChunk.segmentsFromString
import utest.{assert => _, _}

object SegmentsFromStringTests extends TestSuite {

val tests = Tests {
test("segmentsFromString") {
def testSegmentsFromString(s: String, expected: List[String]) = {
assert(segmentsFromString(s).sameElements(expected))
}

testSegmentsFromString(" ", List(" "))

testSegmentsFromString("", List(""))

testSegmentsFromString("""foo/bar/baz""", List("foo", "bar", "baz"))

testSegmentsFromString("""/""", List("", ""))
testSegmentsFromString("""//""", List("", "", ""))
testSegmentsFromString("""///""", List("", "", "", ""))

testSegmentsFromString("""a/""", List("a", ""))
testSegmentsFromString("""a//""", List("a", "", ""))
testSegmentsFromString("""a///""", List("a", "", "", ""))

testSegmentsFromString("""ahs/""", List("ahs", ""))
testSegmentsFromString("""ahs//""", List("ahs", "", ""))

testSegmentsFromString("""ahs/aa/""", List("ahs", "aa", ""))
testSegmentsFromString("""ahs/aa//""", List("ahs", "aa", "", ""))

testSegmentsFromString("""/a""", List("", "a"))
testSegmentsFromString("""//a""", List("", "", "a"))
testSegmentsFromString("""//a/""", List("", "", "a", ""))
}
}
}

0 comments on commit 4262d5c

Please sign in to comment.