Skip to content

Commit

Permalink
Change find* methods to return nothing instead of than empty ranges f…
Browse files Browse the repository at this point in the history
…or no match

This is more consistent with functions which return a single index. It also allows
distinguishing no match from an empty match.
  • Loading branch information
nalimilan committed Feb 21, 2018
1 parent abf8be3 commit d6fde5a
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 85 deletions.
5 changes: 3 additions & 2 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,9 @@ This section lists changes that do not have deprecation warnings.
and higher-dimensional arrays insted of linear indices as was previously the case.
Use `LinearIndices(a)[findall(f, a)]` and similar constructs to compute linear indices.

* The `find*` functions which return scalars, i.e. `findnext`, `findprev`, `findfirst`,
* The `find*` functions, i.e. `findnext`, `findprev`, `findfirst`,
and `findlast`, as well as `indexin`, now return `nothing` when no match is found rather
than 0 ([#25472], [#25662]).
than `0` or `0:-1` ([#25472], [#25662], [#26149])

* The `Base.HasShape` iterator trait has gained a type parameter `N` indicating the
number of dimensions, which must correspond to the length of the tuple returned by
Expand Down Expand Up @@ -1359,3 +1359,4 @@ Command-line option changes
[#25990]: https://github.com/JuliaLang/julia/issues/25990
[#25998]: https://github.com/JuliaLang/julia/issues/25998
[#26009]: https://github.com/JuliaLang/julia/issues/26009
[#26149]: https://github.com/JuliaLang/julia/issues/26149
30 changes: 15 additions & 15 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1193,9 +1193,9 @@ end
@deprecate_binding HasOrder Ordered
@deprecate_binding ArithmeticOverflows ArithmeticWraps

@deprecate search(str::Union{String,SubString}, re::Regex, idx::Integer) findnext(re, str, idx)
@deprecate search(s::AbstractString, r::Regex, idx::Integer) findnext(r, s, idx)
@deprecate search(s::AbstractString, r::Regex) findfirst(r, s)
@deprecate search(str::Union{String,SubString}, re::Regex, idx::Integer) coalesce(findnext(re, str, idx), 0:-1)
@deprecate search(s::AbstractString, r::Regex, idx::Integer) coalesce(findnext(r, s, idx), 0:-1)
@deprecate search(s::AbstractString, r::Regex) coalesce(findfirst(r, s), 0:-1)
@deprecate search(s::AbstractString, c::Char, i::Integer) coalesce(findnext(equalto(c), s, i), 0)
@deprecate search(s::AbstractString, c::Char) coalesce(findfirst(equalto(c), s), 0)
@deprecate search(a::ByteArray, b::Union{Int8,UInt8}, i::Integer) coalesce(findnext(equalto(b), a, i), 0)
Expand All @@ -1207,31 +1207,31 @@ end

@deprecate search(s::AbstractString, c::Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}, i::Integer) coalesce(findnext(occursin(c), s, i), 0)
@deprecate search(s::AbstractString, c::Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}) coalesce(findfirst(occursin(c), s), 0)
@deprecate search(s::AbstractString, t::AbstractString, i::Integer) findnext(t, s, i)
@deprecate search(s::AbstractString, t::AbstractString) findfirst(t, s)
@deprecate search(s::AbstractString, t::AbstractString, i::Integer) coalesce(findnext(t, s, i), 0:-1)
@deprecate search(s::AbstractString, t::AbstractString) coalesce(findfirst(t, s), 0:-1)

@deprecate search(buf::IOBuffer, delim::UInt8) coalesce(findfirst(equalto(delim), buf), 0)
@deprecate search(buf::Base.GenericIOBuffer, delim::UInt8) coalesce(findfirst(equalto(delim), buf), 0)

@deprecate rsearch(s::AbstractString, c::Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}, i::Integer) coalesce(findprev(occursin(c), s, i), 0)
@deprecate rsearch(s::AbstractString, c::Union{Tuple{Vararg{Char}},AbstractVector{Char},Set{Char}}) coalesce(findlast(occursin(c), s), 0)
@deprecate rsearch(s::AbstractString, t::AbstractString, i::Integer) findprev(t, s, i)
@deprecate rsearch(s::AbstractString, t::AbstractString) findlast(t, s)
@deprecate rsearch(s::AbstractString, t::AbstractString, i::Integer) coalesce(findprev(t, s, i), 0:-1)
@deprecate rsearch(s::AbstractString, t::AbstractString) coalesce(findlast(t, s), 0:-1)

@deprecate rsearch(str::Union{String,SubString}, re::Regex, idx::Integer) findprev(re, str, idx)
@deprecate rsearch(str::Union{String,SubString}, re::Regex) findlast(re, str)
@deprecate rsearch(s::AbstractString, r::Regex, idx::Integer) findprev(r, s, idx)
@deprecate rsearch(s::AbstractString, r::Regex) findlast(r, s)
@deprecate rsearch(str::Union{String,SubString}, re::Regex, idx::Integer) coalesce(findprev(re, str, idx), 0:-1)
@deprecate rsearch(str::Union{String,SubString}, re::Regex) coalesce(findlast(re, str), 0:-1)
@deprecate rsearch(s::AbstractString, r::Regex, idx::Integer) coalesce(findprev(r, s, idx), 0:-1)
@deprecate rsearch(s::AbstractString, r::Regex) coalesce(findlast(r, s), 0:-1)
@deprecate rsearch(s::AbstractString, c::Char, i::Integer) coalesce(findprev(equalto(c), s, i), 0)
@deprecate rsearch(s::AbstractString, c::Char) coalesce(findlast(equalto(c), s), 0)
@deprecate rsearch(a::Union{String,ByteArray}, b::Union{Int8,UInt8}, i::Integer = lastindex(a)) coalesce(findprev(equalto(b), a, i), 0)
@deprecate rsearch(a::String, b::Union{Int8,UInt8}, i::Integer = lastindex(a)) coalesce(findprev(equalto(Char(b)), a, i), 0)
@deprecate rsearch(a::ByteArray, b::Char, i::Integer = lastindex(a)) coalesce(findprev(equalto(UInt8(b)), a, i), 0)

@deprecate searchindex(s::AbstractString, t::AbstractString) first(findfirst(t, s))
@deprecate searchindex(s::AbstractString, t::AbstractString, i::Integer) first(findnext(t, s, i))
@deprecate rsearchindex(s::AbstractString, t::AbstractString) first(findlast(t, s))
@deprecate rsearchindex(s::AbstractString, t::AbstractString, i::Integer) first(findprev(t, s, i))
@deprecate searchindex(s::AbstractString, t::AbstractString) first(coalesce(findfirst(t, s), 0:-1))
@deprecate searchindex(s::AbstractString, t::AbstractString, i::Integer) first(coalesce(findnext(t, s, i), 0:-1))
@deprecate rsearchindex(s::AbstractString, t::AbstractString) first(coalesce(findlast(t, s), 0:-1))
@deprecate rsearchindex(s::AbstractString, t::AbstractString, i::Integer) first(coalesce(findprev(t, s, i), 0:-1))

@deprecate searchindex(s::AbstractString, c::Char) coalesce(findfirst(equalto(c), s), 0)
@deprecate searchindex(s::AbstractString, c::Char, i::Integer) coalesce(findnext(equalto(c), s, i), 0)
Expand Down
7 changes: 5 additions & 2 deletions base/regex.jl
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,11 @@ function findnext(re::Regex, str::Union{String,SubString}, idx::Integer)
end
opts = re.match_options
compile(re)
PCRE.exec(re.regex, str, idx-1, opts, re.match_data) ?
((Int(re.ovec[1])+1):prevind(str,Int(re.ovec[2])+1)) : (0:-1)
if PCRE.exec(re.regex, str, idx-1, opts, re.match_data)
(Int(re.ovec[1])+1):prevind(str,Int(re.ovec[2])+1)
else
nothing
end
end
findnext(r::Regex, s::AbstractString, idx::Integer) = throw(ArgumentError(
"regex search is only available for the String type; use String(s) to convert"
Expand Down
24 changes: 14 additions & 10 deletions base/strings/search.jl
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,10 @@ function _search(s::Union{AbstractString,ByteArray},
idx = _searchindex(s,t,i)
if isempty(t)
idx:idx-1
elseif idx > 0
idx:(idx + lastindex(t) - 1)
else
idx:(idx > 0 ? idx + lastindex(t) - 1 : -1)
nothing
end
end

Expand All @@ -230,16 +232,16 @@ Find the next occurrence of `pattern` in `string` starting at position `start`.
`pattern` can be either a string, or a regular expression, in which case `string`
must be of type `String`.
The return value is a range of indexes where the matching sequence is found, such that
The return value is a range of indices where the matching sequence is found, such that
`s[findnext(x, s, i)] == x`:
`findnext("substring", string, i)` = `start:end` such that
`string[start:end] == "substring"`, or `0:-1` if unmatched.
`string[start:end] == "substring"`, or `nothing` if unmatched.
# Examples
```jldoctest
julia> findnext("z", "Hello to the world", 1)
0:-1
julia> findnext("z", "Hello to the world", 1) === nothing
true
julia> findnext("o", "Hello to the world", 6)
8:8
Expand Down Expand Up @@ -392,8 +394,10 @@ function _rsearch(s::Union{AbstractString,ByteArray},
idx = _rsearchindex(s,t,i)
if isempty(t)
idx:idx-1
elseif idx > 0
idx:(idx + lastindex(t) - 1)
else
idx:(idx > 0 ? idx + lastindex(t) - 1 : -1)
nothing
end
end

Expand All @@ -405,16 +409,16 @@ Find the previous occurrence of `pattern` in `string` starting at position `star
`pattern` can be either a string, or a regular expression, in which case `string`
must be of type `String`.
The return value is a range of indexes where the matching sequence is found, such that
The return value is a range of indices where the matching sequence is found, such that
`s[findprev(x, s, i)] == x`:
`findprev("substring", string, i)` = `start:end` such that
`string[start:end] == "substring"`, or `0:-1` if unmatched.
`string[start:end] == "substring"`, or `nothing` if unmatched.
# Examples
```jldoctest
julia> findprev("z", "Hello to the world", 18)
0:-1
julia> findprev("z", "Hello to the world", 18) === nothing
true
julia> findprev("o", "Hello to the world", 18)
15:15
Expand Down
4 changes: 2 additions & 2 deletions stdlib/REPL/src/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ function history_search(hist::REPLHistoryProvider, query_buffer::IOBuffer, respo
# First the current response buffer
if 1 <= searchstart <= lastindex(response_str)
match = searchfunc2(searchdata, response_str, searchstart)
if match != 0:-1
if match !== nothing
seek(response_buffer, first(match) - 1)
return true
end
Expand All @@ -642,7 +642,7 @@ function history_search(hist::REPLHistoryProvider, query_buffer::IOBuffer, respo
for idx in idxs
h = hist.history[idx]
match = searchfunc1(searchdata, h)
if match != 0:-1 && h != response_str && haskey(hist.mode_mapping, hist.modes[idx])
if match !== nothing && h != response_str && haskey(hist.mode_mapping, hist.modes[idx])
truncate(response_buffer, 0)
write(response_buffer, h)
seek(response_buffer, first(match) - 1)
Expand Down
2 changes: 1 addition & 1 deletion stdlib/REPL/src/REPLCompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ function afterusing(string::String, startpos::Int)
isempty(str) && return false
rstr = reverse(str)
r = findfirst(r"\s(gnisu|tropmi)\b", rstr)
isempty(r) && return false
r === nothing && return false
fr = reverseind(str, last(r))
return contains(str[fr:end], r"^\b(using|import)\s*((\w+[.])*\w+\s*,\s*)*$")
end
Expand Down
2 changes: 1 addition & 1 deletion stdlib/REPL/src/docview.jl
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ doc_completions(name::Symbol) = doc_completions(string(name))
# Searching and apropos

# Docsearch simply returns true or false if an object contains the given needle
docsearch(haystack::AbstractString, needle) = !isempty(findfirst(needle, haystack))
docsearch(haystack::AbstractString, needle) = findfirst(needle, haystack) !== nothing
docsearch(haystack::Symbol, needle) = docsearch(string(haystack), needle)
docsearch(::Nothing, needle) = false
function docsearch(haystack::Array, needle)
Expand Down
4 changes: 2 additions & 2 deletions stdlib/REPL/test/repl.jl
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,10 @@ fake_repl() do stdin_write, stdout_read, repl
# Issue #10222
# Test ignoring insert key in standard and prefix search modes
write(stdin_write, "\e[2h\e[2h\n") # insert (VT100-style)
@test findfirst("[2h", readline(stdout_read)) == 0:-1
@test findfirst("[2h", readline(stdout_read)) === nothing
readline(stdout_read)
write(stdin_write, "\e[2~\e[2~\n") # insert (VT220-style)
@test findfirst("[2~", readline(stdout_read)) == 0:-1
@test findfirst("[2~", readline(stdout_read)) === nothing
readline(stdout_read)
write(stdin_write, "1+1\n") # populate history with a trivial input
readline(stdout_read)
Expand Down
98 changes: 49 additions & 49 deletions test/strings/search.jl
Original file line number Diff line number Diff line change
Expand Up @@ -115,83 +115,83 @@ for str in [u8str]
end

# string forward search with a single-char string
@test findfirst("x", astr) == 0:-1
@test findfirst("x", astr) == nothing
@test findfirst("H", astr) == 1:1
@test findnext("H", astr, 2) == 0:-1
@test findnext("H", astr, 2) == nothing
@test findfirst("l", astr) == 3:3
@test findnext("l", astr, 4) == 4:4
@test findnext("l", astr, 5) == 11:11
@test findnext("l", astr, 12) == 0:-1
@test findnext("l", astr, 12) == nothing
@test findfirst("\n", astr) == 14:14
@test findnext("\n", astr, 15) == 0:-1
@test findnext("\n", astr, 15) == nothing

@test findfirst("z", u8str) == 0:-1
@test findfirst("", u8str) == 0:-1
@test findfirst("z", u8str) == nothing
@test findfirst("", u8str) == nothing
@test findfirst("", u8str) == 1:1
@test findnext("", u8str, 4) == 0:-1
@test findnext("", u8str, 4) == nothing
@test findfirst("", u8str) == 13:13
@test findnext("", u8str, 16) == 0:-1
@test findnext("", u8str, 16) == nothing
@test findfirst("x", u8str) == 26:26
@test findnext("x", u8str, 27) == 43:43
@test findnext("x", u8str, 44) == 0:-1
@test findnext("x", u8str, 44) == nothing
@test findfirst("ε", u8str) == 5:5
@test findnext("ε", u8str, 7) == 54:54
@test findnext("ε", u8str, 56) == 0:-1
@test findnext("ε", u8str, 56) == nothing

# strifindprev backward search with a single-char string
@test findlast("x", astr) == 0:-1
@test findlast("x", astr) == nothing
@test findlast("H", astr) == 1:1
@test findprev("H", astr, 2) == 1:1
@test findprev("H", astr, 0) == 0:-1
@test findprev("H", astr, 0) == nothing
@test findlast("l", astr) == 11:11
@test findprev("l", astr, 10) == 4:4
@test findprev("l", astr, 4) == 4:4
@test findprev("l", astr, 3) == 3:3
@test findprev("l", astr, 2) == 0:-1
@test findprev("l", astr, 2) == nothing
@test findlast("\n", astr) == 14:14
@test findprev("\n", astr, 13) == 0:-1
@test findprev("\n", astr, 13) == nothing

@test findlast("z", u8str) == 0:-1
@test findlast("", u8str) == 0:-1
@test findlast("z", u8str) == nothing
@test findlast("", u8str) == nothing
@test findlast("", u8str) == 1:1
@test findprev("", u8str, 0) == 0:-1
@test findprev("", u8str, 0) == nothing
#TODO: setting the limit in the middle of a wide char
# makes findnext fail but findprev succeed.
# Should findprev fail as well?
#@test findprev("∀", u8str, 2) == 0:-1 # gives 1:3
#@test findprev("∀", u8str, 2) == nothing # gives 1:3
@test findlast("", u8str) == 13:13
@test findprev("", u8str, 12) == 0:-1
@test findprev("", u8str, 12) == nothing
@test findlast("x", u8str) == 43:43
@test findprev("x", u8str, 42) == 26:26
@test findprev("x", u8str, 25) == 0:-1
@test findprev("x", u8str, 25) == nothing
@test findlast("ε", u8str) == 54:54
@test findprev("ε", u8str, 53) == 5:5
@test findprev("ε", u8str, 4) == 0:-1
@test findprev("ε", u8str, 4) == nothing

# string forward search with a single-char regex
@test findfirst(r"x", astr) == 0:-1
@test findfirst(r"x", astr) == nothing
@test findfirst(r"H", astr) == 1:1
@test findnext(r"H", astr, 2) == 0:-1
@test findnext(r"H", astr, 2) == nothing
@test findfirst(r"l", astr) == 3:3
@test findnext(r"l", astr, 4) == 4:4
@test findnext(r"l", astr, 5) == 11:11
@test findnext(r"l", astr, 12) == 0:-1
@test findnext(r"l", astr, 12) == nothing
@test findfirst(r"\n", astr) == 14:14
@test findnext(r"\n", astr, 15) == 0:-1
@test findfirst(r"z", u8str) == 0:-1
@test findfirst(r"", u8str) == 0:-1
@test findnext(r"\n", astr, 15) == nothing
@test findfirst(r"z", u8str) == nothing
@test findfirst(r"", u8str) == nothing
@test findfirst(r"", u8str) == 1:1
@test findnext(r"", u8str, 4) == 0:-1
@test findnext(r"", u8str, 4) == nothing
@test findfirst(r"", u8str) == findfirst(r"\u2200", u8str)
@test findnext(r"", u8str, 4) == findnext(r"\u2200", u8str, 4)
@test findfirst(r"", u8str) == 13:13
@test findnext(r"", u8str, 16) == 0:-1
@test findnext(r"", u8str, 16) == nothing
@test findfirst(r"x", u8str) == 26:26
@test findnext(r"x", u8str, 27) == 43:43
@test findnext(r"x", u8str, 44) == 0:-1
@test findnext(r"x", u8str, 44) == nothing
@test findfirst(r"ε", u8str) == 5:5
@test findnext(r"ε", u8str, 7) == 54:54
@test findnext(r"ε", u8str, 56) == 0:-1
@test findnext(r"ε", u8str, 56) == nothing
for i = 1:lastindex(astr)
@test findnext(r"."s, astr, i) == i:i
end
Expand Down Expand Up @@ -231,18 +231,18 @@ for i = 1:lastindex(u8str)
end

# string forward search with a two-char string literal
@test findfirst("xx", "foo,bar,baz") == 0:-1
@test findfirst("xx", "foo,bar,baz") == nothing
@test findfirst("fo", "foo,bar,baz") == 1:2
@test findnext("fo", "foo,bar,baz", 3) == 0:-1
@test findnext("fo", "foo,bar,baz", 3) == nothing
@test findfirst("oo", "foo,bar,baz") == 2:3
@test findnext("oo", "foo,bar,baz", 4) == 0:-1
@test findnext("oo", "foo,bar,baz", 4) == nothing
@test findfirst("o,", "foo,bar,baz") == 3:4
@test findnext("o,", "foo,bar,baz", 5) == 0:-1
@test findnext("o,", "foo,bar,baz", 5) == nothing
@test findfirst(",b", "foo,bar,baz") == 4:5
@test findnext(",b", "foo,bar,baz", 6) == 8:9
@test findnext(",b", "foo,bar,baz", 10) == 0:-1
@test findnext(",b", "foo,bar,baz", 10) == nothing
@test findfirst("az", "foo,bar,baz") == 10:11
@test findnext("az", "foo,bar,baz", 12) == 0:-1
@test findnext("az", "foo,bar,baz", 12) == nothing

# issue #9365
# string forward search with a two-char UTF-8 (2 byte) string literal
Expand Down Expand Up @@ -286,32 +286,32 @@ end
@test findprev("\U1f596\U1f596", "\U1f596\U1f596", lastindex("\U1f596\U1f596\U1f596")) == 1:5

# string backward search with a two-char string literal
@test findlast("xx", "foo,bar,baz") == 0:-1
@test findlast("xx", "foo,bar,baz") == nothing
@test findlast("fo", "foo,bar,baz") == 1:2
@test findprev("fo", "foo,bar,baz", 1) == 0:-1
@test findprev("fo", "foo,bar,baz", 1) == nothing
@test findlast("oo", "foo,bar,baz") == 2:3
@test findprev("oo", "foo,bar,baz", 2) == 0:-1
@test findprev("oo", "foo,bar,baz", 2) == nothing
@test findlast("o,", "foo,bar,baz") == 3:4
@test findprev("o,", "foo,bar,baz", 1) == 0:-1
@test findprev("o,", "foo,bar,baz", 1) == nothing
@test findlast(",b", "foo,bar,baz") == 8:9
@test findprev(",b", "foo,bar,baz", 6) == 4:5
@test findprev(",b", "foo,bar,baz", 3) == 0:-1
@test findprev(",b", "foo,bar,baz", 3) == nothing
@test findlast("az", "foo,bar,baz") == 10:11
@test findprev("az", "foo,bar,baz", 10) == 0:-1
@test findprev("az", "foo,bar,baz", 10) == nothing

# string search with a two-char regex
@test findfirst(r"xx", "foo,bar,baz") == 0:-1
@test findfirst(r"xx", "foo,bar,baz") == nothing
@test findfirst(r"fo", "foo,bar,baz") == 1:2
@test findnext(r"fo", "foo,bar,baz", 3) == 0:-1
@test findnext(r"fo", "foo,bar,baz", 3) == nothing
@test findfirst(r"oo", "foo,bar,baz") == 2:3
@test findnext(r"oo", "foo,bar,baz", 4) == 0:-1
@test findnext(r"oo", "foo,bar,baz", 4) == nothing
@test findfirst(r"o,", "foo,bar,baz") == 3:4
@test findnext(r"o,", "foo,bar,baz", 5) == 0:-1
@test findnext(r"o,", "foo,bar,baz", 5) == nothing
@test findfirst(r",b", "foo,bar,baz") == 4:5
@test findnext(r",b", "foo,bar,baz", 6) == 8:9
@test findnext(r",b", "foo,bar,baz", 10) == 0:-1
@test findnext(r",b", "foo,bar,baz", 10) == nothing
@test findfirst(r"az", "foo,bar,baz") == 10:11
@test findnext(r"az", "foo,bar,baz", 12) == 0:-1
@test findnext(r"az", "foo,bar,baz", 12) == nothing

# contains with a String and Char needle
@test contains("foo", "o")
Expand Down
2 changes: 1 addition & 1 deletion test/strings/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ end
# search and SubString (issue #5679)
let str = "Hello, world!"
u = SubString(str, 1, 5)
@test findlast("World", u) == 0:-1
@test findlast("World", u) == nothing
@test findlast(equalto('z'), u) == nothing
@test findlast("ll", u) == 3:4
end
Expand Down

0 comments on commit d6fde5a

Please sign in to comment.