Spike refactoring of arguments for Ruby 3

[wasabigeek]

Proposed changes:

• parse the invocation arguments / block once and then pass those down, instead of re-splatting/delegating them. In the future, this will let us keep the ruby2_keywords logic at the "entry points" (Mock#method_missing and StubbedMethod#define_new_method) instead of having to do it at multiple layers.
• have ParametersMatcher check against Invocation, so in the future Invocation could expose positional/keyword arguments instead of a single arguments array

If this seems reasonable, I'll work on breaking this into smaller PRs with proper tests.

[floehopper]

@wasabigeek Thanks for continuing to work on this. I'll try to give you some feedback on this as soon as I can.

[floehopper]

Sorry it's taken me a while to find time to review this. It all looks good to me and makes complete sense.

Smaller PRs with suitable tests would be much appreciated. I like to try to keep each commit small & atomic and with all the tests passing and with the commit note explaining as much as possible about the motivation behind the change.

Let me know if there's anything I can do to help.

[wasabigeek]

I'm going to try to go further with this spike to check some of my assumptions.

In particular, I realise that with strict matching of keyword arguments, we might not be able to support this apparently valid syntax:

# This is apparently valid, even in Ruby 3.
def foo(opt={});  end;
foo(key: 42)

# This matches the original method signature.
# `with` would recognise the param as a last positional hash. 
expects(:foo).with({ key: 42}) 
# However, this would not match, as `method_missing` would recognise the param as a keyword argument.
foo(key: 42)
# Only this would match
foo({ key: 42 })

I think the above result is OK, since we're effectively stricter than Ruby 3. What do you feel @floehopper?

[floehopper]

I'm going to try to go further with this spike to check some of my assumptions.

OK. Sounds good.

In particular, I realise that with strict matching of keyword arguments, we might not be able to support this apparently valid syntax:

# This is apparently valid, even in Ruby 3.
def foo(opt={});  end;
foo(key: 42)

# This matches the original method signature.
# `with` would recognise the param as a last positional hash. 
expects(:foo).with({ key: 42}) 
# However, this would not match, as `method_missing` would recognise the param as a keyword argument.
foo(key: 42)
# Only this would match
foo({ key: 42 })

I think the above result is OK, since we're effectively stricter than Ruby 3. What do you feel @floehopper?

Without having spent long thinking about it, that does sound OK. 👍

[wasabigeek]

I went back to passing ruby2_keywords down the chain and checking for that in a ParametersMatchers::Hash. I think this will be easier to:

• support a configuration for turning on "strict ruby 3 keyword matching", as I think we would only need to turn this on of off
• allow the existing hash matchers API (e.g. with(has_value(1))) to work for both last positional hashes and keyword arguments (albeit, ambiguously). If we split into keyword args, it means we'd need some gnarly logic to check both positional and keyword args.

One side note, there's a shim for ruby2_keywords, but it only works with ruby >= 2.0, which could be a sign that it wouldn't work for 1.9 and below. What do you think about dropping support for 1.9 @floehopper? I think if we really wanted to we could try to still support 1.9, but it might mean having to roll our own version of the shim (or have some way to require the shim only if ruby >=2.0?).

[floehopper]

Sorry for the slow response - I'm on holiday this week!

I went back to passing ruby2_keywords down the chain and checking for that in a ParametersMatchers::Hash. I think this will be easier to:

• support a configuration for turning on "strict ruby 3 keyword matching", as I think we would only need to turn this on of off
• allow the existing hash matchers API (e.g. with(has_value(1))) to work for both last positional hashes and keyword arguments (albeit, ambiguously). If we split into keyword args, it means we'd need some gnarly logic to check both positional and keyword args.

Sounds good to me. 👍

One side note, there's a shim for ruby2_keywords, but it only works with ruby >= 2.0, which could be a sign that it wouldn't work for 1.9 and below. What do you think about dropping support for 1.9 @floehopper? I think if we really wanted to we could try to still support 1.9, but it might mean having to roll our own version of the shim (or have some way to require the shim only if ruby >=2.0?).

I'm very open to dropping support for Ruby v1.9 since support for it ended on February 23, 2015! And if I decide against it, I'm happy to take responsibility for creating a new version of the shim.

[floehopper]

@wasabigeek Do the changes in #562 mean we can close this PR?