Skip to content

Make Module#prepend affect the iclasses of the module - #3181

Merged
jeremyevans merged 1 commit into
ruby:masterfrom
jeremyevans:prepend-future-ancestors-9573
Jun 18, 2020
Merged

Make Module#prepend affect the iclasses of the module#3181
jeremyevans merged 1 commit into
ruby:masterfrom
jeremyevans:prepend-future-ancestors-9573

Conversation

@jeremyevans

Copy link
Copy Markdown
Contributor

3556a83 added support for
Module#include to affect the iclasses of the module. It didn't add
support for Module#prepend because there were bugs in the object model
and GC at the time that prevented it. Those problems have been
addressed in ad729a1 and
98286e9, and now adding support for
it is straightforward and does not break any tests or specs.

Fixes [Bug #9573]

3556a83 added support for
Module#include to affect the iclasses of the module.  It didn't add
support for Module#prepend because there were bugs in the object model
and GC at the time that prevented it.  Those problems have been
addressed in ad729a1 and
98286e9, and now adding support for
it is straightforward and does not break any tests or specs.

Fixes [Bug ruby#9573]
@jeremyevans
jeremyevans merged commit 41582d5 into ruby:master Jun 18, 2020
yahonda added a commit to yahonda/rails that referenced this pull request Jun 22, 2020
…Hash(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json for Ruby 2.8.0

This pull request addresses failures at https://buildkite.com/rails/rails/builds/70219#79d96882-6c51-4854-8cab-28f50ac8bca1
According to https://bugs.ruby-lang.org/issues/16973 This is an expected change in Ruby.
These failures has been addressed by changing the order of prepend as suggested.

```diff
% git diff
diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb
index 30a3b8e..1328041bf7 100644
--- a/activesupport/test/json/encoding_test.rb
+++ b/activesupport/test/json/encoding_test.rb
@@ -186,6 +186,8 @@ def test_hash_should_pass_encoding_options_to_children_in_to_json
         country: "UK"
       }
     }
+    p person.method(:to_json)
+    pp person.class.ancestors
     json = person.to_json only: [:address, :city]

     assert_equal(%({"address":{"city":"London"}}), json)
@@ -287,6 +289,8 @@ def test_array_to_json_should_not_keep_options_around
     f.bar = "world"

     array = [f, { "foo" => "other_foo", "test" => "other_test" }]
+    p array.method(:to_json)
+    pp array.class.ancestors
     assert_equal([{ "foo" => "hello", "bar" => "world" },
                   { "foo" => "other_foo", "test" => "other_test" }], ActiveSupport::JSON.decode(array.to_json))
   end
%
```

* Ruby 2.8.0 without this fix uses `Array(JSON::Ext::Generator::GeneratorMethods::Array)#to_json`, which should use `Array(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json`

```
% bin/test test/json/encoding_test.rb -n test_array_to_json_should_not_keep_options_around
Run options: -n test_array_to_json_should_not_keep_options_around --seed 33311

[Array,
 JSON::Ext::Generator::GeneratorMethods::Array,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Enumerable,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Object,
 JSON::Ext::Generator::GeneratorMethods::Object,
 ActiveSupport::Tryable,
 Kernel,
 BasicObject]
F

Failure:
TestJSONEncoding#test_array_to_json_should_not_keep_options_around [/Users/yahonda/src/github.com/rails/rails/activesupport/test/json/encoding_test.rb:294]:
--- expected
+++ actual
@@ -1 +1 @@
-[{"foo"=>"hello", "bar"=>"world"}, {"foo"=>"other_foo", "test"=>"other_test"}]
+["#<TestJSONEncoding::CustomWithOptions:0xXXXXXX>", {"foo"=>"other_foo", "test"=>"other_test"}]

bin/test test/json/encoding_test.rb:286

Finished in 0.015486s, 64.5745 runs/s, 64.5745 assertions/s.
1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
%
```

* Ruby 2.8.0 with this fix uses `Array(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json`

```
% bin/test test/json/encoding_test.rb -n test_array_to_json_should_not_keep_options_around
Run options: -n test_array_to_json_should_not_keep_options_around --seed 12193

[ActiveSupport::ToJsonWithActiveSupportEncoder,
 Array,
 JSON::Ext::Generator::GeneratorMethods::Array,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Enumerable,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Object,
 JSON::Ext::Generator::GeneratorMethods::Object,
 ActiveSupport::Tryable,
 Kernel,
 BasicObject]
.

Finished in 0.008070s, 123.9157 runs/s, 123.9157 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
%
```

* Ruby 2.8.0 without this fix uses `Hash(JSON::Ext::Generator::GeneratorMethods::Hash)#to_json`, which should use `Hash(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json`

```
% bin/test test/json/encoding_test.rb -n test_hash_should_pass_encoding_options_to_children_in_to_json

Run options: -n test_hash_should_pass_encoding_options_to_children_in_to_json --seed 18064

[Hash,
 JSON::Ext::Generator::GeneratorMethods::Hash,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Enumerable,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Object,
 JSON::Ext::Generator::GeneratorMethods::Object,
 ActiveSupport::Tryable,
 Kernel,
 BasicObject]
F

Failure:
TestJSONEncoding#test_hash_should_pass_encoding_options_to_children_in_to_json [/Users/yahonda/src/github.com/rails/rails/activesupport/test/json/encoding_test.rb:193]:
--- expected
+++ actual
@@ -1 +1 @@
-"{\"address\":{\"city\":\"London\"}}"
+"{\"name\":\"John\",\"address\":{\"city\":\"London\",\"country\":\"UK\"}}"

bin/test test/json/encoding_test.rb:181

Finished in 0.015009s, 66.6267 runs/s, 66.6267 assertions/s.
1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
%
```

* Ruby 2.8.0 with this fix uses `Hash(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json`

```
% bin/test test/json/encoding_test.rb -n test_hash_should_pass_encoding_options_to_children_in_to_json

Run options: -n test_hash_should_pass_encoding_options_to_children_in_to_json --seed 56794

[ActiveSupport::ToJsonWithActiveSupportEncoder,
 Hash,
 JSON::Ext::Generator::GeneratorMethods::Hash,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Enumerable,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Object,
 JSON::Ext::Generator::GeneratorMethods::Object,
 ActiveSupport::Tryable,
 Kernel,
 BasicObject]
.

Finished in 0.007434s, 134.5171 runs/s, 134.5171 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
%
```

Refer
ruby/ruby#3181
ruby/ruby#2936
https://bugs.ruby-lang.org/issues/9573
rails#19413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant