RFC: Change 'readdir' to return an iterator.

base/file.jl

@@ -572,14 +572,43 @@ struct uv_dirent_t
     typ::Cint
 end
 
+struct ReadDirIterator
+    path::String
+end
+
+function Base.iterate(iter::ReadDirIterator)
+    uv_readdir_req = zeros(UInt8, ccall(:jl_sizeof_uv_fs_t, Int32, ()))
+    path = iter.path
+    err = ccall(:uv_fs_scandir, Int32, (Ptr{Cvoid}, Ptr{UInt8}, Cstring, Cint, Ptr{Cvoid}),
+                eventloop(), uv_readdir_req, path, 0, C_NULL)
+    err < 0 && throw(SystemError("unable to read directory $path", -err))
+    return iterate(iter, uv_readdir_req)
+end
+
+function Base.iterate(d::ReadDirIterator, uv_readdir_req::Vector{UInt8})
+    ent = Ref{uv_dirent_t}()
+    status =  ccall(:uv_fs_scandir_next, Cint, (Ptr{Cvoid}, Ptr{uv_dirent_t}), uv_readdir_req, ent)
+    if status == Base.UV_EOF
+        ccall(:jl_uv_fs_req_cleanup, Cvoid, (Ptr{UInt8},), uv_readdir_req)
+        return nothing
+    end
+    path = unsafe_string(ent[].name)
+    return (path, uv_readdir_req)
+end
+
+Base.IteratorSize(::Type{ReadDirIterator}) = Base.SizeUnknown()
+Base.eltype(::Type{ReadDirIterator}) = String
+
+Base.show(io::IO, iter::ReadDirIterator) = show(io, collect(iter))
+
 """
-    readdir(dir::AbstractString=".") -> Vector{String}
+    readdir(dir::AbstractString=".") -> ReadDirIterator
 
-Return the files and directories in the directory `dir` (or the current working directory if not given).
+Return an iterator that generates the files and directories in the directory `dir` (or the current working directory if not given).
 
 # Examples
 ```julia-repl
-julia> readdir("/home/JuliaUser/Projects/julia")
+julia> collect(readdir("/home/JuliaUser/Projects/julia"))
 34-element Array{String,1}:
  ".circleci"
  ".freebsdci.sh"
@@ -594,26 +623,11 @@ julia> readdir("/home/JuliaUser/Projects/julia")
 ```
 """
 function readdir(path::AbstractString)
-    # Allocate space for uv_fs_t struct
-    uv_readdir_req = zeros(UInt8, ccall(:jl_sizeof_uv_fs_t, Int32, ()))
-
-    # defined in sys.c, to call uv_fs_readdir, which sets errno on error.
-    err = ccall(:uv_fs_scandir, Int32, (Ptr{Cvoid}, Ptr{UInt8}, Cstring, Cint, Ptr{Cvoid}),
-                eventloop(), uv_readdir_req, path, 0, C_NULL)
-    err < 0 && throw(SystemError("unable to read directory $path", -err))
-    #uv_error("unable to read directory $path", err)
-
-    # iterate the listing into entries
-    entries = String[]
-    ent = Ref{uv_dirent_t}()
-    while Base.UV_EOF != ccall(:uv_fs_scandir_next, Cint, (Ptr{Cvoid}, Ptr{uv_dirent_t}), uv_readdir_req, ent)
-        push!(entries, unsafe_string(ent[].name))
-    end
-
-    # Clean up the request string
-    ccall(:jl_uv_fs_req_cleanup, Cvoid, (Ptr{UInt8},), uv_readdir_req)
+    # Test that the path doesn't contain illegal null characters (issue #10994).
+    # Better to fail fast than wait until iteration starts to raise an error.
+    Base.unsafe_convert(Cstring, path)
 
-    return entries
+    ReadDirIterator(String(path))
 end
 
 readdir() = readdir(".")
@@ -661,7 +675,7 @@ julia> (root, dirs, files) = first(itr)
 function walkdir(root; topdown=true, follow_symlinks=false, onerror=throw)
     content = nothing
     try
-        content = readdir(root)
+        content = collect(readdir(root))
     catch err
         isa(err, SystemError) || throw(err)
         onerror(err)

base/loading.jl

@@ -76,7 +76,7 @@ else
     function isfile_casesensitive(path)
         isfile(path) || return false
         dir, filename = splitdir(path)
-        any(readdir(dir) .== filename)
+        any(collect(readdir(dir)) .== filename)
     end
 end
 

base/show.jl

@@ -40,10 +40,13 @@ function show(io::IO, ::MIME"text/plain", f::Function)
     end
 end
 
+displayname(::KeySet)="Keys"
+displayname(::ValueIterator)="Values"
+displayname(::Filesystem.ReadDirIterator)="Directory contents"
 function show(io::IO, ::MIME"text/plain", iter::Union{KeySet,ValueIterator})
     print(io, summary(iter))
     isempty(iter) && return
-    print(io, ". ", isa(iter,KeySet) ? "Keys" : "Values", ":")
+    print(io, ". ", displayname(iter), ":")
     limit::Bool = get(io, :limit, false)
     if limit
         sz = displaysize(io)

stdlib/InteractiveUtils/test/runtests.jl

@@ -185,7 +185,7 @@ end
             @test occursin("Environment:", ver)
             @test occursin("Package Status:", ver)
             @test occursin("no packages installed", ver)
-            @test isempty(readdir(dir))
+            @test isempty(collect(readdir(dir)))
         end
     end
     let exename = `$(Base.julia_cmd()) --startup-file=no`

stdlib/LibGit2/src/LibGit2.jl

@@ -901,7 +901,7 @@ function snapshot(repo::GitRepo)
     index = with(GitIndex, repo) do idx; write_tree!(idx) end
     work = try
         with(GitIndex, repo) do idx
-            if length(readdir(path(repo))) > 1
+            if length(collect(readdir(path(repo)))) > 1
                 add!(idx, ".")
                 write!(idx)
             end

stdlib/Pkg/src/API.jl

@@ -310,7 +310,7 @@ function gc(ctx::Context=Context(); period = Dates.Week(6), kwargs...)
         if isdir(packagedir)
             for name in readdir(packagedir)
                 name_path = joinpath(packagedir, name)
-                if isempty(readdir(name_path))
+                if isempty(collect(readdir(name_path)))
                     !ctx.preview && Base.rm(name_path)
                 end
             end

stdlib/Pkg/src/Operations.jl

@@ -396,7 +396,7 @@ function install_archive(
                 url_success = false
             end
             url_success || continue
-            dirs = readdir(dir)
+            dirs = collect(readdir(dir))
             # 7z on Win might create this spurious file
             filter!(x -> x != "pax_global_header", dirs)
             @assert length(dirs) == 1

stdlib/Pkg/src/Types.jl

@@ -488,7 +488,7 @@ mutable struct EnvCache
             project_file === nothing && error("package environment does not exist: $env")
         elseif env isa String
             if isdir(env)
-                isempty(readdir(env)) || error("environment is a package directory: $env")
+                isempty(collect(readdir(env))) || error("environment is a package directory: $env")
                 project_file = joinpath(env, Base.project_names[end])
             else
                 project_file = endswith(env, ".toml") ? abspath(env) :
@@ -1004,7 +1004,7 @@ const DEFAULT_REGISTRIES = Dict("Uncurated" => "https://github.com/JuliaRegistri
 function registries(depot::String)::Vector{String}
     d = joinpath(depot, "registries")
     ispath(d) || return String[]
-    regs = filter!(readdir(d)) do r
+    regs = filter!(collect(readdir(d))) do r
         isfile(joinpath(d, r, "Registry.toml"))
     end
     String[joinpath(depot, "registries", r) for r in regs]

test/choosetests.jl

@@ -3,7 +3,7 @@
 using Random, Sockets
 
 const STDLIB_DIR = joinpath(Sys.BINDIR, "..", "share", "julia", "stdlib", "v$(VERSION.major).$(VERSION.minor)")
-const STDLIBS = readdir(STDLIB_DIR)
+const STDLIBS = collect(readdir(STDLIB_DIR))
 
 """
 

test/file.jl

@@ -282,8 +282,8 @@ function check_dir(orig_path::AbstractString, copied_path::AbstractString, follo
     isdir(orig_path) || throw(ArgumentError("'$orig_path' is not a directory."))
     # copied_path must also be a dir.
     @test isdir(copied_path)
-    readir_orig = readdir(orig_path)
-    readir_copied = readdir(copied_path)
+    readir_orig = collect(readdir(orig_path))
+    readir_copied = collect(readdir(copied_path))
     @test readir_orig == readir_copied
     # check recursive
     for name in readir_orig
@@ -446,8 +446,8 @@ if !Sys.iswindows() || Sys.windows_version() >= Sys.WINDOWS_VISTA_VER
         islink(s) && @test readlink(s) == readlink(d)
         islink(s) && @test isabspath(readlink(s)) == isabspath(readlink(d))
         # all should contain 1 file named  "c.txt"
-        @test "c.txt" in readdir(d)
-        @test length(readdir(d)) == 1
+        @test "c.txt" in collect(readdir(d))
+        @test length(collect(readdir(d))) == 1
     end
 
     function mv_check(s, d, d_mv; force=true)
@@ -465,8 +465,8 @@ if !Sys.iswindows() || Sys.windows_version() >= Sys.WINDOWS_VISTA_VER
         islink(s) && @test readlink(s) == readlink(d_mv)
         islink(s) && @test isabspath(readlink(s)) == isabspath(readlink(d_mv))
         # all should contain 1 file named  "c.txt"
-        @test "c.txt" in readdir(d_mv)
-        @test length(readdir(d_mv)) == 1
+        @test "c.txt" in collect(readdir(d_mv))
+        @test length(collect(readdir(d_mv))) == 1
         # d => d_mv same file/dir
         @test Base.samefile(stat_d, stat_d_mv)
     end
@@ -509,7 +509,7 @@ if !Sys.iswindows() || Sys.windows_version() >= Sys.WINDOWS_VISTA_VER
             # Expect no link because a dir is copied (follow_symlinks=false does not effect this)
             @test isdir(d) && !islink(d)
             # none should contain any file
-            @test isempty(readdir(d))
+            @test isempty(collect(readdir(d)))
         end
     end
     # mv ----------------------------------------------------

test/spawn.jl

@@ -476,7 +476,7 @@ if Sys.isunix()
 end
 
 # readlines(::Cmd), accidentally broken in #20203
-@test sort(readlines(`$lscmd -A`)) == sort(readdir())
+@test sort(collect(readlines(`$lscmd -A`))) == sort(collect(readdir()))
 
 # issue #19864 (PR #20497)
 let c19864 = readchomp(pipeline(ignorestatus(