Attempt to close https://github.com/nim-lang/Nim/issues/11430

3 files changed, 57 insertions, 23 deletions

diff --git a/changelog.md b/changelog.md
index 87032fc0e..6feec8c04 100644
--- a/changelog.md
+++ b/changelog.md
@@ -3,6 +3,13 @@
 
 ### Changes affecting backwards compatibility
 
+- All `strutils.rfind` procs now take `start` and `last` like `strutils.find`
+  with the same data slice/index meaning.  This is backwards compatible for
+  calls *not* changing the `rfind` `start` parameter from its default.
+
+  In the unlikely case that you were using `rfind X, start=N`, or `rfind X, N`,
+  then you need to change that to `rfind X, last=N` or `rfind X, 0, N`. (This
+  should minimize gotchas porting code from other languages like Python or C++.)
 
 #### Breaking changes in the standard library
 
diff --git a/lib/pure/strutils.nim b/lib/pure/strutils.nim
index 114141e45..44ca6d98c 100644
--- a/lib/pure/strutils.nim
+++ b/lib/pure/strutils.nim
@@ -1850,6 +1850,8 @@ proc find*(s: string, sub: char, start: Natural = 0, last = 0): int {.noSideEffe
   ## If `last` is unspecified, it defaults to `s.high` (the last element).
   ##
   ## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned.
+  ## Otherwise the index returned is relative to ``s[0]``, not ``start``.
+  ## Use `s[start..last].rfind` for a ``start``-origin index.
   ##
   ## See also:
   ## * `rfind proc<#rfind,string,char,int>`_
@@ -1876,6 +1878,8 @@ proc find*(s: string, chars: set[char], start: Natural = 0, last = 0): int {.noS
   ## If `last` is unspecified, it defaults to `s.high` (the last element).
   ##
   ## If `s` contains none of the characters in `chars`, -1 is returned.
+  ## Otherwise the index returned is relative to ``s[0]``, not ``start``.
+  ## Use `s[start..last].find` for a ``start``-origin index.
   ##
   ## See also:
   ## * `rfind proc<#rfind,string,set[char],int>`_
@@ -1891,6 +1895,8 @@ proc find*(s, sub: string, start: Natural = 0, last = 0): int {.noSideEffect,
   ## If `last` is unspecified, it defaults to `s.high` (the last element).
   ##
   ## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned.
+  ## Otherwise the index returned is relative to ``s[0]``, not ``start``.
+  ## Use `s[start..last].find` for a ``start``-origin index.
   ##
   ## See also:
   ## * `rfind proc<#rfind,string,string,int>`_
@@ -1901,45 +1907,59 @@ proc find*(s, sub: string, start: Natural = 0, last = 0): int {.noSideEffect,
   initSkipTable(a, sub)
   result = find(a, s, sub, start, last)
 
-proc rfind*(s: string, sub: char, start: int = -1): int {.noSideEffect,
+proc rfind*(s: string, sub: char, start: Natural = 0, last = -1): int {.noSideEffect,
   rtl.} =
-  ## Searches for characer `sub` in `s` in reverse, starting at position `start`
-  ## (default: the last character) and going backwards to the first character.
+  ## Searches for `sub` in `s` inside range ``start..last`` (both ends included)
+  ## in reverse -- starting at high indexes and moving lower to the first
+  ## character or ``start``.  If `last` is unspecified, it defaults to `s.high`
+  ## (the last element).
   ##
   ## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned.
+  ## Otherwise the index returned is relative to ``s[0]``, not ``start``.
+  ## Use `s[start..last].find` for a ``start``-origin index.
   ##
   ## See also:
   ## * `find proc<#find,string,char,int,int>`_
-  let realStart = if start == -1: s.len-1 else: start
-  for i in countdown(realStart, 0):
+  let last = if last == -1: s.high else: last
+  for i in countdown(last, start):
     if sub == s[i]: return i
   return -1
 
-proc rfind*(s: string, chars: set[char], start: int = -1): int {.noSideEffect.} =
-  ## Searches for `chars` in `s` in reverse, starting at position `start`
-  ## (default: the last character) and going backwards to the first character.
+proc rfind*(s: string, chars: set[char], start: Natural = 0, last = -1): int {.noSideEffect,
+  rtl.} =
+  ## Searches for `chars` in `s` inside range ``start..last`` (both ends
+  ## included) in reverse -- starting at high indexes and moving lower to the
+  ## first character or ``start``.  If `last` is unspecified, it defaults to
+  ## `s.high` (the last element).
   ##
-  ## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned.
+  ## If `s` contains none of the characters in `chars`, -1 is returned.
+  ## Otherwise the index returned is relative to ``s[0]``, not ``start``.
+  ## Use `s[start..last].rfind` for a ``start``-origin index.
   ##
   ## See also:
   ## * `find proc<#find,string,set[char],Natural,int>`_
-  let realStart = if start == -1: s.len-1 else: start
-  for i in countdown(realStart, 0):
+  let last = if last == -1: s.high else: last
+  for i in countdown(last, start):
     if s[i] in chars: return i
   return -1
 
-proc rfind*(s, sub: string, start: int = -1): int {.noSideEffect.} =
-  ## Searches for string `sub` in `s` in reverse, starting at position `start`
-  ## (default: the last character) and going backwards to the first character.
+proc rfind*(s, sub: string, start: Natural = 0, last = -1): int {.noSideEffect,
+  rtl.} =
+  ## Searches for `sub` in `s` inside range ``start..last`` (both ends included)
+  ## included) in reverse -- starting at high indexes and moving lower to the
+  ## first character or ``start``.   If `last` is unspecified, it defaults to
+  ## `s.high` (the last element).
   ##
   ## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned.
+  ## Otherwise the index returned is relative to ``s[0]``, not ``start``.
+  ## Use `s[start..last].rfind` for a ``start``-origin index.
   ##
   ## See also:
   ## * `find proc<#find,string,string,Natural,int>`_
   if sub.len == 0:
     return -1
-  let realStart = if start == -1: s.len else: start
-  for i in countdown(realStart-sub.len, 0):
+  let last = if last == -1: s.high else: last
+  for i in countdown(last - sub.len + 1, start):
     for j in 0..sub.len-1:
       result = i
       if sub[j] != s[i+j]:
diff --git a/tests/stdlib/tstrutil.nim b/tests/stdlib/tstrutil.nim
index f83020504..304c2c325 100644
--- a/tests/stdlib/tstrutil.nim
+++ b/tests/stdlib/tstrutil.nim
@@ -189,14 +189,21 @@ proc testFind =
 
 proc testRFind =
   assert "0123456789ABCDEFGAH".rfind('A') == 17
-  assert "0123456789ABCDEFGAH".rfind('A', 13) == 10
-  assert "0123456789ABCDEFGAH".rfind('H', 13) == -1
+  assert "0123456789ABCDEFGAH".rfind('A', last=13) == 10
+  assert "0123456789ABCDEFGAH".rfind('H', last=13) == -1
   assert "0123456789ABCDEFGAH".rfind("A") == 17
-  assert "0123456789ABCDEFGAH".rfind("A", 13) == 10
-  assert "0123456789ABCDEFGAH".rfind("H", 13) == -1
+  assert "0123456789ABCDEFGAH".rfind("A", last=13) == 10
+  assert "0123456789ABCDEFGAH".rfind("H", last=13) == -1
   assert "0123456789ABCDEFGAH".rfind({'A'..'C'}) == 17
-  assert "0123456789ABCDEFGAH".rfind({'A'..'C'}, 13) == 12
-  assert "0123456789ABCDEFGAH".rfind({'G'..'H'}, 13) == -1
+  assert "0123456789ABCDEFGAH".rfind({'A'..'C'}, last=13) == 12
+  assert "0123456789ABCDEFGAH".rfind({'G'..'H'}, last=13) == -1
+  assert "0123456789ABCDEFGAH".rfind('A', start=18) == -1
+  assert "0123456789ABCDEFGAH".rfind('A', start=11, last=17) == 17
+  assert "0123456789ABCDEFGAH".rfind("0", start=0) == 0
+  assert "0123456789ABCDEFGAH".rfind("0", start=1) == -1
+  assert "0123456789ABCDEFGAH".rfind("H", start=11) == 18
+  assert "0123456789ABCDEFGAH".rfind({'0'..'9'}, start=5) == 9
+  assert "0123456789ABCDEFGAH".rfind({'0'..'9'}, start=10) == -1
 
 proc testSplitLines() =
   let fixture = "a\nb\rc\r\nd"
@@ -276,7 +283,7 @@ assert(editDistance("prefix__hallo_suffix", "prefix__hao_suffix") == 2)
 assert(editDistance("main", "malign") == 2)
 
 assert "/1/2/3".rfind('/') == 4
-assert "/1/2/3".rfind('/', 1) == 0
+assert "/1/2/3".rfind('/', last=1) == 0
 assert "/1/2/3".rfind('0') == -1
 
 assert(toHex(100i16, 32) == "00000000000000000000000000000064")