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

Incorrect parameters for core methods messes up RSpec proxies #5664

Closed
headius opened this Issue Mar 23, 2019 · 2 comments

Comments

Projects
None yet
1 participant
@headius
Copy link
Member

headius commented Mar 23, 2019

Our core methods report the incorrect parameters when wrapped in a Method or UnboundMethod.

[] ~/projects/jruby/hutch $ rvm ruby-2.5.3 do ruby -e 'class IO; m = instance_method(:write); p m.parameters; define_method(:write, m); end; $stderr.puts IO.instance_method(:write).parameters'
[[:rest]]
rest

[] ~/projects/jruby/hutch $ jruby -e 'class IO; m = instance_method(:write); p m.parameters; define_method(:write, m); end; $stderr.puts IO.instance_method(:write).parameters'
[[:req]]
req

As shown here, for an unlimited-arity method like IO#write we report :req which means single required argument. However the actual #arity method reports -1, indicating varargs.

This is the cause of issues reported by @olleolleolle on our chat. In his case, this bug caused RSpec's signature-verification (which appears to be used by mocking/proxying method logic) to attempt to verify that the the mock method has the same arity as the original, using parameters to build up a parameter description. This description said that IO#write should only take one argument, so a two-argument call fails to verify and it blows up within RSpec internals.

#<ArgumentError: Wrong number of arguments. Expected 1, got 2.>
#<Errno::ENOENT: No such file or directory - /path/to/nonexistant/file>
["/Users/olle/.rvm/gems/jruby-9.2.6.0/gems/rspec-mocks-3.8.0/lib/rspec/mocks/verifying_proxy.rb:167:in `block in validate_arguments!'",
 "/Users/olle/.rvm/gems/jruby-9.2.6.0/gems/rspec-mocks-3.8.0/lib/rspec/mocks/verifying_proxy.rb:191:in `with_signature'",
 "/Users/olle/.rvm/gems/jruby-9.2.6.0/gems/rspec-mocks-3.8.0/lib/rspec/mocks/verifying_proxy.rb:165:in `validate_arguments!'",
 "/Users/olle/.rvm/gems/jruby-9.2.6.0/gems/rspec-mocks-3.8.0/lib/rspec/mocks/verifying_proxy.rb:160:in `proxy_method_invoked'",
 "/Users/olle/.rvm/gems/jruby-9.2.6.0/gems/rspec-mocks-3.8.0/lib/rspec/mocks/method_double.rb:64:in `block in define_proxy_method'",
 "org/jruby/RubyIO.java:2638:in `write'",

Although this looks like one of our ArgumentError message, it actually comes from RSpec's signature verifier.

The root cause was this line in DescriptorInfo:

This line assumes that if the minimum number of arguments matches the maximum number of arguments, it must be a fixed number of arguments, in which case it should just be a single :req symbol. However in this logic, if the maximum and minimum are both -1, that means it's a rest arg.

I have a fix in process locally. I will also try to start reducing the duplication between that class and ArgumentType.

@headius headius added this to the JRuby 9.2.7.0 milestone Mar 23, 2019

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Mar 23, 2019

Hmm this may be a bit deeper...it seems to be calculating the rest flag incorrectly.

@headius headius closed this in 701e04b Mar 23, 2019

headius added a commit that referenced this issue Mar 23, 2019

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Mar 23, 2019

I modified the logic to take note of the "rest" flag, which is set to true if any methods have "varArgs" flag set. I believe the latter flag is not being or'ed right.

eregon added a commit to ruby/spec that referenced this issue Mar 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.