[Ruby] Fix RepeatedField#last, #first inconsistencies

[lucthev]

RepeatedField#last(n) has an off-by-one error resulting in one more element being returned than requested.

While I was at it, I also fixed the following inconsistencies with Array:

• last(n) where n > size should return all elements; the current implementation wraps around such that e.g. last(size + 2) == last(2)
• last(n) where n < 0 should raise an ArgumentError

[fowles]

This looks good. I will merge it after our test suite finishes

[fowles]

Looks like this is failing tests:

Failure: test_last(RepeatedFieldTest)
/tmp/protobuf/protobuf/ruby/tests/repeated_field_test.rb:85:in `test_last'
     82:     end
     83:     assert_equal "negative array size", err.message
     84:     assert_equal [], m.repeated_int32.last(0)
  => 85:     assert_equal [-11], m.repeated_int32.last(1)
     86:     assert_equal [-10, -11], m.repeated_int32.last(2)
     87:     assert_equal [-10, -11], m.repeated_int32.last(3)
     88:   end
org/jruby/RubyKernel.java:1189:in `catch'
org/jruby/RubyKernel.java:1184:in `catch'
org/jruby/RubyKernel.java:1189:in `catch'
org/jruby/RubyKernel.java:1184:in `catch'
<[-11]> expected but was
<[]>****

[fowles]

Looks like the failure is in jruby. Jason?

[JasonLunn]

It appears that the fix for this issue has uncovered a separate bug in the JRuby implementation of the at (aka []) method of RubyRepeatedField. It appears that the implementation is making assumptions here about the range values that don't hold if you use negative indices to count back from the end of the sequence.

@lucthev - if you're comfortable working in Java, you can fix both issues in the same PR. If not, please open a separate ticket for the JRuby-specific issue and I can work up a patch.

[fowles]

rerunning test suite. Will come back tomorrow to see how it went

[lucthev]

Thanks for the reviews! I had a stab at fixing the issue in RubyRepeatedField#[] but there's a lot of odd edge-cases (and I'm not very familiar with Java) so I didn't get very far. I've opened #9751 to track that issue just for completeness (it's pretty niche anyways).

In the meantime, I've worked around that in this PR by not using a negative index.