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.

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

dry-initializer: `to_s': wrong number of arguments (1 for 0) #1462

Closed
deepj opened this issue Nov 7, 2018 · 9 comments
Closed

dry-initializer: `to_s': wrong number of arguments (1 for 0) #1462

deepj opened this issue Nov 7, 2018 · 9 comments
Assignees
Milestone

Comments

@deepj
Copy link

deepj commented Nov 7, 2018

This failed under truffleruby-1.0.0.rc9. No issue under MRI.

require 'dry-initializer'

class Test
  extend Dry::Initializer

  param :foo, proc(&:to_s)
end

Test.new(:FOO)

Error:

dry-initializer.rb:6:in `to_s': wrong number of arguments (1 for 0) (ArgumentError)
	from dry-initializer.rb:6:in `to_s'
	from (eval):5:in `call'
	from (eval):5:in `__dry_initializer_initialize__'
	from ~/.gem/truffleruby/2.4.4/gems/dry-initializer-2.5.0/lib/dry/initializer/mixin/root.rb:7:in `initialize'
	from dry-initializer.rb:9:in `new'
	from dry-initializer.rb:9:in `<main>'
@chrisseaton
Copy link
Collaborator

Thanks - we'll have to dig into the dry stack a bit for this.

@deepj
Copy link
Author

deepj commented Jan 25, 2019

@eregon Is possible to look at this issue, please? This issue affects the whole dry-rb/ROM ecosystem. It's some ugly issue with metaprogramming,

@eregon
Copy link
Member

eregon commented Jan 25, 2019

@deepj This variant seems to work fine:

require 'dry-initializer'

class Test
  extend Dry::Initializer

  # param :foo, proc(&:to_s)
  param :foo, &:to_s
end

p Test.new(:FOO)

@eregon
Copy link
Member

eregon commented Jan 25, 2019

Mmh, but that's different as it passes a block argument to param.

require 'dry-initializer'

class Test
  extend Dry::Initializer

  # param :foo, proc(&:to_s)
  param :foo, :to_s.to_proc
end

p Test.new(:FOO)

also fails.

@eregon eregon self-assigned this Jan 25, 2019
@eregon eregon added the bug label Jan 25, 2019
@eregon
Copy link
Member

eregon commented Jan 25, 2019

This works:

require 'dry-initializer'

class Test
  extend Dry::Initializer

  param :foo, -> obj { obj.to_s }
end

p Test.new(:FOO)
p Test.new(:FOO).foo # => "FOO"

The code generated by dry-initializer here differs based on the Proc#arity of a Symbol#to_proc Proc. TruffleRuby currently returns -2, MRI -1.

@eregon
Copy link
Member

eregon commented Jan 25, 2019

@deepj Fix coming.
FWIW, an arity of -2 would better express the number of required arguments: Symbol#to_proc needs at least one argument (the receiver).
An arity of -1 means "0 or more arguments".
:to_s.to_proc.call gives ArgumentError: no receiver given on MRI, showing at least one argument is indeed required.

So it might be a MRI bug, on which dry-initializer depends. I'll report it to them.
In any case, I'll follow MRI semantics for now for compatibility.

@eregon
Copy link
Member

eregon commented Jan 25, 2019

I filed dry-rb/dry-initializer#52.

dougxc pushed a commit that referenced this issue Jan 25, 2019
* dry-initializer relies on the arity being -1, see
  #1462
  dry-rb/dry-initializer#52
@dougxc dougxc closed this as completed in efd67a7 Jan 25, 2019
@eregon eregon added this to the 1.0.0-rc12 milestone Jan 25, 2019
@eregon
Copy link
Member

eregon commented Jan 25, 2019

@deepj Fixed, it should be in the next release. Thank you for the report and the ping!

@deepj
Copy link
Author

deepj commented Jan 25, 2019

@eregon thank you! it's great this has been fixed. I can't wait on running dry-rb/ROM on TruffleRuby. If you would have another time, this is another issue in that ecosystems. #1469

I'm not sure how this can be relative to this issue.

eregon added a commit to ruby/spec that referenced this issue Jan 27, 2019
* dry-initializer relies on the arity being -1, see
  oracle/truffleruby#1462
  dry-rb/dry-initializer#52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants