Merge pull request #2400 from herwinw/struct_new

Fixes to constructor of Struct
natalie-lang · Dec 16, 2024 · 6374965 · 6374965
2 parents 43f114d + 05d21aa
commit 6374965
Show file tree

Hide file tree

Showing 2 changed files with 165 additions and 160 deletions.
diff --git a/src/struct.rb b/src/struct.rb
@@ -11,210 +11,210 @@ def self.new(*attrs, &block)
       raise ArgumentError, "duplicate member: #{duplicates.first}"
     end
 
-    if respond_to?(:members)
-      BasicObject.method(:new).unbind.bind(self).(*attrs)
-    else
-      if !attrs.first.is_a?(Symbol) && attrs.first.respond_to?(:to_str)
-        klass = attrs.shift.to_str.to_sym
-      elsif attrs.first.nil?
-        attrs.shift
+    if !attrs.first.is_a?(Symbol) && attrs.first.respond_to?(:to_str)
+      klass = attrs.shift.to_str.to_sym
+    elsif attrs.first.nil?
+      attrs.shift
+    end
+
+    if attrs.last.is_a?(Hash)
+      options = attrs.pop
+      unknown_keys = options.keys.difference([:keyword_init])
+      if unknown_keys.any?
+        raise ArgumentError, "unknown keyword: #{unknown_keys.map(&:inspect).join(', ')}"
       end
+    else
+      options = {}
+    end
+    result = Class.new(Struct) do
+      include Enumerable
 
-      if attrs.last.is_a?(Hash)
-        options = attrs.pop
-        unknown_keys = options.keys.difference([:keyword_init])
-        if unknown_keys.any?
-          raise ArgumentError, "unknown keyword: #{unknown_keys.map(&:inspect).join(', ')}"
-        end
-      else
-        options = {}
+      define_singleton_method :members do
+        attrs.dup
       end
-      result = Class.new(Struct) do
-        include Enumerable
 
-        define_singleton_method :members do
-          attrs.dup
-        end
+      define_method :length do
+        attrs.length
+      end
+      alias_method :size, :length
 
-        define_method :length do
-          attrs.length
-        end
-        alias_method :size, :length
-
-        if options[:keyword_init]
-          define_method :initialize do |args = {}|
-            unless args.is_a?(Hash)
-              raise ArgumentError, "wrong number of arguments (given #{args.size}, expected 0)"
-            end
-            invalid_keywords = args.each_key.reject { |arg| attrs.include?(arg) }
-            unless invalid_keywords.empty?
-              raise ArgumentError, "unknown keywords: #{invalid_keywords.join(', ')}"
-            end
-            args.each do |attr, value|
-              send("#{attr}=", value)
-            end
+      if options[:keyword_init]
+        define_method :initialize do |args = {}|
+          unless args.is_a?(Hash)
+            raise ArgumentError, "wrong number of arguments (given #{args.size}, expected 0)"
           end
-        else
-          define_method :initialize do |*vals|
-            if vals.size > attrs.size
-              raise ArgumentError, 'struct size differs'
-            end
-            attrs.each_with_index { |attr, index| send("#{attr}=", vals[index]) }
+          invalid_keywords = args.each_key.reject { |arg| attrs.include?(arg) }
+          unless invalid_keywords.empty?
+            raise ArgumentError, "unknown keywords: #{invalid_keywords.join(', ')}"
+          end
+          args.each do |attr, value|
+            send("#{attr}=", value)
           end
         end
-
-        self.class.define_method :keyword_init? do
-          options[:keyword_init] ? true : options[:keyword_init]
-        end
-
-        define_method :each do
-          if block_given?
-            attrs.each { |attr| yield send(attr) }
-            self
-          else
-            enum_for(:each) { length }
+      else
+        define_method :initialize do |*vals|
+          if vals.size > attrs.size
+            raise ArgumentError, 'struct size differs'
           end
+          attrs.each_with_index { |attr, index| send("#{attr}=", vals[index]) }
         end
+      end
 
-        define_method :each_pair do
-          if block_given?
-            attrs.each { |attr| yield [attr, send(attr)] }
-            self
-          else
-            enum_for(:each_pair) { length }
-          end
+      self.class.define_method :keyword_init? do
+        options[:keyword_init] ? true : options[:keyword_init]
+      end
+
+      define_method :each do
+        if block_given?
+          attrs.each { |attr| yield send(attr) }
+          self
+        else
+          enum_for(:each) { length }
         end
+      end
 
-        define_method :eql? do |other|
-          self.class == other.class && values.zip(other.values).all? { |x, y| x.eql?(y) }
+      define_method :each_pair do
+        if block_given?
+          attrs.each { |attr| yield [attr, send(attr)] }
+          self
+        else
+          enum_for(:each_pair) { length }
         end
+      end
 
-        alias_method :values, :to_a
+      define_method :eql? do |other|
+        self.class == other.class && values.zip(other.values).all? { |x, y| x.eql?(y) }
+      end
 
-        define_method :inspect do
-          inspected_attrs = attrs.map { |attr| "#{attr}=#{send(attr).inspect}" }
-          "#<struct #{inspected_attrs.join(', ')}>"
-        end
-        alias_method :to_s, :inspect
+      alias_method :values, :to_a
 
-        define_method(:deconstruct) do
-          attrs.map { |attr| send(attr) }
+      define_method :inspect do
+        inspected_attrs = attrs.map { |attr| "#{attr}=#{send(attr).inspect}" }
+        "#<struct #{inspected_attrs.join(', ')}>"
+      end
+      alias_method :to_s, :inspect
+
+      define_method(:deconstruct) do
+        attrs.map { |attr| send(attr) }
+      end
+
+      define_method :deconstruct_keys do |arg|
+        if arg.nil?
+          arg = attrs
+        elsif !arg.is_a?(Array)
+          raise TypeError, "wrong argument type #{arg.class} (expected Array or nil)"
         end
 
-        define_method :deconstruct_keys do |arg|
-          if arg.nil?
-            arg = attrs
-          elsif !arg.is_a?(Array)
-            raise TypeError, "wrong argument type #{arg.class} (expected Array or nil)"
+        if arg.size > attrs.size
+          {}
+        else
+          arg = arg.take_while do |key|
+            key.is_a?(Integer) ? key < attrs.size : attrs.include?(key.to_sym)
           end
-
-          if arg.size > attrs.size
-            {}
-          else
-            arg = arg.take_while do |key|
-              key.is_a?(Integer) ? key < attrs.size : attrs.include?(key.to_sym)
-            end
-            arg.each_with_object({}) do |key, memo|
-              memo[key] = self[key]
-            end
+          arg.each_with_object({}) do |key, memo|
+            memo[key] = self[key]
           end
         end
+      end
 
-        define_method :[] do |arg, *rest|
-          raise ArgumentError, "too many arguments given" if rest.any?
-          case arg
-          when Integer
-            arg = attrs.fetch(arg)
-          when String, Symbol
-            unless attrs.include?(arg.to_sym)
-              raise NameError, "no member '#{arg}' in struct"
-            end
+      define_method :[] do |arg, *rest|
+        raise ArgumentError, "too many arguments given" if rest.any?
+        case arg
+        when Integer
+          arg = attrs.fetch(arg)
+        when String, Symbol
+          unless attrs.include?(arg.to_sym)
+            raise NameError, "no member '#{arg}' in struct"
           end
-          send(arg)
         end
+        send(arg)
+      end
 
-        define_method :[]= do |key, value|
-          case key
-          when String, Symbol
-            unless attrs.include?(key.to_sym)
-              raise NameError, "no member '#{key}' in struct"
-            end
-          else
-            key = attrs.fetch(key)
+      define_method :[]= do |key, value|
+        case key
+        when String, Symbol
+          unless attrs.include?(key.to_sym)
+            raise NameError, "no member '#{key}' in struct"
           end
-          send("#{key}=", value)
+        else
+          key = attrs.fetch(key)
         end
+        send("#{key}=", value)
+      end
 
-        define_method :== do |other|
-          self.class == other.class && values == other.values
-        end
+      define_method :== do |other|
+        self.class == other.class && values == other.values
+      end
 
-        define_method :dig do |*args|
-          if args.empty?
-            raise ArgumentError, 'wrong number of arguments (given 0, expected 1+)'
-          end
-          arg = args.shift
-          res = begin
-                  self[arg]
-                rescue
-                  nil
-                end
-          if args.empty? || res.nil?
-            res
-          elsif !res.respond_to?(:dig)
-            raise TypeError, "#{res.class} does not have #dig method"
-          else
-            res.dig(*args)
-          end
+      define_method :dig do |*args|
+        if args.empty?
+          raise ArgumentError, 'wrong number of arguments (given 0, expected 1+)'
+        end
+        arg = args.shift
+        res = begin
+                self[arg]
+              rescue
+                nil
+              end
+        if args.empty? || res.nil?
+          res
+        elsif !res.respond_to?(:dig)
+          raise TypeError, "#{res.class} does not have #dig method"
+        else
+          res.dig(*args)
         end
+      end
 
-        define_method :to_h do |&block|
-          attrs.to_h do |attr|
-            result = [attr.to_sym, send(attr)]
-            if block
-              result = block.call(*result)
-            end
-            result
+      define_method :to_h do |&block|
+        attrs.to_h do |attr|
+          result = [attr.to_sym, send(attr)]
+          if block
+            result = block.call(*result)
           end
+          result
         end
+      end
 
-        define_method :members do
-          attrs.dup
-        end
+      define_method :members do
+        attrs.dup
+      end
 
-        define_method :values_at do |*idxargs|
-          result = []
-          idxargs.each do |idx|
-            case idx
-            when Integer
-              realidx = (idx < 0) ? idx+size : idx
-              raise IndexError, "offset #{idx} too small for struct(size:#{size})" if realidx < 0
-              raise IndexError, "offset #{idx} too large for struct(size:#{size})" if realidx >= size
-              result << send(attrs.fetch(idx))
-            when Range
-              result.concat attrs.values_at(idx).map{ |k| k.nil? ? nil : self[k]}
-            else
-              raise TypeError, "no implicit conversion of #{idx.class} into Integer"
-            end
+      define_method :values_at do |*idxargs|
+        result = []
+        idxargs.each do |idx|
+          case idx
+          when Integer
+            realidx = (idx < 0) ? idx+size : idx
+            raise IndexError, "offset #{idx} too small for struct(size:#{size})" if realidx < 0
+            raise IndexError, "offset #{idx} too large for struct(size:#{size})" if realidx >= size
+            result << send(attrs.fetch(idx))
+          when Range
+            result.concat attrs.values_at(idx).map{ |k| k.nil? ? nil : self[k]}
+          else
+            raise TypeError, "no implicit conversion of #{idx.class} into Integer"
           end
-          result
         end
+        result
+      end
 
-        attrs.each { |attr| attr_accessor attr }
+      attrs.each { |attr| attr_accessor attr }
 
-        if block
-          instance_eval(&block)
-        end
+      if block
+        instance_eval(&block)
       end
+    end
 
-      if klass
-        if Struct.constants.include?(klass)
-          warn("warning: redefining constant Struct::#{klass}")
-        end
-        Struct.const_set(klass, result)
+    if klass
+      if Struct.constants.include?(klass)
+        warn("warning: redefining constant Struct::#{klass}")
       end
+      Struct.const_set(klass, result)
+    end
 
-      result
+    def result.new(...)
+      Object.method(:new).unbind.bind(self).call(...)
     end
+
+    result
   end
 end
diff --git a/test/natalie/struct_test.rb b/test/natalie/struct_test.rb
@@ -14,6 +14,11 @@ class Bar < Foo; end
     s.new(1, 2)
   end
 
+  it 'unbinds the .new method once a struct is created' do
+    s = Struct.new(:a, :b)
+    -> { s.new(1, 1) }.should_not raise_error(ArgumentError, 'duplicate member: 1')
+  end
+
   ruby_version_is ''...'3.3' do
     it 'cannot be initialized without arguments' do
       -> { Struct.new }.should raise_error(ArgumentError, 'wrong number of arguments (given 0, expected 1+)')