egonSchiele / contracts.ruby

Contracts for Ruby.
http://egonschiele.github.com/contracts.ruby
BSD 2-Clause "Simplified" License
1.44k stars 82 forks source link

Test fails on ruby 3.0 due to keyword argument separation #294

Closed mtasaka closed 3 years ago

mtasaka commented 3 years ago

With using ruby 3.0.0p0, current contracts.ruby https://github.com/egonSchiele/contracts.ruby/commit/cf8775a9d8af79271daf9889d71dac60daedef49 fails on test suite, perhaps due to ruby 3.0 keyword argument separation: https://rubyreferences.github.io/rubychanges/3.0.html#keyword-arguments-are-now-fully-separated-from-positional-arguments

[mockbuild@158a2a164d014af48811d45adaaa9bd6 contracts.ruby]$ rspec spec
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 4683
Failures:

  1) Contracts: Nat: should pass for keyword args with correct arg given
     Failure/Error: expect { @o.nat_test_with_kwarg(foo: 10) }.to_not raise_error

       expected no Exception, got #<ArgumentError: wrong number of arguments (given 1, expected 0)> with backtrace:
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:8:in `nat_test_with_kwarg'
         # ./lib/contracts/method_reference.rb:43:in `send_to'
         # ./lib/contracts/call_with.rb:87:in `call_with_inner'
         # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
         # ./lib/contracts/method_handler.rb:132:in `each'
         # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:34:in `block (4 levels) in <top (required)>'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:34:in `block (3 levels) in <top (required)>'
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:34:in `block (3 levels) in <top (required)>'

  2) Contracts: Optional named arguments should work with optional named argument unfilled after splat
     Failure/Error: expect { @o.splat_then_optional_named("hello", "world") }.to_not raise_error

       expected no Exception, got #<NoMethodError: undefined method `*' for {}:Hash> with backtrace:
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:4:in `block in splat_then_optional_named'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:4:in `map'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:4:in `splat_then_optional_named'
         # ./lib/contracts/method_reference.rb:43:in `send_to'
         # ./lib/contracts/call_with.rb:87:in `call_with_inner'
         # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
         # ./lib/contracts/method_handler.rb:132:in `each'
         # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:24:in `block (4 levels) in <top (required)>'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:24:in `block (3 levels) in <top (required)>'
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:24:in `block (3 levels) in <top (required)>'

  3) Contracts: Optional named arguments should work with optional named argument filled after splat
     Failure/Error: expect { @o.splat_then_optional_named("hello", "world", repeat: 3) }.to_not raise_error

       expected no Exception, got #<NoMethodError: undefined method `*' for {:repeat=>3}:Hash> with backtrace:
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:4:in `block in splat_then_optional_named'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:4:in `map'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:4:in `splat_then_optional_named'
         # ./lib/contracts/method_reference.rb:43:in `send_to'
         # ./lib/contracts/call_with.rb:87:in `call_with_inner'
         # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
         # ./lib/contracts/method_handler.rb:132:in `each'
         # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:28:in `block (4 levels) in <top (required)>'
         # ./spec/ruby_version_specific/contracts_spec_2.0.rb:28:in `block (3 levels) in <top (required)>'
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:28:in `block (3 levels) in <top (required)>'

  4) Contracts: keyword args with defaults, with a block should work even when keyword args aren't given
     Failure/Error:
       def keyword_args_hello(name: "Adit", &block)
         "Hey, #{yield name}!"
       end

     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:12:in `keyword_args_hello'
     # ./lib/contracts/method_reference.rb:43:in `send_to'
     # ./lib/contracts/call_with.rb:87:in `call_with_inner'
     # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
     # ./lib/contracts/method_handler.rb:132:in `each'
     # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:52:in `block (3 levels) in <top (required)>'

  5) Contracts: keyword args with defaults, with a block should work when both keyword args and a block is given
     Failure/Error:
       def keyword_args_hello(name: "Adit", &block)
         "Hey, #{yield name}!"
       end

     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:12:in `keyword_args_hello'
     # ./lib/contracts/method_reference.rb:43:in `send_to'
     # ./lib/contracts/call_with.rb:87:in `call_with_inner'
     # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
     # ./lib/contracts/method_handler.rb:132:in `each'
     # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
     # ./spec/ruby_version_specific/contracts_spec_2.0.rb:48:in `block (3 levels) in <top (required)>'

  6) Contracts: Required named arguments really complicated method signature should work with all args filled manually, with extra splat and hash
     Failure/Error:
       expect do
         @o.complicated("a", true, :b, :c, 2.0, e: (1..5), f: 8.3, g: :d) do |x|
           x
         end
       end.to_not raise_error

       expected no Exception, got #<ArgumentError: missing keyword: :e> with backtrace:
         # ./spec/ruby_version_specific/contracts_spec_2.1.rb:4:in `complicated'
         # ./lib/contracts/method_reference.rb:43:in `send_to'
         # ./lib/contracts/call_with.rb:87:in `call_with_inner'
         # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
         # ./lib/contracts/method_handler.rb:132:in `each'
         # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
         # ./spec/ruby_version_specific/contracts_spec_2.1.rb:24:in `block (5 levels) in <top (required)>'
         # ./spec/ruby_version_specific/contracts_spec_2.1.rb:23:in `block (4 levels) in <top (required)>'
     # ./spec/ruby_version_specific/contracts_spec_2.1.rb:23:in `block (4 levels) in <top (required)>'

  7) Contracts: Required named arguments really complicated method signature should work with default named args used
     Failure/Error:
       expect do
         @o.complicated("a", false, :b, 2.0, e: (1..5), g: :d) { |x| x }
       end.to_not raise_error

       expected no Exception, got #<ArgumentError: missing keyword: :e> with backtrace:
         # ./spec/ruby_version_specific/contracts_spec_2.1.rb:4:in `complicated'
         # ./lib/contracts/method_reference.rb:43:in `send_to'
         # ./lib/contracts/call_with.rb:87:in `call_with_inner'
         # ./lib/contracts/method_handler.rb:133:in `block (2 levels) in redefine_method'
         # ./lib/contracts/method_handler.rb:132:in `each'
         # ./lib/contracts/method_handler.rb:132:in `block in redefine_method'
         # ./spec/ruby_version_specific/contracts_spec_2.1.rb:18:in `block (5 levels) in <top (required)>'
         # ./spec/ruby_version_specific/contracts_spec_2.1.rb:17:in `block (4 levels) in <top (required)>'
     # ./spec/ruby_version_specific/contracts_spec_2.1.rb:17:in `block (4 levels) in <top (required)>'

Please take a look at this. contracts.ruby seems to handle mixture of hash and keyword in its own way, and currently I cannot figure out how to fix this issue by myself.

PikachuEXE commented 3 years ago

Thanks for raising this I can see you use rspec spec instead of rake There is a logic on what spec files to be included written at https://github.com/egonSchiele/contracts.ruby/blob/v0.16.0/spec/spec_helper.rb#L25-L27 Using rspec spec would skip it and make it fail I will update CI config to include MRI 3.0

PikachuEXE commented 3 years ago

OK it seems all spec files are supposed to be run... :P

PikachuEXE commented 3 years ago

WIP PR: #295 It passes on 3.0 only at the moment This might take some time...

mtasaka commented 3 years ago

Thank you for working on this!

PikachuEXE commented 3 years ago

Found an article explaining the chaos around this keyword arguments issue https://eregon.me/blog/2019/11/10/the-delegation-challenge-of-ruby27.html

I still have no idea how to fix it across ruby 2 & 3 though Need some help 😬

PikachuEXE commented 3 years ago

Maybe we can have 0.16.x to support Ruby 2.x and future versions for Ruby 3.x But I am not sure yet as there might be more Ruby 2 versions coming (e.g. 2.8)

mtasaka commented 3 years ago

Maybe we can have 0.16.x to support Ruby 2.x and future versions for Ruby 3.x But I am not sure yet as there might be more Ruby 2 versions coming (e.g. 2.8)

I think this is a reasonable idea, and I don't expect that ruby 2.8 will be released in the future.

PikachuEXE commented 3 years ago

@egonSchiele What do you think?

Will prepare to release 0.16.1 anyway There a few changes since 0.16.0, not much but no point keeping them unreleased

Edit: Or wait until we agreed on a plan and announce the plan via post install message in 0.16.1

PikachuEXE commented 3 years ago

@mtasaka @egonSchiele Opened #296 for discussion about the plan raised above Will prepare for 0.16.1 release regardless

PikachuEXE commented 3 years ago

@egonSchiele

Changelog is prepared but I am unable to release a new version (0.16.1) without permission

egonSchiele commented 3 years ago

Thank you for working on this! Let me see if I can give permissions

egonSchiele commented 3 years ago

Ah I don't think I can, but I can release if that works for you @PikachuEXE ?

PikachuEXE commented 3 years ago

Changelog already prepared Only need to update version in gem and change log for new version And see if you want to include #296 in this release

egonSchiele commented 3 years ago

0.16.1 published to rubygems

PikachuEXE commented 3 years ago

295 which makes the gem supports Ruby 3.x only is ready for review