In Ruby repeated fields, each_index actually iterates over the index (#11767)

Currently we're aliasing `each_index` to `each_with_index`, incorrectly passing both the index and the value of a repeated field to the block.

What we want is to just pass the index. Luckily this is a method on Ruby arrays, so we just wrap the native Ruby array method.

Fixes https://github.com/protocolbuffers/protobuf/issues/7806

Closes #11767

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/11767 from shaldengeki:shaldengeki-repeated-field-each-index-returns-actual-index 874916c21d
PiperOrigin-RevId: 593835025
pull/15190/head
Charles OuGuo 11 months ago committed by Copybara-Service
parent da56def7c2
commit f869cfa479
  1. 14
      ruby/compatibility_tests/v3.0.0/tests/repeated_field_test.rb
  2. 3
      ruby/lib/google/protobuf/repeated_field.rb
  3. 14
      ruby/tests/repeated_field_test.rb

@ -122,6 +122,20 @@ class RepeatedFieldTest < Test::Unit::TestCase
end end
def test_each_index
m = TestMessage.new
5.times{|i| m.repeated_string << 'string' }
expected = 0
m.repeated_string.each_index do |idx|
assert_equal expected, idx
expected += 1
assert_equal 'string', m.repeated_string[idx]
end
assert_equal 5, expected
end
def test_empty? def test_empty?
m = TestMessage.new m = TestMessage.new
assert_empty m.repeated_string assert_empty m.repeated_string

@ -94,7 +94,6 @@ module Google
end end
# array aliases into enumerable # array aliases into enumerable
alias_method :each_index, :each_with_index
alias_method :slice, :[] alias_method :slice, :[]
alias_method :values_at, :select alias_method :values_at, :select
alias_method :map, :collect alias_method :map, :collect
@ -145,7 +144,7 @@ module Google
end end
%w(collect! compact! delete_if fill flatten! insert reverse! %w(collect! compact! delete_if each_index fill flatten! insert reverse!
rotate! select! shuffle! sort! sort_by! uniq!).each do |method_name| rotate! select! shuffle! sort! sort_by! uniq!).each do |method_name|
define_array_wrapper_with_result_method(method_name) define_array_wrapper_with_result_method(method_name)
end end

@ -143,6 +143,20 @@ class RepeatedFieldTest < Test::Unit::TestCase
end end
def test_each_index
m = TestMessage.new
5.times{|i| m.repeated_string << 'string' }
expected = 0
m.repeated_string.each_index do |idx|
assert_equal expected, idx
expected += 1
assert_equal 'string', m.repeated_string[idx]
end
assert_equal 5, expected
end
def test_empty? def test_empty?
m = TestMessage.new m = TestMessage.new
assert_empty m.repeated_string assert_empty m.repeated_string

Loading…
Cancel
Save