egonSchiele / contracts.ruby

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

Contract error when mixing positional and keyword arguments with defaults #304

Open vlad-pisanov opened 1 month ago

vlad-pisanov commented 1 month ago

I'm having trouble defining a contract for a method that takes two optional arguments -- a positional one and a keyword one. Consider:

class C
  include Contracts

  Contract String, KeywordArgs[b: Optional[String]] => Any
  def foo(a = 'a', b: 'b')
    p [a, b]
  end
end

c = C.new
c.foo('a', b: 'b') # Works ✔️
c.foo('a')         # Works ✔️
c.foo(b: 'b')      # Contract error ❌
c.foo              # Contract error ❌

The stack trace is

Contract violation for argument 1 of 1: (ParamContractError)
        Expected: String,
        Actual: {:b=>"b"}
        Value guarded in: C::foo
        With Contract: String, KeywordArgs => Any
        At: c.rb:9

      fail data[:contracts].failure_exception.new(failure_msg(data), data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        from /Users/main/.gem/ruby/3.3.2/gems/contracts-0.17/lib/contracts.rb:197:in `failure_callback'
        from /Users/main/.gem/ruby/3.3.2/gems/contracts-0.17/lib/contracts/method_handler.rb:144:in `block in redefine_method'

It seems Contracts wants me to provide the positional argument even if a default is specified in the method signature? 🤔

PikachuEXE commented 1 month ago

I don't even know how it handles positional optional arguments I see nothing from https://egonschiele.github.io/contracts.ruby/ Have you used contract on methods with positional optional arguments only?

vlad-pisanov commented 1 month ago

It is indeed not documented, but it works as expected when only positional arguments are used:

class C
  include Contracts

  Contract String, String => Any
  def goo(a = 'a', b = 'b')
    p [a, b]
  end
end

c = C.new
c.goo           # Works ✔️
c.goo('a')      # Works ✔️
c.goo('a', 'b') # Works ✔️

The issue seems to be in KeywordArgs, because if I replace the original example with Any, everything works again:

class C
  include Contracts

  Contract String, Any => Any
  def hoo(a = 'a', b: 'b')
    p [a, b]
  end
end

c = C.new
c.hoo('a', b: 'b') # Works ✔️
c.hoo('a')         # Works ✔️
c.hoo(b: 'b')      # Works ✔️
c.hoo              # Works ✔️
PikachuEXE commented 2 weeks ago

I think it's a deeper issue of lacking a contract for optional positional argument From my limited understand it works like this

  1. Declare contracts, e.g. Contract contract_at_pos_1, contract_at_pos_2, contract_keyword_args
  2. When called, check against contracts according to position: arg1 => contract_at_pos_1, arg2 => contract_at_pos_2...

It's fine for methods with optional arguments only coz it works like: Contracts declared: contract_at_pos_1, contract_at_pos_2 When some arguments not passed, it checks like: arg1 => contract_at_pos_1, no arg2 = no check

But if optional positional argument is mixed with keyword arguments Contracts declared: contract_at_pos_1, contract_keyword_args It checks like: keyword_args => contract_at_pos_1

I am guessing from https://github.com/egonSchiele/contracts.ruby/blob/v0.17/lib/contracts/call_with.rb#L51-L81

PikachuEXE commented 2 weeks ago

Please check https://github.com/egonSchiele/contracts.ruby/pull/305 and see if you want to add more examples to ensure I don't break anything Use it in your app for a while to ensure nothing breaks

PikachuEXE commented 2 days ago

I won't merge https://github.com/egonSchiele/contracts.ruby/pull/305 until some has tested it~

vlad-pisanov commented 2 days ago

Thanks for the patch! I'll test it later this week.