Merge pull request #223 from pkmiec/simpleTypedListAsArray

SimpleTypedList subclass of Array

4 files changed, 66 insertions, 50 deletions

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 8f8f4bd7..043f7daf 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -91,7 +91,6 @@ Lint/UselessAssignment:
 Lint/Void:
   Exclude:
     - 'lib/axlsx/drawing/title.rb'
-    - 'lib/axlsx/util/simple_typed_list.rb'
     - 'lib/axlsx/util/storage.rb'
     - 'lib/axlsx/workbook/worksheet/data_bar.rb'
     - 'lib/axlsx/workbook/worksheet/pivot_table.rb'
@@ -216,10 +215,6 @@ Style/ConditionalAssignment:
     - 'lib/axlsx/workbook/workbook.rb'
     - 'lib/axlsx/workbook/worksheet/data_bar.rb'
 
-Style/DocumentDynamicEvalDefinition:
-  Exclude:
-    - 'lib/axlsx/util/simple_typed_list.rb'
-
 # Configuration parameters: AllowedConstants.
 Style/Documentation:
   Exclude:
@@ -350,7 +345,6 @@ Style/MutableConstant:
     - 'lib/axlsx/drawing/pic.rb'
     - 'lib/axlsx/drawing/view_3D.rb'
     - 'lib/axlsx/stylesheet/dxf.rb'
-    - 'lib/axlsx/util/simple_typed_list.rb'
     - 'lib/axlsx/util/storage.rb'
     - 'lib/axlsx/workbook/worksheet/auto_filter/filter_column.rb'
     - 'lib/axlsx/workbook/worksheet/auto_filter/filters.rb'
@@ -395,7 +389,6 @@ Style/NumericPredicate:
     - 'spec/**/*'
     - 'lib/axlsx/package.rb'
     - 'lib/axlsx/stylesheet/font.rb'
-    - 'lib/axlsx/util/simple_typed_list.rb'
     - 'lib/axlsx/util/validators.rb'
     - 'lib/axlsx/workbook/workbook.rb'
     - 'lib/axlsx/workbook/worksheet/cell.rb'
@@ -596,7 +589,6 @@ Style/SlicingWithRange:
     - 'lib/axlsx.rb'
     - 'lib/axlsx/drawing/area_chart.rb'
     - 'lib/axlsx/drawing/line_chart.rb'
-    - 'lib/axlsx/util/simple_typed_list.rb'
     - 'lib/axlsx/workbook/worksheet/pivot_table.rb'
     - 'lib/axlsx/workbook/worksheet/worksheet.rb'
 
@@ -698,7 +690,6 @@ Style/YodaCondition:
 Style/ZeroLengthPredicate:
   Exclude:
     - 'lib/axlsx/package.rb'
-    - 'lib/axlsx/util/simple_typed_list.rb'
     - 'lib/axlsx/workbook/workbook.rb'
     - 'lib/axlsx/workbook/worksheet/sheet_pr.rb'
 
@@ -706,4 +697,4 @@ Style/ZeroLengthPredicate:
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https
 Layout/LineLength:
-  Max: 574
+  Max: 559
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 49cae46a..19f5d742 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@ CHANGELOG
 - **Unreleased**: 4.0.0
   - Drop support for Ruby versions < 2.6
   - Added frozen string literals
+  - Fix `SimpleTypedList#to_a` and `SimpleTypedList#to_ary` returning the internal list instance
 
 - **April.23.23**: 3.4.1
   - [PR #209](https://github.com/caxlsx/caxlsx/pull/209) - Revert characters other than `=` being considered as formulas.
diff --git a/lib/axlsx/util/simple_typed_list.rb b/lib/axlsx/util/simple_typed_list.rb
index 60da0372..c3910fca 100644
--- a/lib/axlsx/util/simple_typed_list.rb
+++ b/lib/axlsx/util/simple_typed_list.rb
@@ -3,12 +3,25 @@
 module Axlsx
   # A SimpleTypedList is a type restrictive collection that allows some of the methods from Array and supports basic xml serialization.
   # @private
-  class SimpleTypedList
+  class SimpleTypedList < Array
+    DESTRUCTIVE = [
+      'replace', 'insert', 'collect!', 'map!', 'pop', 'delete_if',
+      'reverse!', 'shift', 'shuffle!', 'slice!', 'sort!', 'uniq!',
+      'unshift', 'zip', 'flatten!', 'fill', 'drop', 'drop_while',
+      'clear'
+    ].freeze
+
+    DESTRUCTIVE.each do |name|
+      undef_method name
+    end
+
     # Creats a new typed list
     # @param [Array, Class] type An array of Class objects or a single Class object
     # @param [String] serialize_as The tag name to use in serialization
     # @raise [ArgumentError] if all members of type are not Class objects
     def initialize(type, serialize_as = nil, start_size = 0)
+      super(start_size)
+
       if type.is_a? Array
         type.each { |item| raise ArgumentError, "All members of type must be Class objects" unless item.is_a? Class }
         @allowed_types = type
@@ -18,7 +31,6 @@ module Axlsx
         @allowed_types = [type]
       end
       @serialize_as = serialize_as unless serialize_as.nil?
-      @list = Array.new(start_size)
     end
 
     # The class constants of allowed types
@@ -39,16 +51,16 @@ module Axlsx
     # Transposes the list (without blowing up like ruby does)
     # any non populated cell in the matrix will be a nil value
     def transpose
-      return @list.clone if @list.size == 0
+      return clone if size.zero?
 
-      row_count = @list.size
-      max_column_count = @list.map { |row| row.cells.size }.max
+      row_count = size
+      max_column_count = map { |row| row.cells.size }.max
       result = Array.new(max_column_count) { Array.new(row_count) }
       # yes, I know it is silly, but that warning is really annoying
       row_count.times do |row_index|
         max_column_count.times do |column_index|
-          datum = if @list[row_index].cells.size >= max_column_count
-                    @list[row_index].cells[column_index]
+          datum = if self[row_index].cells.size >= max_column_count
+                    self[row_index].cells[column_index]
                   elsif block_given?
                     yield(column_index, row_index)
                   end
@@ -61,7 +73,7 @@ module Axlsx
     # Lock this list at the current size
     # @return [self]
     def lock
-      @locked_at = @list.size
+      @locked_at = size
       self
     end
 
@@ -72,12 +84,6 @@ module Axlsx
       self
     end
 
-    def to_ary
-      @list
-    end
-
-    alias :to_a :to_ary
-
     # join operator
     # @param [Array] v the array to join
     # @raise [ArgumentError] if any of the values being joined are not
@@ -85,9 +91,9 @@ module Axlsx
     # @return [SimpleTypedList]
     def +(v)
       v.each do |item|
-        DataTypeValidator.validate :SimpleTypedList_plus, @allowed_types, item
-        @list << item
+        self << item
       end
+      super
     end
 
     # Concat operator
@@ -96,8 +102,8 @@ module Axlsx
     # @return [Integer] returns the index of the item added.
     def <<(v)
       DataTypeValidator.validate :SimpleTypedList_push, @allowed_types, v
-      @list << v
-      @list.size - 1
+      super
+      size - 1
     end
 
     alias :push :<<
@@ -110,17 +116,16 @@ module Axlsx
       return unless include? v
       raise ArgumentError, "Item is protected and cannot be deleted" if protected? index(v)
 
-      @list.delete v
+      super
     end
 
     # delete the item from the list at the index position provided
     # @raise [ArgumentError] if the index is protected by locking
     # @return [Any] The item deleted
     def delete_at(index)
-      @list[index]
       raise ArgumentError, "Item is protected and cannot be deleted" if protected? index
 
-      @list.delete_at index
+      super
     end
 
     # positional assignment. Adds the item at the index specified
@@ -128,12 +133,12 @@ module Axlsx
     # @param [Any] v
     # @raise [ArgumentError] if the index is protected by locking
     # @raise [ArgumentError] if the item is not one of the allowed types
+    # @return [Any] The item added
     def []=(index, v)
       DataTypeValidator.validate :SimpleTypedList_insert, @allowed_types, v
       raise ArgumentError, "Item is protected and cannot be changed" if protected? index
 
-      @list[index] = v
-      v
+      super
     end
 
     # inserts an item at the index specfied
@@ -141,11 +146,12 @@ module Axlsx
     # @param [Any] v
     # @raise [ArgumentError] if the index is protected by locking
     # @raise [ArgumentError] if the index is not one of the allowed types
+    # @return [Any] The item inserted
     def insert(index, v)
       DataTypeValidator.validate :SimpleTypedList_insert, @allowed_types, v
       raise ArgumentError, "Item is protected and cannot be changed" if protected? index
 
-      @list.insert(index, v)
+      super
       v
     end
 
@@ -157,23 +163,9 @@ module Axlsx
       index < locked_at
     end
 
-    DESTRUCTIVE = ['replace', 'insert', 'collect!', 'map!', 'pop', 'delete_if',
-                   'reverse!', 'shift', 'shuffle!', 'slice!', 'sort!', 'uniq!',
-                   'unshift', 'zip', 'flatten!', 'fill', 'drop', 'drop_while',
-                   'delete_if', 'clear']
-    DELEGATES = Array.instance_methods - self.instance_methods - DESTRUCTIVE
-
-    DELEGATES.each do |method|
-      class_eval %{
-        def #{method}(*args, &block)
-          @list.send(:#{method}, *args, &block)
-        end
-      }, __FILE__, __LINE__ - 4
-    end
-
     def to_xml_string(str = +'')
       classname = @allowed_types[0].name.split('::').last
-      el_name = serialize_as.to_s || (classname[0, 1].downcase + classname[1..-1])
+      el_name = serialize_as.to_s || (classname[0, 1].downcase + classname[1..])
       str << '<' << el_name << ' count="' << size.to_s << '">'
       each { |item| item.to_xml_string(str) }
       str << '</' << el_name << '>'
diff --git a/test/util/tc_simple_typed_list.rb b/test/util/tc_simple_typed_list.rb
index ac65529d..1a3d907b 100644
--- a/test/util/tc_simple_typed_list.rb
+++ b/test/util/tc_simple_typed_list.rb
@@ -58,6 +58,9 @@ class TestSimpleTypedList < Test::Unit::TestCase
     assert_raise(ArgumentError) { @list.delete 1  }
     assert_raise(ArgumentError) { @list.delete_at 1 }
     assert_raise(ArgumentError) { @list.delete_at 2 }
+    assert_raise(ArgumentError) { @list.insert(1, 3) }
+    assert_raise(ArgumentError) { @list[1] = 3 }
+
     @list.push 4
     assert_nothing_raised { @list.delete_at 3 }
     @list.unlock
@@ -79,6 +82,35 @@ class TestSimpleTypedList < Test::Unit::TestCase
     @list.push 1
     @list.push 2
 
-    assert_equal([1, 2], @list.to_ary)
+    assert_equal([1, 2], @list)
+  end
+
+  def test_to_a
+    refute_equal(@list.object_id, @list.to_a.object_id)
+    assert_instance_of(Array, @list.to_a)
+  end
+
+  def test_to_ary
+    assert_equal(@list.object_id, @list.to_ary.object_id)
+  end
+
+  def test_insert
+    assert_raise(ArgumentError) { @list << nil }
+
+    assert_equal(1, @list.insert(0, 1))
+    assert_equal(2, @list.insert(1, 2))
+    assert_equal(3, @list.insert(0, 3))
+
+    assert_equal([3, 1, 2], @list)
+  end
+
+  def test_setter
+    assert_raise(ArgumentError) { @list[0] = nil }
+
+    assert_equal(1, @list[0] = 1)
+    assert_equal(2, @list[1] = 2)
+    assert_equal(3, @list[0] = 3)
+
+    assert_equal([3, 2], @list)
   end
 end