make replace with count=0 a no-op (#22325)

Author: rfourquet

NEWS.md

@@ -127,6 +127,9 @@ Deprecated or removed
     Ternaries must now include some amount of whitespace, e.g. `x ? a : b` rather than
     `x? a : b` ([#22523]).
 
+  * The method `replace(s::AbstractString, pat, r, count)` with `count <= 0` is deprecated
+    in favor of `replace(s::AbstractString, pat, r, typemax(Int))` ([#22325]).
+
 
 Julia v0.6.0 Release Notes
 ==========================

base/deprecated.jl

@@ -1503,6 +1503,22 @@ function bkfact!(A::StridedMatrix, uplo::Symbol, symmetric::Bool = issymmetric(A
     return bkfact!(symmetric ? Symmetric(A, uplo) : Hermitian(A, uplo), rook)
 end
 
+# PR #22325
+# TODO: when this replace is removed from deprecated.jl:
+# 1) rename the function replace_new from strings/util.jl to replace
+# 2) update the replace(s::AbstractString, pat, f) method, below replace_new
+#    (see instructions there)
+function replace(s::AbstractString, pat, f, n::Integer)
+    if n <= 0
+        depwarn(string("`replace(s, pat, r, count)` with `count <= 0` is deprecated, use ",
+                       "`replace(s, pat, r, typemax(Int))` or `replace(s, pat, r)` instead"),
+                :replace)
+        replace(s, pat, f)
+    else
+        replace_new(String(s), pat, f, n)
+    end
+end
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

base/strings/util.jl

@@ -359,7 +359,10 @@ _replace(io, repl, str, r, pattern) = print(io, repl)
 _replace(io, repl::Function, str, r, pattern) =
     print(io, repl(SubString(str, first(r), last(r))))
 
-function replace(str::String, pattern, repl, limit::Integer)
+# TODO: rename to `replace` when `replace` is removed from deprecated.jl
+function replace_new(str::String, pattern, repl, count::Integer)
+    count == 0 && return str
+    count < 0 && throw(DomainError())
     n = 1
     e = endof(str)
     i = a = start(str)
@@ -384,25 +387,29 @@ function replace(str::String, pattern, repl, limit::Integer)
         end
         r = search(str,pattern,k)
         j, k = first(r), last(r)
-        n == limit && break
+        n == count && break
         n += 1
     end
     write(out, SubString(str,i))
     String(take!(out))
 end
 
 """
-    replace(string::AbstractString, pat, r[, n::Integer=0])
+    replace(s::AbstractString, pat, r, [count::Integer])
 
-Search for the given pattern `pat`, and replace each occurrence with `r`. If `n` is
-provided, replace at most `n` occurrences. As with search, the second argument may be a
+Search for the given pattern `pat` in `s`, and replace each occurrence with `r`.
+If `count` is provided, replace at most `count` occurrences.
+As with [`search`](@ref), the second argument may be a
 single character, a vector or a set of characters, a string, or a regular expression. If `r`
 is a function, each occurrence is replaced with `r(s)` where `s` is the matched substring.
 If `pat` is a regular expression and `r` is a `SubstitutionString`, then capture group
 references in `r` are replaced with the corresponding matched text.
 """
-replace(s::AbstractString, pat, f, n::Integer) = replace(String(s), pat, f, n)
-replace(s::AbstractString, pat, r) = replace(s, pat, r, 0)
+replace(s::AbstractString, pat, f) = replace_new(String(s), pat, f, typemax(Int))
+# TODO: change this to the following when `replace` is removed from deprecated.jl:
+# replace(s::AbstractString, pat, f, count::Integer=typemax(Int)) =
+#     replace(String(s), pat, f, count)
+
 
 # hex <-> bytes conversion
 

test/strings/util.jl

@@ -208,6 +208,18 @@ end
 # Issue 13332
 @test replace("abc", 'b', 2.1) == "a2.1c"
 
+# test replace with a count for String and GenericString
+# check that replace is a no-op if count==0
+for s in ["aaa", Base.Test.GenericString("aaa")]
+    # @test replace("aaa", 'a', 'z', 0) == "aaa" # enable when undeprecated
+    @test replace(s, 'a', 'z', 1) == "zaa"
+    @test replace(s, 'a', 'z', 2) == "zza"
+    @test replace(s, 'a', 'z', 3) == "zzz"
+    @test replace(s, 'a', 'z', 4) == "zzz"
+    @test replace(s, 'a', 'z', typemax(Int)) == "zzz"
+    @test replace(s, 'a', 'z')    == "zzz"
+end
+
 # chomp/chop
 @test chomp("foo\n") == "foo"
 @test chop("fooε") == "foo"