Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Fix and_call_original for Ruby 3.2 #1552

Merged

Conversation

igor-drozdov
Copy link
Contributor

When the original method isn't present, a proc that sends method_missing is returned. This block must support ruby2_keywords in order to work correctly with Ruby 3.2.

Otherwise, the following error is raised

     Failure/Error:
       def existing_method_with_args(arg, kwarg:)
         [arg, kwarg]
       end

     ArgumentError:
       wrong number of arguments (given 2, expected 1; required keyword: kwarg)


it 'works for the method propagated by method missing' do
expect(instance).to receive(:existing_method).with(:arg, kwarg: 1).and_call_original
expect(instance.existing_method(:arg, kwarg: 1)).to eq([:arg, 1])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test actually reproduces the error in Ruby 3.2

     Failure/Error:
       def existing_method_with_args(arg, kwarg:)
         [arg, kwarg]
       end

     ArgumentError:
       wrong number of arguments (given 2, expected 1; required keyword: kwarg)

@igor-drozdov igor-drozdov force-pushed the ruby2-keywords-for-missing-method-double branch 2 times, most recently from 623ee32 to fa671b0 Compare July 5, 2023 15:18
Comment on lines 251 to 259
def existing_method_with_args(arg, kwarg:)
[arg, kwarg]
end

def method_missing(name, *args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to eval these from strings conditionally to pass our build

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonRowe thanks, done! Let's see if it helps 👍

@igor-drozdov igor-drozdov force-pushed the ruby2-keywords-for-missing-method-double branch from fa671b0 to e262bf4 Compare July 5, 2023 16:28
if name.to_s == "greet_jack"
"Hello, jack"
elsif name.to_s == "method_with_kwarg"
call_method_with_kwarg(*args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also need to be wrapped to pass our build, until we release RSpec 4 we support pre keyword rubies (sorry)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonRowe ah, got it, I've changed the code, and now all the new examples are evaluated conditionally 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby 2.0 failed because the kwarg doesn't have a default value, I changed the condition to run only when required-kwarg are available. Hope it will pass now

@igor-drozdov igor-drozdov force-pushed the ruby2-keywords-for-missing-method-double branch 2 times, most recently from 828ba9d to f43911b Compare July 5, 2023 19:31
When the original method isn't present, a proc that sends
method_missing is returned. This block must support ruby2_keywords
in order to work correctly with Ruby 3.2
@igor-drozdov igor-drozdov force-pushed the ruby2-keywords-for-missing-method-double branch from f43911b to c926345 Compare July 5, 2023 21:40
@JonRowe JonRowe merged commit 8fa2c12 into rspec:main Jul 6, 2023
@JonRowe
Copy link
Member

JonRowe commented Jul 6, 2023

Thanks!

@igor-drozdov igor-drozdov deleted the ruby2-keywords-for-missing-method-double branch July 6, 2023 07:19
JonRowe added a commit that referenced this pull request Jul 6, 2023
JonRowe added a commit that referenced this pull request Jul 11, 2023
…-method-double

Fix and_call_original for Ruby 3.2
JonRowe added a commit that referenced this pull request Jul 11, 2023
@JonRowe
Copy link
Member

JonRowe commented Jul 11, 2023

Released in 3.12.6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants