Bugs in search -> findfirst deprecation and Compat

[tbole]

In 0.7 search has been deprecated in favor of findfirst / findlast, however these behave differently than search in some cases, e.g. if a value is not found.

julia> search("x",'y')   # returns 0 in 0.6, nothing in 0.7
0

Another error occurs with 0.6 and Compat when using Compat.findfirst and passing Unicode strings.

julia> Compat.findfirst("uniçº∂e",'x')  # works on 0.7, fails on 0.6
ERROR: UnicodeError: invalid character index 5 (0xa7 is a continuation byte)
Stacktrace:
 [1] slow_utf8_next(::Ptr{UInt8}, ::UInt8, ::Int64, ::Int64) at ./strings/string.jl:172
 [2] next at ./strings/string.jl:204 [inlined]
 [3] getindex(::String, ::Int64) at ./strings/basic.jl:32
 [4] findnext(::String, ::Char, ::Int64) at ./array.jl:1262
 [5] findfirst(::String, ::Char) at ./array.jl:1288
 [6] findfirst(::String, ::Vararg{Any,N} where N) at /home/tbole/.julia/v0.6/Compat/src/Compat.jl:1589

[nalimilan]

The 0 vs nothing issue is handled via the unexported Compat.findfirst method, which returns nothing in both 0.6 and 0.7. It's not possible to provide a consistent behavior across Julia versions with the exported function without breaking existing code. See JuliaLang/Compat.jl#484 (comment).

The second error corresponds to an invalid call: you're trying to look for a string inside a character, which doesn't make sense. Try swapping the arguments and using equalto('x') instead of x. Anyway on 0.7 it prints a deprecation warning already and it won't be supported in 1.0.

So AFAICT there's no bug here.

[tbole]

I am aware that one should use Compat.findfirst, but was pointing to search.
Deprecating search in 0.7, while at the same time changing its behavior is not very intuitive. IMHO it would make more sense to either completely drop search in 0.7, or to make it behave identically as in 0.6, i.e. return 0 instead of nothing. This is what I would expect from a deprecation, not breaking existing code. The current behavior is likely to trigger unexpected runtime errors, which could be avoided with a nothingtozero conversion similar to the opposite Compat.

The second error is my fault, but originates from the deprecation of findfirst(A, v) in 0.7 in favor of findfirst(equalto(v), A), while Compat 0.6 only supports the newest version.

[mbauman]

I agree with tbole — I think this can be improved. Changing names is a great opportunity to provide non-breaking API changes. This deprecation could be:

julia> search("x",'y')
┌ Warning: `search(s::AbstractString, c::Char)` is deprecated, use `i = findfirst(equalto(c), s); i === nothing ? 0 : i` instead.
│   caller = top-level scope
└ @ Core :0
0

Ok, that's a mouthful, but it jives with our practice of using deprecations to provide non-breaking functionality until the code is updated.

[nalimilan]

I am aware that one should use Compat.findfirst, but was pointing to search.
Deprecating search in 0.7, while at the same time changing its behavior is not very intuitive. IMHO it would make more sense to either completely drop search in 0.7, or to make it behave identically as in 0.6, i.e. return 0 instead of nothing. This is what I would expect from a deprecation, not breaking existing code. The current behavior is likely to trigger unexpected runtime errors, which could be avoided with a nothingtozero conversion similar to the opposite Compat.

Ah, good catch. See #26149.