Skip to content

Commit

Permalink
make find(first|last|next|prev) return nothing rather than 0:-1
Browse files Browse the repository at this point in the history
deprecate Base extended methods that did not return nothing, in favor of Compat. versions
  • Loading branch information
fredrikekre committed Mar 9, 2018
1 parent fd3b92a commit 11ad15a
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 43 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,8 @@ Currently, the `@compat` macro supports the following syntaxes:
sometimes combined with `equalto` or `occursin` ([#24673]).

* `Compat.findfirst`, `Compat.findnext`, `Compat.findlast` and `Compat.findprev`,
return `nothing` when no match is found (rather than `0`) as on Julia 0.7 ([#24673]).
return `nothing` when no match is found (rather than `0` or `0:-1`)
as on Julia 0.7 ([#24673], [#26149]).

* `findin(a, b)` is now `findall(occursin(b), a)` ([#24673]).

Expand Down Expand Up @@ -586,4 +587,5 @@ includes this fix. Find the minimum version from there.
[#25990]: https://github.com/JuliaLang/julia/issues/25990
[#26069]: https://github.com/JuliaLang/julia/issues/26069
[#26089]: https://github.com/JuliaLang/julia/issues/26089
[#26149]: https://github.com/JuliaLang/julia/issues/26149
[#26156]: https://github.com/JuliaLang/julia/issues/26156
61 changes: 31 additions & 30 deletions src/Compat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1498,46 +1498,47 @@ else
export occursin

zero2nothing(x::Integer) = x == 0 ? nothing : x
zero2nothing(x::AbstractUnitRange{<:Integer}) = isempty(x) ? nothing : x
zero2nothing(x) = x

findnext(xs...) = zero2nothing(Base.findnext(xs...))
findfirst(xs...) = zero2nothing(Base.findfirst(xs...))
findprev(xs...) = zero2nothing(Base.findprev(xs...))
findlast(xs...) = zero2nothing(Base.findlast(xs...))

Base.findnext(r::Regex, s::AbstractString, idx::Integer) = search(s, r, idx)
Base.findfirst(r::Regex, s::AbstractString) = search(s, r)
Base.findnext(c::EqualTo{Char}, s::AbstractString, i::Integer) = search(s, c.x, i)
Base.findfirst(c::EqualTo{Char}, s::AbstractString) = search(s, c.x)
Base.findnext(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer) =
search(a, b.x, i)
Base.findfirst(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}) =
search(a, b.x)
findnext(r::Regex, s::AbstractString, idx::Integer) = zero2nothing(search(s, r, idx))
findfirst(r::Regex, s::AbstractString) = zero2nothing(search(s, r))
findnext(c::EqualTo{Char}, s::AbstractString, i::Integer) = zero2nothing(search(s, c.x, i))
findfirst(c::EqualTo{Char}, s::AbstractString) = zero2nothing(search(s, c.x))
findnext(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer) =
zero2nothing(search(a, b.x, i))
findfirst(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}) =
zero2nothing(search(a, b.x))

Base.findnext(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
findnext(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
s::AbstractString, i::Integer) =
search(s, c.x, i)
Base.findfirst(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
zero2nothing(search(s, c.x, i))
findfirst(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
s::AbstractString) =
search(s, c.x)
Base.findnext(t::AbstractString, s::AbstractString, i::Integer) = search(s, t, i)
Base.findfirst(t::AbstractString, s::AbstractString) = search(s, t)

Base.findfirst(delim::EqualTo{UInt8}, buf::Base.IOBuffer) = search(buf, delim.x)

Base.findprev(c::EqualTo{Char}, s::AbstractString, i::Integer) = rsearch(s, c.x, i)
Base.findlast(c::EqualTo{Char}, s::AbstractString) = rsearch(s, c.x)
Base.findprev(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer) =
rsearch(a, b.x, i)
Base.findlast(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}) =
rsearch(a, b.x)

Base.findprev(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
s::AbstractString, i::Integer) = rsearch(s, c.x, i)
Base.findlast(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
s::AbstractString) = rsearch(s, c.x)
Base.findprev(t::AbstractString, s::AbstractString, i::Integer) = rsearch(s, t, i)
Base.findlast(t::AbstractString, s::AbstractString) = rsearch(s, t)
zero2nothing(search(s, c.x))
findnext(t::AbstractString, s::AbstractString, i::Integer) = zero2nothing(search(s, t, i))
findfirst(t::AbstractString, s::AbstractString) = zero2nothing(search(s, t))

findfirst(delim::EqualTo{UInt8}, buf::Base.IOBuffer) = zero2nothing(search(buf, delim.x))

findprev(c::EqualTo{Char}, s::AbstractString, i::Integer) = zero2nothing(rsearch(s, c.x, i))
findlast(c::EqualTo{Char}, s::AbstractString) = zero2nothing(rsearch(s, c.x))
findprev(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer) =
zero2nothing(rsearch(a, b.x, i))
findlast(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}) =
zero2nothing(rsearch(a, b.x))

findprev(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
s::AbstractString, i::Integer) = zero2nothing(rsearch(s, c.x, i))
findlast(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}},
s::AbstractString) = zero2nothing(rsearch(s, c.x))
findprev(t::AbstractString, s::AbstractString, i::Integer) = zero2nothing(rsearch(s, t, i))
findlast(t::AbstractString, s::AbstractString) = zero2nothing(rsearch(s, t))

findall(b::OccursIn, a) = findin(a, b.x)
# To fix ambiguity
Expand Down
98 changes: 98 additions & 0 deletions src/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,101 @@ if VERSION < v"0.7.0-DEV.2915"
const textwidth = Compat.Unicode.textwidth
export textwidth
end

@static if VERSION < v"0.7.0-DEV.3272"
function Base.findnext(r::Regex, s::AbstractString, idx::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findnext(r::Regex, s::AbstractString, idx::Integer) is deprecated, ",
"use Compat.findnext(r, s, idx) instead, which returns `nothing` to indicate no match."), :findnext)
search(s, r, idx)
end
function Base.findfirst(r::Regex, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findfirst(r::Regex, s::AbstractString) is deprecated, ",
"use Compat.findfirst(r, s) instead, which returns `nothing` to indicate no match."), :findfirst)
search(s, r)
end
function Base.findnext(c::EqualTo{Char}, s::AbstractString, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findnext(c::EqualTo{Char}, s::AbstractString, i::Integer) is deprecated, ",
"use Compat.findnext(c, s, i) instead, which returns `nothing` to indicate no match."), :findnext)
search(s, c.x, i)
end
function Base.findfirst(c::EqualTo{Char}, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findfirst(c::EqualTo{Char}, s::AbstractString) is deprecated, ",
"use Compat.findfirst(c, s) instead, which returns `nothing` to indicate no match."), :findfirst)
search(s, c.x)
end
function Base.findnext(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findnext(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer) is deprecated, ",
"use Compat.findnext(b, a, i) instead, which returns `nothing` to indicate no match."), :findnext)
search(a, b.x, i)
end
function Base.findfirst(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}})
Base.depwarn(string("Compat.jl's implementation of Base.findfirst(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}) is deprecated, ",
"use Compat.findfirst(b, a) instead, which returns `nothing` to indicate no match."), :findfirst)
search(a, b.x)
end
function Base.findnext(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findnext(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString, i::Integer) is deprecated, ",
"use Compat.findnext(c, s, i) instead, which returns `nothing` to indicate no match."), :findnext)
search(s, c.x, i)
end
function Base.findfirst(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findfirst(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString) is deprecated, ",
"use Compat.findfirst(c, s) instead, which returns `nothing` to indicate no match."), :findfirst)
search(s, c.x)
end
function Base.findnext(t::AbstractString, s::AbstractString, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findnext(t::AbstractString, s::AbstractString, i::Integer) is deprecated, ",
"use Compat.findnext(t, s, i) instead, which returns `nothing` to indicate no match."), :findnext)
search(s, t, i)
end
function Base.findfirst(t::AbstractString, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findfirst(t::AbstractString, s::AbstractString) is deprecated, ",
"use Compat.findfirst(t, s) instead, which returns `nothing` to indicate no match."), :findfirst)
search(s, t)
end
function Base.findfirst(delim::EqualTo{UInt8}, buf::Base.IOBuffer)
Base.depwarn(string("Compat.jl's implementation of Base.findfirst(delim::EqualTo{UInt8}, buf::Base.IOBuffer) is deprecated, ",
"use Compat.findfirst(delim, buf) instead, which returns `nothing` to indicate no match."), :findfirst)
search(buf, delim.x)
end
function Base.findprev(c::EqualTo{Char}, s::AbstractString, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findprev(c::EqualTo{Char}, s::AbstractString, i::Integer) is deprecated, ",
"use Compat.findprev(c, s, i) instead, which returns `nothing` to indicate no match."), :findprev)
rsearch(s, c.x, i)
end
function Base.findlast(c::EqualTo{Char}, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findlast(c::EqualTo{Char}, s::AbstractString) is deprecated, ",
"use Compat.findlast(c, s) instead, which returns `nothing` to indicate no match."), :findlast)
rsearch(s, c.x)
end
function Base.findprev(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findprev(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}, i::Integer) is deprecated, ",
"use Compat.findprev(b, a, i) instead, which returns `nothing` to indicate no match."), :findprev)
rsearch(a, b.x, i)
end
function Base.findlast(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}})
Base.depwarn(string("Compat.jl's implementation of Base.findlast(b::EqualTo{<:Union{Int8,UInt8}}, a::Vector{<:Union{Int8,UInt8}}) is deprecated, ",
"use Compat.findlast(b, a) instead, which returns `nothing` to indicate no match."), :findlast)
rsearch(a, b.x)
end
function Base.findprev(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findprev(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString, i::Integer) is deprecated, ",
"use Compat.findprev(c, s, i) instead, which returns `nothing` to indicate no match."), :findprev)
rsearch(s, c.x, i)
end
function Base.findlast(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findlast(c::OccursIn{<:Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}}, s::AbstractString) is deprecated, ",
"use Compat.findlast(c, s) instead, which returns `nothing` to indicate no match."), :findlast)
rsearch(s, c.x)
end
function Base.findprev(t::AbstractString, s::AbstractString, i::Integer)
Base.depwarn(string("Compat.jl's implementation of Base.findprev(t::AbstractString, s::AbstractString, i::Integer) is deprecated, ",
"use Compat.findprev(t, s, i) instead, which returns `nothing` to indicate no match."), :findprev)
rsearch(s, t, i)
end
function Base.findlast(t::AbstractString, s::AbstractString)
Base.depwarn(string("Compat.jl's implementation of Base.findlast(t::AbstractString, s::AbstractString) is deprecated, ",
"use Compat.findlast(t, s) instead, which returns `nothing` to indicate no match."), :findlast)
rsearch(s, t)
end
end
20 changes: 8 additions & 12 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1378,18 +1378,14 @@ for (f1, f2, i) in ((Compat.findfirst, Compat.findnext, 1),
@test f2(occursin(chars), "bx", i) == f1(occursin(chars), "bx") == nothing
end
end
for (f1, f2, i) in ((findfirst, findnext, 1),
(findlast, findprev, 2),
(Compat.findfirst, Compat.findnext, 1),
(Compat.findlast, Compat.findprev, 2))
@test f2("a", "ba", i) == f1("a", "ba") == 2:2
@test f2("z", "ba", i) == f1("z", "ba") == 0:-1
end
for (f1, f2, i) in ((findfirst, findnext, 1),
(Compat.findfirst, Compat.findnext, 1))
@test f2(r"a", "ba", 1) == f1(r"a", "ba") == 2:2
@test f2(r"z", "ba", 1) == f1(r"z", "ba") == 0:-1
end

@test Compat.findnext("a", "ba", 1) == Compat.findfirst("a", "ba") == 2:2
@test Compat.findnext("z", "ba", 1) == Compat.findfirst("z", "ba") == nothing
@test Compat.findprev("a", "ba", 2) == Compat.findlast("a", "ba") == 2:2
@test Compat.findprev("z", "ba", 2) == Compat.findlast("z", "ba") == nothing

@test Compat.findnext(r"a", "ba", 1) == Compat.findfirst(r"a", "ba") == 2:2
@test Compat.findnext(r"z", "ba", 1) == Compat.findfirst(r"z", "ba") == nothing

@test Compat.findfirst(equalto(UInt8(0)), IOBuffer(UInt8[1, 0])) == 2
@test Compat.findfirst(equalto(UInt8(9)), IOBuffer(UInt8[1, 0])) == nothing
Expand Down

0 comments on commit 11ad15a

Please sign in to comment.