Skip to content

Commit

Permalink
Fix offset handling of String#rindex with Regex (crystal-lang#5594)
Browse files Browse the repository at this point in the history
* Fix offset handling of String#rindex with Regex

This also addas a few specs to ensure all variants of #rindex treat offset similarly.

* Fix negative offset and remove substring
  • Loading branch information
straight-shoota authored and asterite committed Jan 23, 2018
1 parent ff02d2d commit b05ad8d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 5 deletions.
24 changes: 22 additions & 2 deletions spec/std/string_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -707,24 +707,40 @@ describe "String" do

describe "rindex" do
describe "by char" do
it { "bbbb".rindex('b').should eq(3) }
it { "foobar".rindex('a').should eq(4) }
it { "foobar".rindex('g').should be_nil }
it { "日本語日本語".rindex('本').should eq(4) }
it { "あいう_えお".rindex('_').should eq(3) }

describe "with offset" do
it { "bbbb".rindex('b', 2).should eq(2) }
it { "abbbb".rindex('b', 0).should be_nil }
it { "abbbb".rindex('b', 1).should eq(1) }
it { "abbbb".rindex('a', 0).should eq(0) }
it { "bbbb".rindex('b', -2).should eq(2) }
it { "bbbb".rindex('b', -5).should be_nil }
it { "bbbb".rindex('b', -4).should eq(0) }
it { "faobar".rindex('a', 3).should eq(1) }
it { "faobarbaz".rindex('a', -3).should eq(4) }
it { "日本語日本語".rindex('本', 3).should eq(1) }
end
end

describe "by string" do
it { "bbbb".rindex("b").should eq(3) }
it { "foo baro baz".rindex("o b").should eq(7) }
it { "foo baro baz".rindex("fg").should be_nil }
it { "日本語日本語".rindex("日本").should eq(3) }

describe "with offset" do
it { "bbbb".rindex("b", 2).should eq(2) }
it { "abbbb".rindex("b", 0).should be_nil }
it { "abbbb".rindex("b", 1).should eq(1) }
it { "abbbb".rindex("a", 0).should eq(0) }
it { "bbbb".rindex("b", -2).should eq(2) }
it { "bbbb".rindex("b", -5).should be_nil }
it { "bbbb".rindex("b", -4).should eq(0) }
it { "foo baro baz".rindex("o b", 6).should eq(2) }
it { "foo".rindex("", 3).should eq(3) }
it { "foo".rindex("", 4).should eq(3) }
Expand All @@ -738,8 +754,12 @@ describe "String" do
it { "bbbb".rindex(/\d/).should be_nil }

describe "with offset" do
it { "bbbb".rindex(/b/, -3).should eq(2) }
it { "bbbb".rindex(/b/, -1235).should be_nil }
it { "bbbb".rindex(/b/, 2).should eq(2) }
it { "abbbb".rindex(/b/, 0).should be_nil }
it { "abbbb".rindex(/a/, 0).should eq(0) }
it { "bbbb".rindex(/b/, -2).should eq(2) }
it { "bbbb".rindex(/b/, -5).should be_nil }
it { "bbbb".rindex(/b/, -4).should eq(0) }
it { "日本語日本語".rindex(/日本/, 2).should eq(0) }
end
end
Expand Down
7 changes: 4 additions & 3 deletions src/string.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2738,16 +2738,17 @@ class String
end

# ditto
def rindex(search : Regex, offset = 0)
def rindex(search : Regex, offset = size - 1)
offset += size if offset < 0
return nil unless 0 <= offset <= size

match_result = nil
self[0, self.size - offset].scan(search) do |match_data|
scan(search) do |match_data|
break if (index = match_data.begin) && index > offset
match_result = match_data
end

match_result.try &.begin(0)
match_result.try &.begin
end

# Searches separator or pattern (`Regex`) in the string, and returns
Expand Down

0 comments on commit b05ad8d

Please sign in to comment.