Skip to content

Commit

Permalink
Make .ext on empty paths return "" rather than crashing with a NPE (#87)
Browse files Browse the repository at this point in the history
Fixes com-lihaoyi/Ammonite#1017

This is more consistent with calling `.ext` on non-empty paths which have no extension, since those currently return `""`

We also introduce a new a `lastOption`  method, as a way to get the last segment in a list without throwing. Not sure if that's the right thing to do, or whether we should make `.last` return `""` (which is otherwise not a valid return value, since path segments cannot be empty).

Covered with simple unit tests
  • Loading branch information
lihaoyi authored Dec 8, 2021
1 parent c81c2b5 commit a603803
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
2 changes: 1 addition & 1 deletion os/src-jvm/ResourcePath.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class ResourcePath private[os](val resRoot: ResourceRoot, segments0: Array[Strin
def toSource = new Source.WritableSource(getInputStream)
val segments: IndexedSeq[String] = segments0
type ThisType = ResourcePath
def last = segments0.last
def lastOpt = segments0.lastOption
override def toString = resRoot.errorName + "/" + segments0.mkString("/")
protected[this] def make(p: Seq[String], ups: Int) = {
if (ups > 0){
Expand Down
24 changes: 17 additions & 7 deletions os/src/Path.scala
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,14 @@ trait BasePathImpl extends BasePath{
def /(chunk: PathChunk): ThisType

def ext = {
val li = last.lastIndexOf('.')
if (li == -1) ""
else last.slice(li+1, last.length)
lastOpt match{
case None => ""
case Some(lastSegment) =>
val li = lastSegment.lastIndexOf('.')
if (li == -1) ""
else last.slice(li+1, last.length)
}

}

override def baseName: String = {
Expand All @@ -193,7 +198,9 @@ trait BasePathImpl extends BasePath{
else last.slice(0, li)
}

def last: String
def last: String = lastOpt.getOrElse(throw PathError.LastOnEmptyPath())

def lastOpt: Option[String]
}

object PathError{
Expand All @@ -208,6 +215,9 @@ object PathError{

case class NoRelativePath(src: RelPath, base: RelPath)
extends IAE(s"Can't relativize relative paths $src from $base")

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

/**
Expand Down Expand Up @@ -239,7 +249,7 @@ object FilePath {
*/
class RelPath private[os](segments0: Array[String], val ups: Int)
extends FilePath with BasePathImpl with SegmentedPath {
def last = segments.last
def lastOpt = segments.lastOption
val segments: IndexedSeq[String] = segments0
type ThisType = RelPath
require(ups >= 0)
Expand Down Expand Up @@ -305,7 +315,7 @@ object RelPath {
*/
class SubPath private[os](val segments0: Array[String])
extends FilePath with BasePathImpl with SegmentedPath {
def last = segments.last
def lastOpt = segments.lastOption
val segments: IndexedSeq[String] = segments0
type ThisType = SubPath
protected[this] def make(p: Seq[String], ups: Int) = {
Expand Down Expand Up @@ -433,7 +443,7 @@ class Path private[os](val wrapped: java.nio.file.Path)
def segmentCount = wrapped.getNameCount
type ThisType = Path

def last = wrapped.getFileName.toString
def lastOpt = Option(wrapped.getFileName).map(_.toString)

def /(chunk: PathChunk): ThisType = {
if (chunk.ups > wrapped.getNameCount) throw PathError.AbsolutePathOutsideRoot
Expand Down
18 changes: 18 additions & 0 deletions os/test/src/PathTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ object PathTests extends TestSuite{
assert((base / "baseOnly").ext == "")
assert((base / "baseOnly.").ext == "")
}

test("emptyExt"){
os.root.ext ==> ""
os.rel.ext ==> ""
os.sub.ext ==> ""
os.up.ext ==> ""
}

test("emptyLast"){
intercept[PathError.LastOnEmptyPath](os.root.last).getMessage ==>
"empty path has no last segment"
intercept[PathError.LastOnEmptyPath](os.rel.last).getMessage ==>
"empty path has no last segment"
intercept[PathError.LastOnEmptyPath](os.sub.last).getMessage ==>
"empty path has no last segment"
intercept[PathError.LastOnEmptyPath](os.up.last).getMessage ==>
"empty path has no last segment"
}
}

test("RelPath"){
Expand Down

0 comments on commit a603803

Please sign in to comment.