Drop string subtyping

appveyor.yml

@@ -35,4 +35,10 @@ build_script:
 
 test_script:
   - echo "%JL_TEST_SCRIPT%"
-  - C:\julia\bin\julia -e "%JL_TEST_SCRIPT%"
+  - C:\julia\bin\julia -e "%JL_TEST_SCRIPT%"
+
+# Uncomment to support code coverage upload. Should only be enabled for packages
+# which would have coverage gaps without running on Windows
+on_success:
+  - echo "%JL_CODECOV_SCRIPT%"
+  - C:\julia\bin\julia -e "%JL_CODECOV_SCRIPT%"

src/FilePathsBase.jl

@@ -69,33 +69,42 @@ function __init__()
     register(WindowsPath)
 end
 
-abstract type AbstractPath <: AbstractString end
+"""
+    AbstractPath
+
+Defines an abstract filesystem path. Subtypes of `AbstractPath` should implement the
+following methods:
+
+- `Base.print(io, p)` (default: call base julia's joinpath with drive and path parts)
+- `FilePathsBase.parts(p)`
+- `FilePathsBase.root(p)`
+- `FilePathsBase.drive(p)`
+- `FilePathsBase.ispathtype(::Type{MyPath}, x::AbstractString) = true`
+"""
+abstract type AbstractPath end
 
 function register(T::Type{<:AbstractPath})
-    # We add the type to the beginning of our PATH_TYPES, 
-    # so that they can take precedence over the Posix and 
+    # We add the type to the beginning of our PATH_TYPES,
+    # so that they can take precedence over the Posix and
     # Windows paths.
     Compat.pushfirst!(PATH_TYPES, T)
 end
 
-# Required methods for subtype of AbstractString
-Compat.lastindex(p::AbstractPath) = lastindex(String(p))
-Compat.ncodeunits(p::AbstractPath) = ncodeunits(String(p))
-if VERSION >= v"0.7-"
-    Base.iterate(p::AbstractPath) = iterate(String(p))
-    Base.iterate(p::AbstractPath, state::Int) = iterate(String(p), state)
-else
-    Base.next(p::AbstractPath, i::Int) = next(String(p), i)
-end
+#=
+We only want to print the macro string syntax when compact is true and
+we want print to just return the string (this allows `string` to work normally)
+=#
+Base.print(io::IO, path::AbstractPath) = print(io, joinpath(drive(path), parts(path)...))
 
-# The following should be implemented in the concrete types
-Base.String(path::AbstractPath) = error("`String not implemented")
-parts(path::AbstractPath) = error("`parts` not implemented.")
-root(path::AbstractPath) = error("`root` not implemented.")
-drive(path::AbstractPath) = error("`drive` not implemented.")
+function Base.show(io::IO, path::AbstractPath)
+    get(io, :compact, false) ? print(io, path) : print(io, "p\"$path\"")
+end
 
+Base.parse(::Type{<:AbstractPath}, x::AbstractString) = Path(x)
 Base.convert(::Type{AbstractPath}, x::AbstractString) = Path(x)
-ispathtype(::Type{T}, x::AbstractString) where {T<:AbstractPath} = false
+Base.convert(::Type{String}, x::AbstractPath) = string(x)
+Base.promote_rule(::Type{String}, ::Type{<:AbstractPath}) = String
+ispathtype(::Type{<:AbstractPath}, x::AbstractString) = false
 
 include("constants.jl")
 include("utils.jl")

src/constants.jl

@@ -56,30 +56,30 @@ const S_IWOTH = 0o0002        # write by others
 const S_IXOTH = 0o0001        # execute by others
 
 const FILEMODE_TABLE = (
-    ((S_IFLNK,         "l"),
-     (S_IFREG,         "-"),
-     (S_IFBLK,         "b"),
-     (S_IFDIR,         "d"),
-     (S_IFCHR,         "c"),
-     (S_IFIFO,         "p")),
+    ((S_IFLNK,         'l'),
+     (S_IFREG,         '-'),
+     (S_IFBLK,         'b'),
+     (S_IFDIR,         'd'),
+     (S_IFCHR,         'c'),
+     (S_IFIFO,         'p')),
 
-    ((S_IRUSR,         "r"),),
-    ((S_IWUSR,         "w"),),
-    ((S_IXUSR|S_ISUID, "s"),
-     (S_ISUID,         "S"),
-     (S_IXUSR,         "x")),
+    ((S_IRUSR,         'r'),),
+    ((S_IWUSR,         'w'),),
+    ((S_IXUSR|S_ISUID, 's'),
+     (S_ISUID,         'S'),
+     (S_IXUSR,         'x')),
 
-    ((S_IRGRP,         "r"),),
-    ((S_IWGRP,         "w"),),
-    ((S_IXGRP|S_ISGID, "s"),
-     (S_ISGID,         "S"),
-     (S_IXGRP,         "x")),
+    ((S_IRGRP,         'r'),),
+    ((S_IWGRP,         'w'),),
+    ((S_IXGRP|S_ISGID, 's'),
+     (S_ISGID,         'S'),
+     (S_IXGRP,         'x')),
 
-    ((S_IROTH,         "r"),),
-    ((S_IWOTH,         "w"),),
-    ((S_IXOTH|S_ISVTX, "t"),
-     (S_ISVTX,         "T"),
-     (S_IXOTH,         "x"))
+    ((S_IROTH,         'r'),),
+    ((S_IWOTH,         'w'),),
+    ((S_IXOTH|S_ISVTX, 't'),
+     (S_ISVTX,         'T'),
+     (S_IXOTH,         'x'))
 )
 
-const DATA_SUFFIX = ["", "K", "M", "G", "T", "P", "E", "Z", "Y"]
+const DATA_SUFFIX = ["", "K", "M", "G", "T", "P", "E", "Z", "Y"]

src/mode.jl

@@ -43,8 +43,68 @@ function Mode(mode::UInt8, usr_grps::Symbol...)
     return Mode(user=user, group=group, other=other)
 end
 
-Base.show(io::IO, mode::Mode) = print(io, string(mode))
-Base.String(mode::Mode) = string(mode)
+Mode(s::AbstractString) = parse(Mode, s)
+
+function Base.parse(::Type{Mode}, x::AbstractString)
+    n = length(FILEMODE_TABLE)
+
+    if length(x) != n
+        throw(ArgumentError(
+            "Expected a mode permission string with $n characters (e.g., '-rwxrwxrwx')"
+        ))
+    end
+
+    m = zero(UInt64)
+    for i in 1:n
+        table = FILEMODE_TABLE[i]
+        found = false
+        c = x[i]
+        c == '-' && continue
+
+        for (bit, char) in table
+            if c == char
+                m |= bit
+                found = true
+                break
+            end
+        end
+        if !found
+            options = last.(table)
+            throw(ArgumentError(
+                "Unknown character '$c' at position $i, expected one of $options."
+            ))
+        end
+    end
+
+    return Mode(m)
+end
+
+"""Convert a file's mode to a string of the form '-rwxrwxrwx'."""
+function Base.print(io::IO, mode::Mode)
+    n = length(FILEMODE_TABLE)
+    perm = Vector{Char}(undef, n)
+
+    for i in 1:n
+        table = FILEMODE_TABLE[i]
+        found = false
+        for (bit, char) in table
+            if mode.m & bit == bit
+                perm[i] = char
+                found = true
+                break
+            end
+        end
+        if !found
+            perm[i] = '-'
+        end
+    end
+
+    print(io, String(perm))
+end
+
+function Base.show(io::IO, mode::Mode)
+    get(io, :compact, false) ? print(io, mode) : print(io, "Mode(\"$mode\")")
+end
 
 Base.:-(a::Mode, b::Mode) = Mode(a.m & ~b.m)
 Base.:+(a::Mode, b::Mode) = Mode(a.m | b.m)
@@ -119,27 +179,6 @@ Base.ischardev(mode::Mode) = _meta(mode.m) == S_IFCHR
 """Return True if mode is from a block special device file."""
 Base.isblockdev(mode::Mode) = _meta(mode.m) == S_IFBLK
 
-
-"""Convert a file's mode to a string of the form '-rwxrwxrwx'."""
-function Base.string(mode::Mode)
-    perm = []
-    for table in FILEMODE_TABLE
-        found = false
-        for (bit, char) in table
-            if mode.m & bit == bit
-                push!(perm, char)
-                found = true
-                break
-            end
-        end
-        if !found
-            push!(perm, '-')
-        end
-    end
-
-    return join(perm)
-end
-
 """
 Return the portion of the file's mode that can be set by
 os.chmod().

src/path.jl

@@ -249,14 +249,14 @@ Base.isempty(path::AbstractPath) = isempty(parts(path))
 
 Returns whether the path actually exists on the system.
 """
-exists(path::AbstractPath) = ispath(String(path))
+exists(path::AbstractPath) = ispath(string(path))
 
 """
     real(path::AbstractPath) -> AbstractPath
 
 Canonicalizes a path by expanding symlinks and removing "." and ".." entries.
 """
-Base.real(path::AbstractPath) = Path(realpath(String(path)))
+Base.real(path::AbstractPath) = Path(realpath(string(path)))
 
 """
     norm(path::AbstractPath) -> AbstractPath
@@ -351,8 +351,8 @@ end
 The following a descriptive methods for paths
 built around stat
 =#
-Base.stat(path::AbstractPath) = Status(stat(String(path)))
-Base.lstat(path::AbstractPath) = Status(lstat(String(path)))
+Base.stat(path::AbstractPath) = Status(stat(string(path)))
+Base.lstat(path::AbstractPath) = Status(lstat(string(path)))
 
 """
     mode(path::AbstractPath) -> Mode
@@ -469,7 +469,7 @@ code in the implementation instances.
 TODO: Document these once we're comfortable with them.
 =#
 
-Base.cd(path::AbstractPath) = cd(String(path))
+Base.cd(path::AbstractPath) = cd(string(path))
 function Base.cd(fn::Function, dir::AbstractPath)
     old = cwd()
     try
@@ -495,8 +495,8 @@ function Base.mkdir(path::AbstractPath; mode=0o777, recursive=false, exist_ok=fa
 			end
         end
 
-		
-		VERSION >= v"0.7-" ? mkdir(String(path), mode=mode) : mkdir(String(path), mode)
+
+		VERSION >= v"0.7-" ? mkdir(string(path), mode=mode) : mkdir(string(path), mode)
 
     end
 end
@@ -508,7 +508,7 @@ function Base.symlink(src::AbstractPath, dest::AbstractPath; exist_ok=false, ove
         end
 
         if !exists(dest)
-            symlink(String(src), String(dest))
+            symlink(string(src), string(dest))
         elseif !exist_ok
             error("$dest already exists.")
         end
@@ -552,7 +552,7 @@ function move(src::AbstractPath, dest::AbstractPath; recursive=false, exist_ok=f
                 mkdir(parent(dest); recursive=recursive, exist_ok=true)
             end
 
-            mv(String(src), String(dest))
+            mv(string(src), string(dest))
         elseif !exist_ok
             error("$dest already exists.")
         end
@@ -562,21 +562,21 @@ function move(src::AbstractPath, dest::AbstractPath; recursive=false, exist_ok=f
 end
 
 function Base.cp(src::AbstractPath, dest::AbstractPath; force::Bool=false, follow_symlinks::Bool=false)
-    Compat.cp(String(src), String(dest); force=force, follow_symlinks=follow_symlinks)
+    Compat.cp(string(src), string(dest); force=force, follow_symlinks=follow_symlinks)
 end
 
-remove(path::AbstractPath; recursive=false) = rm(String(path); recursive=recursive)
-Base.touch(path::AbstractPath) = touch(String(path))
+remove(path::AbstractPath; recursive=false) = rm(string(path); recursive=recursive)
+Base.touch(path::AbstractPath) = touch(string(path))
 
 tmpname() = Path(tempname())
 tmpdir() = Path(tempdir())
 
 function mktmp(parent::AbstractPath=Path(tempdir()))
-    path, io = mktemp(String(parent))
+    path, io = mktemp(string(parent))
     return Path(path), io
 end
 
-mktmpdir(parent::AbstractPath=tmpdir()) = Path(mktempdir(String(parent)))
+mktmpdir(parent::AbstractPath=tmpdir()) = Path(mktempdir(string(parent)))
 
 function mktmp(fn::Function, parent=tmpdir())
     (tmp_path, tmp_io) = mktmp(parent)
@@ -608,7 +608,7 @@ function Base.chown(path::AbstractPath, user::AbstractString, group::AbstractStr
         if recursive
             push!(chown_cmd, "-R")
         end
-        append!(chown_cmd, String["$(user):$(group)", String(path)])
+        append!(chown_cmd, String["$(user):$(group)", string(path)])
 
         run(Cmd(chown_cmd))
     else
@@ -654,7 +654,7 @@ julia> mode(p"newfile")
 ```
 """
 function Base.chmod(path::AbstractPath, mode::Mode; recursive=false)
-    chmod_path = String(path)
+    chmod_path = string(path)
     chmod_mode = raw(mode)
 
     if isdir(path) && recursive
@@ -725,20 +725,25 @@ function Base.chmod(path::AbstractPath, symbolic_mode::AbstractString; recursive
     end
 end
 
-Base.read(path::AbstractPath) = read(String(path), String)
+Base.open(path::AbstractPath, args...) = open(string(path), args...)
+function Base.open(f::Function, path::AbstractPath, args...; kwargs...)
+    open(f, string(path), args...; kwargs...)
+end
+
+Base.read(path::AbstractPath) = read(string(path), String)
 
 function Base.write(path::AbstractPath, content::AbstractString, mode="w")
-    open(String(path), mode) do f
+    open(path, mode) do f
         write(f, content)
     end
 end
 
-Base.readlink(path::AbstractPath) = Path(readlink(String(path)))
-Base.readdir(path::AbstractPath) = map(Path, readdir(String(path)))
+Base.readlink(path::AbstractPath) = Path(readlink(string(path)))
+Base.readdir(path::AbstractPath) = map(Path, readdir(string(path)))
 
 function Base.download(src::AbstractString, dest::AbstractPath, overwrite::Bool=false)
     if !exists(dest) || overwrite
-        download(src, String(dest))
+        download(src, string(dest))
     end
     return dest
 end

src/posix.jl

@@ -5,7 +5,7 @@ end
 PosixPath() = PosixPath(tuple())
 
 function PosixPath(str::AbstractString)
-    str = String(str)
+    str = string(str)
 
     if isempty(str)
         return PosixPath(tuple("."))
@@ -20,12 +20,9 @@ end
 
 # The following should be implemented in the concrete types
 ==(a::PosixPath, b::PosixPath) = parts(a) == parts(b)
-Base.String(path::PosixPath) = joinpath(parts(path)...)
 parts(path::PosixPath) = path.parts
 ispathtype(::Type{PosixPath}, str::AbstractString) = Compat.Sys.isunix()
 
-Base.show(io::IO, path::PosixPath) = print(io, "p\"$(join(parts(path), '/'))\"")
-
 function isabs(path::PosixPath)
     if parts(path)[1] == POSIX_PATH_SEPARATOR
         return true

src/windows.jl

@@ -67,7 +67,6 @@ function ==(a::WindowsPath, b::WindowsPath)
         lowercase(drive(a)) == lowercase(drive(b)) &&
         lowercase(root(a)) == lowercase(root(b))
 end
-Base.String(path::WindowsPath) = joinpath(parts(path)...)
 parts(path::WindowsPath) = path.parts
 drive(path::WindowsPath) = path.drive
 root(path::WindowsPath) = path.root