Merge pull request #340 from skippy/add-array-like-methods

ruby: make repeated_field quack like an array

ruby/ext/google/protobuf_c/protobuf.h

@@ -361,13 +361,13 @@ extern VALUE cRepeatedField;
 RepeatedField* ruby_to_RepeatedField(VALUE value);
 
 VALUE RepeatedField_each(VALUE _self);
-VALUE RepeatedField_index(VALUE _self, VALUE _index);
+VALUE RepeatedField_index(int argc, VALUE* argv, VALUE _self);
 void* RepeatedField_index_native(VALUE _self, int index);
 VALUE RepeatedField_index_set(VALUE _self, VALUE _index, VALUE val);
 void RepeatedField_reserve(RepeatedField* self, int new_size);
 VALUE RepeatedField_push(VALUE _self, VALUE val);
 void RepeatedField_push_native(VALUE _self, void* data);
-VALUE RepeatedField_pop(VALUE _self);
+VALUE RepeatedField_pop_one(VALUE _self);
 VALUE RepeatedField_insert(int argc, VALUE* argv, VALUE _self);
 VALUE RepeatedField_replace(VALUE _self, VALUE list);
 VALUE RepeatedField_clear(VALUE _self);

ruby/ext/google/protobuf_c/repeated_field.c

@@ -55,6 +55,21 @@ static int index_position(VALUE _index, RepeatedField* repeated_field) {
   return index;
 }
 
+VALUE RepeatedField_subarray(VALUE _self, long beg, long len) {
+  RepeatedField* self = ruby_to_RepeatedField(_self);
+  int element_size = native_slot_size(self->field_type);
+  upb_fieldtype_t field_type = self->field_type;
+  VALUE field_type_class = self->field_type_class;
+
+  size_t off = beg * element_size;
+  VALUE ary = rb_ary_new2(len);
+  for (int i = beg; i < beg + len; i++, off += element_size) {
+    void* mem = ((uint8_t *)self->elements) + off;
+    VALUE elem = native_slot_get(field_type, field_type_class, mem);
+    rb_ary_push(ary, elem);
+  }
+  return ary;
+}
 
 /*
  * call-seq:
@@ -76,28 +91,57 @@ VALUE RepeatedField_each(VALUE _self) {
     VALUE val = native_slot_get(field_type, field_type_class, memory);
     rb_yield(val);
   }
-  return Qnil;
+  return _self;
 }
 
+
 /*
  * call-seq:
  *     RepeatedField.[](index) => value
  *
  * Accesses the element at the given index. Returns nil on out-of-bounds
  */
-VALUE RepeatedField_index(VALUE _self, VALUE _index) {
+VALUE RepeatedField_index(int argc, VALUE* argv, VALUE _self) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
   int element_size = native_slot_size(self->field_type);
   upb_fieldtype_t field_type = self->field_type;
   VALUE field_type_class = self->field_type_class;
 
-  int index = index_position(_index, self);
-  if (index < 0 || index >= self->size) {
+  VALUE arg = argv[0];
+  long beg, len;
+
+  if (argc == 1){
+    if (FIXNUM_P(arg)) {
+      /* standard case */
+      int index = index_position(argv[0], self);
+      if (index < 0 || index >= self->size) {
+        return Qnil;
+      }
+      void* memory = (void *) (((uint8_t *)self->elements) + index * element_size);
+      return native_slot_get(field_type, field_type_class, memory);
+    }else{
+      /* check if idx is Range */
+      size_t off;
+      switch (rb_range_beg_len(arg, &beg, &len, self->size, 0)) {
+        case Qfalse:
+          break;
+        case Qnil:
+          return Qnil;
+        default:
+          return RepeatedField_subarray(_self, beg, len);
+      }
+    }
+  }
+  /* assume 2 arguments */
+  beg = NUM2LONG(argv[0]);
+  len = NUM2LONG(argv[1]);
+  if (beg < 0) {
+    beg += self->size;
+  }
+  if (beg >= self->size) {
     return Qnil;
   }
-
-  void* memory = (void *) (((uint8_t *)self->elements) + index * element_size);
-  return native_slot_get(field_type, field_type_class, memory);
+  return RepeatedField_subarray(_self, beg, len);
 }
 
 /*
@@ -173,6 +217,7 @@ VALUE RepeatedField_push(VALUE _self, VALUE val) {
   return _self;
 }
 
+
 // Used by parsing handlers.
 void RepeatedField_push_native(VALUE _self, void* data) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
@@ -193,19 +238,15 @@ void* RepeatedField_index_native(VALUE _self, int index) {
 }
 
 /*
- * call-seq:
- *     RepeatedField.pop => value
- *
- * Removes the last element and returns it. Throws an exception if the repeated
- * field is empty.
+ * Private ruby method, used by RepeatedField.pop
  */
-VALUE RepeatedField_pop(VALUE _self) {
+VALUE RepeatedField_pop_one(VALUE _self) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
   upb_fieldtype_t field_type = self->field_type;
   VALUE field_type_class = self->field_type_class;
   int element_size = native_slot_size(field_type);
   if (self->size == 0) {
-    rb_raise(rb_eRangeError, "Pop from empty repeated field is not allowed.");
+    return Qnil;
   }
   int index = self->size - 1;
   void* memory = (void *) (((uint8_t *)self->elements) + index * element_size);
@@ -214,19 +255,6 @@ VALUE RepeatedField_pop(VALUE _self) {
   return ret;
 }
 
-/*
- * call-seq:
- *     RepeatedField.insert(*args)
- *
- * Pushes each arg in turn onto the end of the repeated field.
- */
-VALUE RepeatedField_insert(int argc, VALUE* argv, VALUE _self) {
-  for (int i = 0; i < argc; i++) {
-    RepeatedField_push(_self, argv[i]);
-  }
-  return Qnil;
-}
-
 /*
  * call-seq:
  *     RepeatedField.replace(list)
@@ -240,7 +268,7 @@ VALUE RepeatedField_replace(VALUE _self, VALUE list) {
   for (int i = 0; i < RARRAY_LEN(list); i++) {
     RepeatedField_push(_self, rb_ary_entry(list, i));
   }
-  return Qnil;
+  return list;
 }
 
 /*
@@ -252,7 +280,7 @@ VALUE RepeatedField_replace(VALUE _self, VALUE list) {
 VALUE RepeatedField_clear(VALUE _self) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
   self->size = 0;
-  return Qnil;
+  return _self;
 }
 
 /*
@@ -341,7 +369,6 @@ VALUE RepeatedField_to_ary(VALUE _self) {
   for (int i = 0; i < self->size; i++, off += elem_size) {
     void* mem = ((uint8_t *)self->elements) + off;
     VALUE elem = native_slot_get(field_type, self->field_type_class, mem);
-
     rb_ary_push(ary, elem);
   }
   return ary;
@@ -417,19 +444,6 @@ VALUE RepeatedField_hash(VALUE _self) {
   return hash;
 }
 
-/*
- * call-seq:
- *     RepeatedField.inspect => string
- *
- * Returns a string representing this repeated field's elements. It will be
- * formated as "[<element>, <element>, ...]", with each element's string
- * representation computed by its own #inspect method.
- */
-VALUE RepeatedField_inspect(VALUE _self) {
-  VALUE self_ary = RepeatedField_to_ary(_self);
-  return rb_funcall(self_ary, rb_intern("inspect"), 0);
-}
-
 /*
  * call-seq:
  *     RepeatedField.+(other) => repeated field
@@ -466,6 +480,22 @@ VALUE RepeatedField_plus(VALUE _self, VALUE list) {
   return dupped;
 }
 
+/*
+ * call-seq:
+ *     RepeatedField.concat(other) => self
+ *
+ * concats the passed in array to self.  Returns a Ruby array.
+ */
+VALUE RepeatedField_concat(VALUE _self, VALUE list) {
+  RepeatedField* self = ruby_to_RepeatedField(_self);
+  Check_Type(list, T_ARRAY);
+  for (int i = 0; i < RARRAY_LEN(list); i++) {
+    RepeatedField_push(_self, rb_ary_entry(list, i));
+  }
+  return _self;
+}
+
+
 void validate_type_class(upb_fieldtype_t type, VALUE klass) {
   if (rb_iv_get(klass, kDescriptorInstanceVar) == Qnil) {
     rb_raise(rb_eArgError,
@@ -585,22 +615,23 @@ void RepeatedField_register(VALUE module) {
   rb_define_method(klass, "initialize",
                    RepeatedField_init, -1);
   rb_define_method(klass, "each", RepeatedField_each, 0);
-  rb_define_method(klass, "[]", RepeatedField_index, 1);
+  rb_define_method(klass, "[]", RepeatedField_index, -1);
+  rb_define_method(klass, "at", RepeatedField_index, -1);
   rb_define_method(klass, "[]=", RepeatedField_index_set, 2);
   rb_define_method(klass, "push", RepeatedField_push, 1);
   rb_define_method(klass, "<<", RepeatedField_push, 1);
-  rb_define_method(klass, "pop", RepeatedField_pop, 0);
-  rb_define_method(klass, "insert", RepeatedField_insert, -1);
+  rb_define_private_method(klass, "pop_one", RepeatedField_pop_one, 0);
   rb_define_method(klass, "replace", RepeatedField_replace, 1);
   rb_define_method(klass, "clear", RepeatedField_clear, 0);
   rb_define_method(klass, "length", RepeatedField_length, 0);
+  rb_define_method(klass, "size", RepeatedField_length, 0);
   rb_define_method(klass, "dup", RepeatedField_dup, 0);
   // Also define #clone so that we don't inherit Object#clone.
   rb_define_method(klass, "clone", RepeatedField_dup, 0);
   rb_define_method(klass, "==", RepeatedField_eq, 1);
   rb_define_method(klass, "to_ary", RepeatedField_to_ary, 0);
   rb_define_method(klass, "hash", RepeatedField_hash, 0);
-  rb_define_method(klass, "inspect", RepeatedField_inspect, 0);
   rb_define_method(klass, "+", RepeatedField_plus, 1);
+  rb_define_method(klass, "concat", RepeatedField_concat, 1);
   rb_include_module(klass, rb_mEnumerable);
 }

ruby/lib/google/protobuf/repeated_field.rb

@@ -28,12 +28,160 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-# add syntatic sugar on top of the core library
+require 'forwardable'
+
+#
+# This class makes RepeatedField act (almost-) like a Ruby Array.
+# It has convenience methods that extend the core C or Java based
+# methods.
+#
+# This is a best-effort to mirror Array behavior.  Two comments:
+#  1) patches always welcome :)
+#  2) if performance is an issue, feel free to rewrite the method
+#     in jruby and C.  The source code has plenty of examples
+#
+# KNOWN ISSUES
+#   - #[]= doesn't allow less used approaches such as `arr[1, 2] = 'fizz'`
+#   - #concat should return the orig array
+#   - #push should accept multiple arguments and push them all at the same time
+#
 module Google
   module Protobuf
     class RepeatedField
+      extend Forwardable
+
+      # methods defined in C or Java:
+      #   +
+      #   [], at
+      #   []=
+      #   concat
+      #   clear
+      #   dup, clone
+      #   each
+      #   push, <<
+      #   replace
+      #   length, size
+      #   ==
+      #   to_ary, to_a
+      #   also all enumerable
+      #
+      # NOTE:  using delegators rather than method_missing to make the
+      #        relationship explicit instead of implicit
+      def_delegators :to_ary,
+        :&, :*, :-, :'<=>',
+        :assoc, :bsearch, :combination, :compact, :count, :cycle,
+        :drop, :drop_while, :eql?, :fetch, :find_index, :flatten,
+        :include?, :index, :inspect, :join,
+        :pack, :permutation, :product, :pretty_print, :pretty_print_cycle,
+        :rassoc, :repeated_combination, :repeated_permutation, :reverse,
+        :rindex, :rotate, :sample, :shuffle, :shelljoin, :slice,
+        :to_s, :transpose, :uniq, :|
+
+
+      def first(n=nil)
+        n ? self[0..n] : self[0]
+      end
+
+
+      def last(n=nil)
+        n ? self[(self.size-n-1)..-1] : self[-1]
+      end
+
+
+      def pop(n=nil)
+        if n
+          results = []
+          n.times{ results << pop_one }
+          return results
+        else
+          return pop_one
+        end
+      end
+
+
+      def empty?
+        self.size == 0
+      end
+
+      # array aliases into enumerable
+      alias_method :each_index, :each_with_index
+      alias_method :slice, :[]
+      alias_method :values_at, :select
+      alias_method :map, :collect
+
+
+      class << self
+        def define_array_wrapper_method(method_name)
+          define_method(method_name) do |*args, &block|
+            arr = self.to_a
+            result = arr.send(method_name, *args)
+            self.replace(arr)
+            return result if result
+            return block ? block.call : result
+          end
+        end
+        private :define_array_wrapper_method
+
+
+        def define_array_wrapper_with_result_method(method_name)
+          define_method(method_name) do |*args, &block|
+            # result can be an Enumerator, Array, or nil
+            # Enumerator can sometimes be returned if a block is an optional argument and it is not passed in
+            # nil usually specifies that no change was made
+            result = self.to_a.send(method_name, *args, &block)
+            if result
+              new_arr = result.to_a
+              self.replace(new_arr)
+              if result.is_a?(Enumerator)
+                # generate a fresh enum; rewinding the exiting one, in Ruby 2.2, will
+                # reset the enum with the same length, but all the #next calls will
+                # return nil
+                result = new_arr.to_enum
+                # generate a wrapper enum so any changes which occur by a chained
+                # enum can be captured
+                ie = ProxyingEnumerator.new(self, result)
+                result = ie.to_enum
+              end
+            end
+            result
+          end
+        end
+        private :define_array_wrapper_with_result_method
+      end
+
+
+      %w(delete delete_at delete_if shift slice! unshift).each do |method_name|
+        define_array_wrapper_method(method_name)
+      end
+
+
+      %w(collect! compact! fill flatten! insert reverse!
+        rotate! select! shuffle! sort! sort_by! uniq!).each do |method_name|
+        define_array_wrapper_with_result_method(method_name)
+      end
+      alias_method :keep_if, :select!
+      alias_method :map!, :collect!
+      alias_method :reject!, :delete_if
+
+
+      # propagates changes made by user of enumerator back to the original repeated field.
+      # This only applies in cases where the calling function which created the enumerator,
+      # such as #sort!, modifies itself rather than a new array, such as #sort
+      class ProxyingEnumerator < Struct.new(:repeated_field, :external_enumerator)
+        def each(*args, &block)
+          results = []
+          external_enumerator.each_with_index do |val, i|
+            result = yield(val)
+            results << result
+            #nil means no change occured from yield; usually occurs when #to_a is called
+            if result
+              repeated_field[i] = result if result != val
+            end
+          end
+          results
+        end
+      end
 
-      alias_method :size, :length
 
     end
   end

ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java

@@ -71,6 +71,17 @@ public class RubyFieldDescriptor extends RubyObject {
         return this;
     }
 
+    /*
+     * call-seq:
+     *     FieldDescriptor.label
+     *
+     * Return the label of this field.
+     */
+    @JRubyMethod(name = "label")
+    public IRubyObject getLabel(ThreadContext context) {
+        return this.label;
+    }
+
     /*
      * call-seq:
      *     FieldDescriptor.label = label
@@ -80,8 +91,10 @@ public class RubyFieldDescriptor extends RubyObject {
      */
     @JRubyMethod(name = "label=")
     public IRubyObject setLabel(ThreadContext context, IRubyObject value) {
+        String labelName = value.asJavaString();
+        this.label = context.runtime.newSymbol(labelName.toLowerCase());
         this.builder.setLabel(
-                DescriptorProtos.FieldDescriptorProto.Label.valueOf("LABEL_" + value.asJavaString().toUpperCase()));
+                DescriptorProtos.FieldDescriptorProto.Label.valueOf("LABEL_" + labelName.toUpperCase()));
         return context.runtime.getNil();
     }
 
@@ -89,18 +102,13 @@ public class RubyFieldDescriptor extends RubyObject {
      * call-seq:
      *     FieldDescriptor.name => name
      *
-     * Returns the name of this field.
+     * Returns the name of this field as a Ruby String, or nil if it is not set.
      */
     @JRubyMethod(name = "name")
     public IRubyObject getName(ThreadContext context) {
         return this.name;
     }
 
-    @JRubyMethod(name = "subtype")
-    public IRubyObject getSubType(ThreadContext context) {
-        return subType;
-    }
-
     /*
      * call-seq:
      *     FieldDescriptor.name = name
@@ -116,6 +124,12 @@ public class RubyFieldDescriptor extends RubyObject {
         return context.runtime.getNil();
     }
 
+
+    @JRubyMethod(name = "subtype")
+    public IRubyObject getSubType(ThreadContext context) {
+        return subType;
+    }
+
     /*
      * call-seq:
      *     FieldDescriptor.type => type
@@ -144,6 +158,18 @@ public class RubyFieldDescriptor extends RubyObject {
         return context.runtime.getNil();
     }
 
+    /*
+     * call-seq:
+     *     FieldDescriptor.number => number
+     *
+     * Returns this field's number, as a Ruby Integer, or nil if not yet set.
+     *
+     */
+    @JRubyMethod(name = "number")
+    public IRubyObject getnumber(ThreadContext context) {
+        return this.number;
+    }
+
     /*
      * call-seq:
      *     FieldDescriptor.number = number
@@ -153,6 +179,7 @@ public class RubyFieldDescriptor extends RubyObject {
      */
     @JRubyMethod(name = "number=")
     public IRubyObject setNumber(ThreadContext context, IRubyObject value) {
+        this.number = value;
         this.builder.setNumber(RubyNumeric.num2int(value));
         return context.runtime.getNil();
     }
@@ -240,6 +267,8 @@ public class RubyFieldDescriptor extends RubyObject {
 
     private DescriptorProtos.FieldDescriptorProto.Builder builder;
     private IRubyObject name;
+    private IRubyObject label;
+    private IRubyObject number;
     private IRubyObject subType;
     private IRubyObject oneofName;
     private Descriptors.FieldDescriptor fieldDef;

ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java

@@ -40,6 +40,7 @@ import org.jruby.runtime.Block;
 import org.jruby.runtime.ObjectAllocator;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
+import java.util.Arrays;
 
 @JRubyClass(name = "RepeatedClass", include = "Enumerable")
 public class RubyRepeatedField extends RubyObject {
@@ -110,6 +111,10 @@ public class RubyRepeatedField extends RubyObject {
     public IRubyObject indexSet(ThreadContext context, IRubyObject index, IRubyObject value) {
         int arrIndex = normalizeArrayIndex(index);
         Utils.checkType(context, fieldType, value, (RubyModule) typeClass);
+        IRubyObject defaultValue = defaultValue(context);
+        for (int i = this.storage.size(); i < arrIndex; i++) {
+            this.storage.set(i, defaultValue);
+        }
         this.storage.set(arrIndex, value);
         return context.runtime.getNil();
     }
@@ -120,27 +125,35 @@ public class RubyRepeatedField extends RubyObject {
      *
      * Accesses the element at the given index. Returns nil on out-of-bounds
      */
-    @JRubyMethod(name = "[]")
-    public IRubyObject index(ThreadContext context, IRubyObject index) {
-        int arrIndex = normalizeArrayIndex(index);
-        if (arrIndex < 0 || arrIndex >= this.storage.size()) {
-            return context.runtime.getNil();
+    @JRubyMethod(required=1, optional=1, name = {"at", "[]"})
+    public IRubyObject index(ThreadContext context, IRubyObject[] args) {
+        if (args.length == 1){
+            IRubyObject arg = args[0];
+            if (Utils.isRubyNum(arg)) {
+                /* standard case */
+                int arrIndex = normalizeArrayIndex(arg);
+                if (arrIndex < 0 || arrIndex >= this.storage.size()) {
+                    return context.runtime.getNil();
+                }
+                return this.storage.eltInternal(arrIndex);
+            } else if (arg instanceof RubyRange) {
+                RubyRange range = ((RubyRange) arg);
+                int beg = RubyNumeric.num2int(range.first(context));
+                int to = RubyNumeric.num2int(range.last(context));
+                int len = to - beg + 1;
+                return this.storage.subseq(beg, len);
+            }
         }
-        return this.storage.eltInternal(arrIndex);
-    }
-
-    /*
-     * call-seq:
-     *     RepeatedField.insert(*args)
-     *
-     * Pushes each arg in turn onto the end of the repeated field.
-     */
-    @JRubyMethod(rest = true)
-    public IRubyObject insert(ThreadContext context, IRubyObject[] args) {
-        for (int i = 0; i < args.length; i++) {
-            push(context, args[i]);
+        /* assume 2 arguments */
+        int beg = RubyNumeric.num2int(args[0]);
+        int len = RubyNumeric.num2int(args[1]);
+        if (beg < 0) {
+            beg += this.storage.size();
         }
-        return context.runtime.getNil();
+        if (beg >= this.storage.size()) {
+            return context.runtime.getNil();
+        }
+        return this.storage.subseq(beg, len);
     }
 
     /*
@@ -151,20 +164,19 @@ public class RubyRepeatedField extends RubyObject {
      */
     @JRubyMethod(name = {"push", "<<"})
     public IRubyObject push(ThreadContext context, IRubyObject value) {
-        Utils.checkType(context, fieldType, value, (RubyModule) typeClass);
+        if (!(fieldType == Descriptors.FieldDescriptor.Type.MESSAGE &&
+            value == context.runtime.getNil())) {
+            Utils.checkType(context, fieldType, value, (RubyModule) typeClass);
+        }
         this.storage.add(value);
-        return this;
+        return this.storage;
     }
 
     /*
-     * call-seq:
-     *     RepeatedField.pop => value
-     *
-     * Removes the last element and returns it. Throws an exception if the repeated
-     * field is empty.
+     * private Ruby method used by RepeatedField.pop
      */
-    @JRubyMethod
-    public IRubyObject pop(ThreadContext context) {
+    @JRubyMethod(visibility = org.jruby.runtime.Visibility.PRIVATE)
+    public IRubyObject pop_one(ThreadContext context) {
         IRubyObject ret = this.storage.last();
         this.storage.remove(ret);
         return ret;
@@ -181,7 +193,7 @@ public class RubyRepeatedField extends RubyObject {
         RubyArray arr = (RubyArray) list;
         checkArrayElementType(context, arr);
         this.storage = arr;
-        return context.runtime.getNil();
+        return this.storage;
     }
 
     /*
@@ -193,7 +205,7 @@ public class RubyRepeatedField extends RubyObject {
     @JRubyMethod
     public IRubyObject clear(ThreadContext context) {
         this.storage.clear();
-        return context.runtime.getNil();
+        return this.storage;
     }
 
     /*
@@ -202,7 +214,7 @@ public class RubyRepeatedField extends RubyObject {
      *
      * Returns the length of this repeated field.
      */
-    @JRubyMethod(name = {"count", "length"})
+    @JRubyMethod(name = {"length", "size"})
     public IRubyObject length(ThreadContext context) {
         return context.runtime.newFixnum(this.storage.size());
     }
@@ -215,7 +227,7 @@ public class RubyRepeatedField extends RubyObject {
      * repeated field's elements and other's elements. The other (second) list may
      * be either another repeated field or a Ruby array.
      */
-    @JRubyMethod(name = "+")
+    @JRubyMethod(name = {"+"})
     public IRubyObject plus(ThreadContext context, IRubyObject list) {
         RubyRepeatedField dup = (RubyRepeatedField) dup(context);
         if (list instanceof RubyArray) {
@@ -231,6 +243,27 @@ public class RubyRepeatedField extends RubyObject {
         return dup;
     }
 
+    /*
+     * call-seq:
+     *     RepeatedField.concat(other) => self
+     *
+     * concats the passed in array to self.  Returns a Ruby array.
+     */
+    @JRubyMethod
+    public IRubyObject concat(ThreadContext context, IRubyObject list) {
+        if (list instanceof RubyArray) {
+            checkArrayElementType(context, (RubyArray) list);
+            this.storage.addAll((RubyArray) list);
+        } else {
+            RubyRepeatedField repeatedField = (RubyRepeatedField) list;
+            if (! fieldType.equals(repeatedField.fieldType) || (typeClass != null && !
+                    typeClass.equals(repeatedField.typeClass)))
+                throw context.runtime.newArgumentError("Attempt to append RepeatedField with different element type.");
+            this.storage.addAll((RubyArray) repeatedField.toArray(context));
+        }
+        return this.storage;
+    }
+
     /*
      * call-seq:
      *     RepeatedField.hash => hash_value
@@ -239,7 +272,7 @@ public class RubyRepeatedField extends RubyObject {
      */
     @JRubyMethod
     public IRubyObject hash(ThreadContext context) {
-        int hashCode = System.identityHashCode(this.storage);
+        int hashCode = this.storage.hashCode();
         return context.runtime.newFixnum(hashCode);
     }
 
@@ -268,17 +301,12 @@ public class RubyRepeatedField extends RubyObject {
     @JRubyMethod
     public IRubyObject each(ThreadContext context, Block block) {
         this.storage.each(context, block);
-        return context.runtime.getNil();
+        return this.storage;
     }
 
+
     @JRubyMethod(name = {"to_ary", "to_a"})
     public IRubyObject toArray(ThreadContext context) {
-        for (int i = 0; i < this.storage.size(); i++) {
-            IRubyObject defaultValue = defaultValue(context);
-            if (storage.eltInternal(i).isNil()) {
-                storage.set(i, defaultValue);
-            }
-        }
         return this.storage;
     }
 
@@ -298,31 +326,6 @@ public class RubyRepeatedField extends RubyObject {
         return dup;
     }
 
-    /*
-     * call-seq:
-     *     RepeatedField.inspect => string
-     *
-     * Returns a string representing this repeated field's elements. It will be
-     * formated as "[<element>, <element>, ...]", with each element's string
-     * representation computed by its own #inspect method.
-     */
-    @JRubyMethod
-    public IRubyObject inspect() {
-        StringBuilder str = new StringBuilder("[");
-        for (int i = 0; i < this.storage.size(); i++) {
-            str.append(storage.eltInternal(i).inspect());
-            str.append(", ");
-        }
-
-        if (str.length() > 1) {
-            str.replace(str.length() - 2, str.length(), "]");
-        } else {
-            str.append("]");
-        }
-
-        return getRuntime().newString(str.toString());
-    }
-
     // Java API
     protected IRubyObject get(int index) {
         return this.storage.eltInternal(index);
@@ -376,6 +379,9 @@ public class RubyRepeatedField extends RubyObject {
             case STRING:
                 value = sentinel.getDefaultString();
                 break;
+            case ENUM:
+                IRubyObject defaultEnumLoc = context.runtime.newFixnum(0);
+                return RubyEnum.lookup(context, typeClass, defaultEnumLoc);
             default:
                 return context.runtime.getNil();
         }

ruby/tests/basic.rb

@@ -178,7 +178,7 @@ module BasicTest
                           :optional_msg => TestMessage2.new,
                           :repeated_string => ["hello", "there", "world"])
       expected = '<BasicTest::TestMessage: optional_int32: -42, optional_int64: 0, optional_uint32: 0, optional_uint64: 0, optional_bool: false, optional_float: 0.0, optional_double: 0.0, optional_string: "", optional_bytes: "", optional_msg: <BasicTest::TestMessage2: foo: 0>, optional_enum: :A, repeated_int32: [], repeated_int64: [], repeated_uint32: [], repeated_uint64: [], repeated_bool: [], repeated_float: [], repeated_double: [], repeated_string: ["hello", "there", "world"], repeated_bytes: [], repeated_msg: [], repeated_enum: []>'
-      assert m.inspect == expected
+      assert_equal expected, m.inspect
     end
 
     def test_hash
@@ -276,7 +276,7 @@ module BasicTest
 
       assert l.inspect == '[5, 2, 3, 4]'
 
-      l.insert(7, 8, 9)
+      l.concat([7, 8, 9])
       assert l == [5, 2, 3, 4, 7, 8, 9]
       assert l.pop == 9
       assert l == [5, 2, 3, 4, 7, 8]