From 2bf3339cedb29b5b02e3f43a024f6b6b0afdb983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 16 Jan 2018 11:29:24 +0100 Subject: [PATCH 1/2] Fix offset handling of String#rindex with Regex This also addas a few specs to ensure all variants of #rindex treat offset similarly. --- spec/std/string_spec.cr | 15 +++++++++++++-- src/string.cr | 5 ++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/spec/std/string_spec.cr b/spec/std/string_spec.cr index 9b200572083b..1fb4328b0cb7 100644 --- a/spec/std/string_spec.cr +++ b/spec/std/string_spec.cr @@ -707,12 +707,17 @@ 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 { "faobar".rindex('a', 3).should eq(1) } it { "faobarbaz".rindex('a', -3).should eq(4) } it { "日本語日本語".rindex('本', 3).should eq(1) } @@ -720,11 +725,16 @@ describe "String" do 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 { "foo baro baz".rindex("o b", 6).should eq(2) } it { "foo".rindex("", 3).should eq(3) } it { "foo".rindex("", 4).should eq(3) } @@ -738,8 +748,9 @@ 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 { "日本語日本語".rindex(/日本/, 2).should eq(0) } end end diff --git a/src/string.cr b/src/string.cr index 5f2f4b3b1486..26baad443a95 100644 --- a/src/string.cr +++ b/src/string.cr @@ -2738,12 +2738,11 @@ class String end # ditto - def rindex(search : Regex, offset = 0) - offset += size if offset < 0 + def rindex(search : Regex, offset = size - 1) return nil unless 0 <= offset <= size match_result = nil - self[0, self.size - offset].scan(search) do |match_data| + self[0..offset].scan(search) do |match_data| match_result = match_data end From 3a9931d44be85c1e7a620b5a6f57c65bd816f67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 16 Jan 2018 17:31:05 +0100 Subject: [PATCH 2/2] Fix negative offset and remove substring --- spec/std/string_spec.cr | 9 +++++++++ src/string.cr | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/spec/std/string_spec.cr b/spec/std/string_spec.cr index 1fb4328b0cb7..b6fb05ab4094 100644 --- a/spec/std/string_spec.cr +++ b/spec/std/string_spec.cr @@ -718,6 +718,9 @@ describe "String" do 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) } @@ -735,6 +738,9 @@ describe "String" do 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) } @@ -751,6 +757,9 @@ describe "String" do 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 diff --git a/src/string.cr b/src/string.cr index 26baad443a95..83be128849d1 100644 --- a/src/string.cr +++ b/src/string.cr @@ -2739,14 +2739,16 @@ class String # ditto def rindex(search : Regex, offset = size - 1) + offset += size if offset < 0 return nil unless 0 <= offset <= size match_result = nil - self[0..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