[ruby/ostruct] Reinstate recent changes

This reverts commit 28e60b0045.

lib/ostruct.rb

@@ -36,9 +36,10 @@
 # Hash keys with spaces or characters that could normally not be used for
 # method calls (e.g. <code>()[]*</code>) will not be immediately available
 # on the OpenStruct object as a method for retrieval or assignment, but can
-# still be reached through the Object#send method.
+# still be reached through the Object#send method or using [].
 #
 #   measurements = OpenStruct.new("length (in inches)" => 24)
+#   measurements[:"length (in inches)"]       # => 24
 #   measurements.send("length (in inches)")   # => 24
 #
 #   message = OpenStruct.new(:queued? => true)
@@ -61,8 +62,7 @@
 #   first_pet                 # => #<OpenStruct name="Rowdy">
 #   first_pet == second_pet   # => true
 #
-#
-# == Implementation
+# == Caveats
 #
 # An OpenStruct utilizes Ruby's method lookup structure to find and define the
 # necessary methods for properties. This is accomplished through the methods
@@ -71,10 +71,41 @@
 # This should be a consideration if there is a concern about the performance of
 # the objects that are created, as there is much more overhead in the setting
 # of these properties compared to using a Hash or a Struct.
+# Creating an open struct from a small Hash and accessing a few of the
+# entries can be 200 times slower than accessing the hash directly.
+#
+# This may also be the source of incompatibilities between Ruby versions:
+#
+#   o = OpenStruct.new
+#   o.then # => nil in Ruby < 2.6, enumerator for Ruby >= 2.6
+#
+# Builtin methods may be overwritten this way, which may be a source of bugs
+# or security issues:
+#
+#   o = OpenStruct.new
+#   o.methods # => [:to_h, :marshal_load, :marshal_dump, :each_pair, ...
+#   o.methods = [:foo, :bar]
+#   o.methods # => [:foo, :bar]
+#
+# To help remedy clashes, OpenStruct uses only protected/private methods ending with `!`
+# and defines aliases for builtin public methods by adding a `!`:
+#
+#   o = OpenStruct.new(make: 'Bentley', class: :luxury)
+#   o.class # => :luxury
+#   o.class! # => OpenStruct
+#
+# It is recommended (but not enforced) to not use fields ending in `!`.
+#
+# For all these reasons, consider not using OpenStruct at all.
 #
 class OpenStruct
   VERSION = "0.2.0"
 
+  instance_methods.each do |method|
+    new_name = "#{method}!"
+    alias_method new_name, method
+  end
+
   #
   # Creates a new OpenStruct object.  By default, the resulting OpenStruct
   # object will have no attributes.
@@ -93,18 +124,16 @@ class OpenStruct
     @table = {}
     if hash
       hash.each_pair do |k, v|
-        k = k.to_sym
-        @table[k] = v
-        new_ostruct_member!(k)
+        self[k] = v
       end
     end
   end
 
   # Duplicates an OpenStruct object's Hash table.
   def initialize_copy(orig) # :nodoc:
+    orig.table.each_key{|key| new_ostruct_member!(key)}
     super
     @table = @table.dup
-    @table.each_key{|key| new_ostruct_member!(key)}
   end
 
   #
@@ -161,46 +190,35 @@ class OpenStruct
   # Provides marshalling support for use by the Marshal library.
   #
   def marshal_load(x)
+    x.each_key{|key| new_ostruct_member!(key)}
     @table = x
-    @table.each_key{|key| new_ostruct_member!(key)}
   end
 
-  #
-  # Used internally to check if the OpenStruct is able to be
-  # modified before granting access to the internal Hash table to be modified.
-  #
-  def modifiable? # :nodoc:
-    begin
-      @modifiable = true
-    rescue
-      raise FrozenError, "can't modify frozen #{self.class}", caller(3)
-    end
-    @table
-  end
-  private :modifiable?
-
   #
   # Used internally to defined properties on the
   # OpenStruct. It does this by using the metaprogramming function
   # define_singleton_method for both the getter method and the setter method.
   #
   def new_ostruct_member!(name) # :nodoc:
-    name = name.to_sym
-    unless respond_to?(name)
+    unless @table.key?(name)
       define_singleton_method(name) { @table[name] }
-      define_singleton_method("#{name}=") {|x| modifiable?[name] = x}
+      define_singleton_method("#{name}=") {|x| @table[name] = x}
     end
-    name
   end
   private :new_ostruct_member!
 
+  def freeze
+    @table.freeze
+    super
+  end
+
   def method_missing(mid, *args) # :nodoc:
     len = args.length
     if mname = mid[/.*(?==\z)/m]
       if len != 1
         raise ArgumentError, "wrong number of arguments (given #{len}, expected 1)", caller(1)
       end
-      modifiable?[new_ostruct_member!(mname)] = args[0]
+      self[mname]= args[0]
     elsif len == 0
     elsif @table.key?(mid)
       raise ArgumentError, "wrong number of arguments (given #{len}, expected 0)"
@@ -240,7 +258,9 @@ class OpenStruct
   #   person.age          # => 42
   #
   def []=(name, value)
-    modifiable?[new_ostruct_member!(name)] = value
+    name = name.to_sym
+    new_ostruct_member!(name)
+    @table[name] = value
   end
 
   # :call-seq:

test/ostruct/test_ostruct.rb

@@ -225,9 +225,25 @@ class TC_OpenStruct < Test::Unit::TestCase
     end
   end
 
+  def test_access_undefined
+    os = OpenStruct.new
+    assert_nil os.foo
+  end
+
   def test_overriden_private_methods
     os = OpenStruct.new(puts: :foo, format: :bar)
     assert_equal(:foo, os.puts)
     assert_equal(:bar, os.format)
   end
+
+  def test_overriden_public_methods
+    os = OpenStruct.new(method: :foo, class: :bar)
+    assert_equal(:foo, os.method)
+    assert_equal(:bar, os.class)
+  end
+
+  def test_access_original_methods
+    os = OpenStruct.new(method: :foo)
+    assert_equal(os.object_id, os.method!(:object_id).call)
+  end
 end