Argument Error when using keyword args in matcher block

paul commented 2 years ago

Describe the bug

In our application we make heavy use of Transactions, the vast majority of which operate with kwargs and return Hash objects. In several places, we also use keyword block arguments within the match block when calling the transaction (see example).

We're in the process of upgrading to Ruby 3.x, and have run in to an issue with those keyword args. The code works in 2.7.x, and issues no kwarg warnings, but fails with ArgumentError in 3.x. I've distilled it down to a simple example attached below.

I know this can likely be replaced with Ruby 3 pattern matching, but doing so touches a lot of code in our app, and I'm hoping to avoid making too many code changes while doing this upgrade. Would a PR be accepted to fix this, if I were to attempt it?

To Reproduce

require "dry-transaction"

class MyTransaction
  include Dry::Transaction

  step :foo

  def foo(...)
    Success({ foo: 42 })  # or Success(foo: 42)
  end
end

MyTransaction.new.call do |on|
  on.success { |foo:, **| puts foo }
  on.failure {}
end

Expected behavior

Ruby 2.7.5:

$ ruby --version
ruby 2.7.5p203 (2021-11-24 revision f69aeb8314) [x86_64-linux]

$ ruby ruby-3-transaction.rb
42

Ruby 3.0.3:

$ ruby --version
ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-linux]

$ ruby ruby-3-transaction.rb
ruby-3-transaction.rb:16:in `block (2 levels) in <main>': missing keyword: :foo (ArgumentError)
    from /home/rando/.gem/ruby/3.0.3/gems/dry-matcher-0.9.0/lib/dry/matcher/evaluator.rb:55:in `block in method_missing'
    from /home/rando/.gem/ruby/3.0.3/gems/dry-core-0.7.1/lib/dry/core/constants.rb:88:in `map'
    from /home/rando/.gem/ruby/3.0.3/gems/dry-matcher-0.9.0/lib/dry/matcher/case.rb:32:in `call'
    from /home/rando/.gem/ruby/3.0.3/gems/dry-matcher-0.9.0/lib/dry/matcher/evaluator.rb:54:in `method_missing'
    from ruby-3-transaction.rb:16:in `block in <main>'
    from /home/rando/.gem/ruby/3.0.3/gems/dry-matcher-0.9.0/lib/dry/matcher/evaluator.rb:19:in `call'
    from /home/rando/.gem/ruby/3.0.3/gems/dry-matcher-0.9.0/lib/dry/matcher.rb:91:in `call'
    from /home/rando/.gem/ruby/3.0.3/gems/dry-transaction-0.13.3/lib/dry/transaction/instance_methods.rb:33:in `call'
    from ruby-3-transaction.rb:15:in `<main>'

My environment

Affects my production application: NOT YET
Ruby version: 3.0.3
OS: Linux

This seems like its actually an error with dry-matcher and kwargs, because this code results in the same error:

require "dry/monads/result"
require "dry/matcher/result_matcher"

value = Dry::Monads::Success(foo: 42)

Dry::Matcher::ResultMatcher.call(value) do |on|
  on.success { |foo:| puts foo }
  on.failure {}
end

__END__

$ ruby ruby-3-transaction.rb
ruby-3-transaction.rb:25:in `block (2 levels) in <main>': missing keyword: :foo (ArgumentError)
    from /home/rando/.gem/ruby/3.0.3/gems/dry-matcher-0.9.0/lib/dry/matcher/evaluator.rb:55:in `block in method_missing'

flash-gordon commented 2 years ago

Values are not passed as keywords to blocks so this is kinda expected. It looks like you'll have to change the application code. Being smart and detecting if a block has keywords is problematic performance-wise I guess.

paul commented 2 years ago

@flash-gordon After poking around further, I'm not even sure its really possible in Ruby 3. Eg, this simplest possible example doesn't work any more:

>> {foo: 42}.tap { |foo:, **| puts foo }
(irb):1:in `block in <main>': missing keyword: :foo (ArgumentError)
    from <internal:kernel>:90:in `tap'

flash-gordon commented 2 years ago

It's possible when you define a method

def success
  yield('seq', key: 1, word: 2)
end

The problem is you'll have to know the block's parameters:

def success(&block)
  args, kwargs = split_by_parameters(@value, block.parameters)

  yield(*args, **kwargs)
end

So it can work but it'll be slow for all users, not only for those with keywords.

dry-rb / dry-transaction