Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove OpenStruct references from Jbuilder #567

Merged
merged 1 commit into from
Sep 15, 2024
Merged

Remove OpenStruct references from Jbuilder #567

merged 1 commit into from
Sep 15, 2024
+1 −6

Conversation

mtsmfm
Copy link
Contributor

@mtsmfm mtsmfm commented May 17, 2024 •
edited
Loading

While I was looking at #562, I noticed OpenStruct never responses map and count method.

docker run rubylang/all-ruby ./all-ruby -e 'require "ostruct"; p [:map, :count].map {|m| OpenStruct.new({}).respond_to?(m) }'

ruby-1.6.0            [false, false]
...
ruby-3.3.1            [false, false]

ruby-0.49             -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                  exit 9
ruby-0.50             -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                      -e:1: syntax error
                  exit 9
ruby-0.51             -e:1: syntax error
                  exit 1
...
ruby-0.76             -e:1: syntax error
                  exit 1
ruby-0.95             -e:1:in `require': No such file to load -- ostruct
                  exit 1
ruby-0.99.4-961224    -e:1:in `require': No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.0-961225       -e:1:in `require': No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.0-971002       -e:1:in `require': LoadError: No such file to load -- ostruct
                        from -e:1
                  exit 1
...
ruby-1.0-971225       -e:1:in `require': LoadError: No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.1a0            -e:1:in `require': LoadError| No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.1a1            -e:1:in `require': LoadError| No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.1a2            -e:1:in `require': LoadError:No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.1a3            -e:1:in `require': LoadError: No such file to load -- ostruct
                        from -e:1
                  exit 1
...
ruby-1.1a9            -e:1:in `require': LoadError: No such file to load -- ostruct
                        from -e:1
                  exit 1
ruby-1.1b0            -e:1:in `require': No such file to load -- ostruct (LoadError)
                        from -e:1
                  exit 1
ruby-1.1b1            -e:1:in `require': No such file to load -- ostruct (LoadError)
                        from -e:1
                  exit 1
ruby-1.1b2            -e:1: undefined iterator `map' for [8017, 8025](Array) (NameError)
                  exit 1
ruby-1.1b3            -e:1: undefined iterator `map' for [8017, 8025](Array) (NameError)
                  exit 1
ruby-1.1b4            -e:1: undefined iterator `map' for [8017, 8025] (NameError)
                  exit 1
ruby-1.1b5            -e:1: undefined iterator `map' for [8025, 8033] (NameError)
                  exit 1
ruby-1.1b6            -e:1: undefined iterator `map' for [8001, 8009] (NameError)
                  exit 1
ruby-1.1b7            -e:1: undefined iterator `map' for [8065, 8073] (NameError)
                  exit 1
ruby-1.1b8            -e:1: undefined iterator `map' for [8081, 8089] (NameError)
                  exit 1
ruby-1.1b9            -e:1: undefined iterator `map' for [8081, 8089] (NameError)
                  exit 1
ruby-1.1b9_01         -e:1: undefined iterator `map' for [8121, 8129] (NameError)
                  exit 1
...
ruby-1.1b9_03         -e:1: undefined iterator `map' for [8121, 8129] (NameError)
                  exit 1
ruby-1.1b9_04         -e:1: undefined iterator `map' for [8145, 8153] (NameError)
                  exit 1
ruby-1.1b9_05         -e:1: undefined iterator `map' for [8161, 8169] (NameError)
                  exit 1
ruby-1.1b9_06         -e:1: undefined iterator `map' for [8177, 8185] (NameError)
                  exit 1
ruby-1.1b9_07         -e:1: undefined iterator `map' for [8241, 8249] (NameError)
                  exit 1
ruby-1.1b9_08         -e:1: undefined iterator `map' for [8249, 8257] (NameError)
                  exit 1
ruby-1.1b9_09         -e:1: undefined iterator `map' for [8241, 8249] (NameError)
                  exit 1
ruby-1.1b9_10         -e:1: undefined iterator `map' for [8265, 8273] (NameError)
                  exit 1
ruby-1.1b9_11         -e:1: undefined iterator `map' for [8281, 8289] (NameError)
                  exit 1
ruby-1.1b9_12         -e:1: undefined iterator `map' for [8289, 8297] (NameError)
                  exit 1
...
ruby-1.1b9_14         -e:1: undefined iterator `map' for [8289, 8297] (NameError)
                  exit 1
ruby-1.1b9_15         -e:1: undefined iterator `map' for [8305, 8313] (NameError)
                  exit 1
ruby-1.1b9_16         -e:1: undefined iterator `map' for [8321, 8329] (NameError)
                  exit 1
ruby-1.1b9_17         /tmp/rbeOxo3r:1: undefined iterator `map' for [8353, 8361] (NameError)
                  exit 1
ruby-1.1b9_18         /tmp/rbLXgKTu:1: undefined iterator `map' for [8361, 8369] (NameError)
                  exit 1
ruby-1.1b9_19         /tmp/rbzff2Rx:1: NameError
                  exit 1
ruby-1.1b9_20         /tmp/rbusengG:1: NameError
                  exit 1
ruby-1.1b9_21         /tmp/rb6y6OiI:1: undefined iterator `map' for [8361, 8369] (NameError)
                  exit 1
ruby-1.1b9_22         /tmp/rb2ECrEP:1: undefined iterator `map' for [8361, 8369] (NameError)
                  exit 1
ruby-1.1b9_23         /tmp/rbRjC3NS:1: undefined iterator `map' for [8377, 8385] (NameError)
                  exit 1
ruby-1.1b9_24         /tmp/rbe7hi1Y:1: undefined iterator `map' for [8393, 8401] (NameError)
                  exit 1
ruby-1.1b9_25         /tmp/rbd1IlwZ:1: undefined iterator `map' for [8393, 8401] (NameError)
                  exit 1
ruby-1.1b9_26         /tmp/rbaCV6E5:1: undefined iterator `map' for [8417, 8425] (NameError)
                  exit 1
ruby-1.1b9_27         /tmp/rbtsOvQc:1: undefined iterator `map' for [8417, 8425] (NameError)
                  exit 1
ruby-1.1b9_28         /tmp/rboLvSeg:1: undefined iterator `map' for [8417, 8425] (NameError)
                  exit 1
ruby-1.1b9_29         /tmp/rb2jEekm:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1b9_30         /tmp/rbKwAYsn:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1b9_31         /tmp/rbxT9q7t:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1b9_32         /tmp/rbqIT5Yz:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c0            /tmp/rbx3kWwB:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c1            /tmp/rbkA6tQG:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c2            /tmp/rbjBlWzO:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c3            /tmp/rbhSOmQO:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c4            /tmp/rbcyRhDW:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c5            /tmp/rbDCXRs0:1: undefined iterator `map' for [8433, 8441] (NameError)
                  exit 1
ruby-1.1c6            /tmp/rbGaLFU3:1: undefined iterator `map' for [8161, 8169] (NameError)
                  exit 1
ruby-1.1c7            /tmp/rbEJdNUa:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.1c8            /tmp/rbqSgfCg:1: undefined iterator `map' for [8737, 8745] (NameError)
                  exit 1
ruby-1.1c9            /tmp/rb8cxY4g:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.1d0            /tmp/rbE2S8lo:1: undefined iterator `map' for [8241, 8249] (NameError)
                  exit 1
ruby-1.1d1            /tmp/rbVN0Fet:1: undefined iterator `map' for [8241, 8249] (NameError)
                  exit 1
ruby-1.2              /tmp/rbZn9F4u:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.2.1            /tmp/rbgpMTvB:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.2.2            /tmp/rbsm7VeF:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.2.3            /tmp/rbfnaYEL:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.2.4            /tmp/rbXrp0aQ:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.2.5            /tmp/rbw3le0S:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.2.6            /tmp/rbuoXmWY:1: undefined iterator `map' for [8169, 8177] (NameError)
                  exit 1
ruby-1.3              /tmp/rbH4Vxa2:1: undefined iterator `map' for [8241, 8249] (NameError)
                  exit 1
ruby-1.3.1-990215     /tmp/rbJiv876:1: undefined iterator `map' for [8409, 8417] (NameError)
                  exit 1
ruby-1.3.1-990224     /tmp/rbGx9f0d:1: undefined iterator `map' for [8417, 8425] (NameError)
                  exit 1
ruby-1.3.1-990225     /tmp/rbpkHQFh:1: undefined iterator `map' for [8417, 8425] (NameError)
                  exit 1
ruby-1.3.1-990311     /tmp/rbEhjCXo:1: undefined iterator `map' for [8473, 8481] (NameError)
                  exit 1
ruby-1.3.1-990315     /tmp/rbrcYwYq:1: undefined iterator `map' for [8497, 8505] (NameError)
                  exit 1
ruby-1.3.1-990324     /tmp/rb5U9Lmu:1: undefined iterator `map' for [8513, 8521] (NameError)
                  exit 1
ruby-1.3.2-990402     /tmp/rbJp4WPy:1: undefined iterator `map' for [8529, 8537] (NameError)
                  exit 1
ruby-1.3.2-990405     /tmp/rbmhN9zD:1: undefined iterator `map' for [8529, 8537] (NameError)
                  exit 1
ruby-1.3.2-990408     /tmp/rbDtyMjL:1: undefined iterator `map' for [8529, 8537] (NameError)
                  exit 1
ruby-1.3.2-990413     /tmp/rbzOHQlP:1: undefined iterator `map' for [8529, 8537] (NameError)
                  exit 1
ruby-1.3.3-990430     /tmp/rbFjs8TT:1: undefined iterator `map' for [8537, 8545] (NameError)
                  exit 1
ruby-1.3.3-990507     /tmp/rbP7X9VX:1: undefined iterator `map' for [8537, 8545] (NameError)
                  exit 1
ruby-1.3.3-990513     /tmp/rbuVw2S3:1: undefined iterator `map' for [8537, 8545] (NameError)
                  exit 1
ruby-1.3.3-990518     /tmp/rbkTUut6:1: undefined iterator `map' for [8545, 8553] (NameError)
                  exit 1
ruby-1.3.4-990531     /tmp/rbAKdS4b:1: undefined iterator `map' for [8553, 4225] (NameError)
                  exit 1
ruby-1.3.4-990611     /tmp/rb1IMDCf:1: undefined iterator `map' for [8585, 4233] (NameError)
                  exit 1
ruby-1.3.4-990624     /tmp/rbHKJQ8k:1: undefined iterator `map' for [8585, 4233] (NameError)
                  exit 1
ruby-1.3.4-990625     /tmp/rbatRIFp:1: undefined iterator `map' for [8585, 4233] (NameError)
                  exit 1
ruby-1.3.5            /tmp/rbV1nV7t:1: undefined method `map' for [8585, 4233]:Array (NameError)
                  exit 1
ruby-1.3.6            /tmp/rbmbWPQy:1: undefined method `map' for [8585, 4233]:Array (NameError)
                  exit 1
ruby-1.3.7            /tmp/rbx0D9fG:1: undefined method `map' for [8593, 4233]:Array (NameError)
                  exit 1
ruby-1.4.0            /tmp/rbTiBWPH:1: undefined method `map' for [8593, 4233]:Array (NameError)
                  exit 1
ruby-1.4.1            /tmp/rbatNtEO:1: undefined method `map' for [8593, 4233]:Array (NameError)
                  exit 1
ruby-1.4.2            /tmp/rbbhuiuQ:1: undefined method `map' for [8593, 4233]:Array (NameError)
                  exit 1
ruby-1.4.3            /tmp/rbW4ACmW:1: undefined method `map' for [8593, 4233]:Array (NameError)
                  exit 1
ruby-1.4.4            /tmp/rb5aEcr3:1: undefined method `map' for [8593, 4233]:Array (NameError)
                  exit 1
ruby-1.4.5            /tmp/rbjJF5T6:1: undefined method `map' for [8601, 4233]:Array (NameError)
                  exit 1
ruby-1.4.6            /tmp/rbJxuXgb:1: undefined method `map' for [8601, 4233]:Array (NameError)
                  exit 1
ruby-1.6.0            [false, false]
...
ruby-3.3.1            [false, false]

So, even we remove OpenStruct related codes completely, Jbuilder can handle them

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  if ENV['FORK']
    gem 'jbuilder', path: '.'
  else
    gem 'jbuilder'
  end
end

puts "Jbuilder::NON_ENUMERABLES: #{Jbuilder.const_defined?("NON_ENUMERABLES")}"

require 'ostruct'

ostruct = OpenStruct.new(a: 1)
struct = Struct.new(:a).new(a: 1)

array = [{a: 1}]

result = Jbuilder.new do |json|
  json.array array, :a
  json.struct struct, :a
  json.ostruct ostruct, :a
  json.array_struct [struct], :a
  json.array_ostruct [ostruct], :a
end.target!

if JSON.parse(result) == {"array"=>[{"a"=>1}], "struct"=>{"a"=>1}, "ostruct"=>{"a"=>1}, "array_struct"=>[{"a"=>1}], "array_ostruct"=>[{"a"=>1}]}
  puts 'Passed'
else
  puts 'Failed'
end

__END__
$ ruby foo.rb
Jbuilder::NON_ENUMERABLES: true
Passed

$ FORK=1 ruby foo.rb
Jbuilder::NON_ENUMERABLES: false
Passed

But if an OpenStruct instance has both map and count fields, it would be broken.

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  if ENV['FORK']
    gem 'jbuilder', path: '.'
  else
    gem 'jbuilder'
  end
end

puts "Jbuilder::NON_ENUMERABLES: #{Jbuilder.const_defined?("NON_ENUMERABLES")}"

require 'ostruct'

ostruct = OpenStruct.new(a: 1, count: 1, map: 1)

result = Jbuilder.new do |json|
  json.ostruct ostruct, :a
end.target!

if JSON.parse(result) == {"ostruct"=>{"a"=>1}}
  puts 'Passed'
else
  puts 'Failed'
end

__END__

$ ruby broken.rb
Jbuilder::NON_ENUMERABLES: true
Passed

$ FORK=1 ruby broken.rb
Jbuilder::NON_ENUMERABLES: false
/workspaces/jbuilder/lib/jbuilder.rb:339:in `-': Array can't be coerced into Integer (TypeError)

    end - [BLANK]
          ^^^^^^^
        from /workspaces/jbuilder/lib/jbuilder.rb:339:in `_map_collection'
        from /workspaces/jbuilder/lib/jbuilder.rb:217:in `array!'
        from /workspaces/jbuilder/lib/jbuilder.rb:56:in `block in set!'
        from /workspaces/jbuilder/lib/jbuilder.rb:345:in `_scope'
        from /workspaces/jbuilder/lib/jbuilder.rb:56:in `set!'
        from /workspaces/jbuilder/lib/jbuilder.rb:70:in `method_missing'
        from broken.rb:20:in `block in <main>'
        from /workspaces/jbuilder/lib/jbuilder.rb:21:in `initialize'
        from broken.rb:19:in `new'
        from broken.rb:19:in `<main>'

So, strictly speaking, this change is a breaking change.

What do you think?

If you don't agree with this change, you can just close this PR.
I just want to tell you another possibility.

@mtsmfm
Copy link
Contributor Author

mtsmfm commented May 30, 2024

With Ruby 3.4, a newly created Rails application immediately shows a warning about ostruct

$ docker run --rm -it ruby:3.4.0-preview1 bash -c "gem install rails && rails new foo && cd foo && echo test && bin/rails r 'puts %(hi)'"
(snip)
test
/usr/local/bundle/gems/bootsnap-1.18.3/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30: warning: /usr/local/lib/ruby/3.4.0+0/ostruct.rb was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0. Add ostruct to your Gemfile or gemspec.
hi

With this patch, such warning won't be appeared since it completely removes the require thing for ostruct.

$ docker run --rm -it ruby:3.4.0-preview1 bash -c "gem install rails && rails new foo && cd foo && sed -i 's;gem \"jbuilder\";gem \"jbuilder\", github: \"mtsmfm/jbuilder\", branch: \"remove-ostruct\";' Gemfile && bundle install && echo test && bin/rails r 'puts %(hi)'"
(snip)
test
hi

@yahonda
Copy link
Member

yahonda commented Jun 9, 2024

As a author of #562 ,
I think this pull request should be better because #562 would support Ruby 3.5 that will remove ostruct. however, it will still show the warning below when Ruby 3.4 user executes rails new with Jbuilder.

`ostruct.rb was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0. Add ostruct to your Gemfile or gemspec.

@yahonda
Copy link
Member

yahonda commented Jun 9, 2024

I would also like to hear from other maintainers.

@mattbrictson
Copy link

it will still show the warning below when Ruby 3.4

It looks like Ruby 3.3.5 (released today) emits the warning as well.

/Users/mbrictson/.rbenv/versions/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75: warning: /Users/mbrictson/.rbenv/versions/3.3.5/lib/ruby/3.3.0/ostruct.rb was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add ostruct to your Gemfile or gemspec to silence this warning.
Also please contact the author of jbuilder-2.12.0 to request adding ostruct into its gemspec.

This was referenced Sep 4, 2024
Closed
Closed
@yahonda
Copy link
Member

yahonda commented Sep 15, 2024

It looks like Ruby 3.3.5 (released today) emits the warning as well.

According to https://bugs.ruby-lang.org/issues/20737 , this warning will not appear in Ruby 3.3.6 and future 3.3 versions of Rubies.

@dhh dhh merged commit 5288e8a into rails:main Sep 15, 2024
38 checks passed
@mtsmfm mtsmfm deleted the remove-ostruct branch September 16, 2024 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mtsmfm @yahonda @mattbrictson @dhh