egonSchiele / contracts.ruby

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

keep properties of given blocks #278

Open md-work opened 6 years ago

md-work commented 6 years ago

Since #242 (Func contract's return value isn't enforced with blocks) was fixed in contracts-0.15, given blocks are being put into a lambda. So the forwarded block isn't 100% identical with the original one. Especially it's arity and it's source_location changed.

Please keep as many properties as possible of the original block, especially the arity.

Or find a way to forward the original block without breaking #242 again. So the original block could be kept. I'd really appreciate any ideas for this, because it might be the cleanest solution!

 


 

Commit for #242, introducing this bug: https://github.com/egonSchiele/contracts.ruby/commit/13e56bdf859a54c569d1a7fe4680251880c6d089#diff-a45dd775c2769b9ab16a23f9d4560adc lib/contracts/call_with.rb

@@ -73,7 +75,8 @@ module Contracts
                  method.call(*args, &blk)
                else
                  # original method name referrence
-                 method.send_to(this, *args, &blk)
+                 added_block = blk ? lambda { |*params| blk.call(*params) } : nil
+                 method.send_to(this, *args, &added_block)
                end

       unless @ret_validator[result]

 


 

I especially need the arity to stay unchanged. Some examples about arity:

def a(&b)
  puts b.arity
end

a {|x| puts x}
=> 1

a {|x,y| puts x+y}
=> 2

a {puts ''}
=> 0

a {|*x| puts x}                                                                                                   
=> -1                                                                                                                                   

 

A very "simple" way how to keep the arity is using eval (see below). But I hope we find a better way.

--- a/lib/contracts/call_with.rb
+++ b/lib/contracts/call_with.rb
@@ -81,7 +81,15 @@ module Contracts
                  method.call(*args, &blk)
                else
                  # original method name reference
-                 added_block = blk ? lambda { |*params| blk.call(*params) } : nil
+                 added_block = nil
+                 if blk
+                    params_ary = []
+                    for i in 1..blk.arity
+                      params_ary << 'x_'+i.to_s
+                    end
+                    params_str = params_ary.join(',')
+                    added_block = eval('lambda { |'+params_str+'| blk.call('+params_str+') }')
+                 end
                  method.send_to(this, *args, &added_block)
                end
md-work commented 6 years ago

Looks like every Block being passed is wrapped into a lambda. Even if there's no "subcontract" checking the blocks parameters.

Contract Func[Num=>Num] => nil
def b(&blk)
  # blk is now a lambda, which is necessary to perform the Func check.
  blk.call(2)
  return nil
end
b {|*x| puts x}

Contract Proc => nil
def c(&blk)
  # blk is now also a lambda, even though it's not necessary because there's no Func check to perform.
  blk.call(2)
  return nil
end
c {|*x| puts x}

 


 

Given that, the problem can be solved for all cases in which there's no Func check in the Contract.

--- a/lib/contracts/call_with.rb
+++ b/lib/contracts/call_with.rb
@@ -81,7 +81,8 @@ module Contracts
                  method.call(*args, &blk)
                else
                  # original method name reference
-                 added_block = blk ? lambda { |*params| blk.call(*params) } : nil
+                 added_block = blk ? blk : nil
+                 added_block = lambda { |*params| blk.call(*params) } if blk && blk.is_a?(Contract)
                  method.send_to(this, *args, &added_block)
                end

By this, the block will only be wrapped into a lambda if there's a Func check in the Contract. So if you need the block to stay identical (like I do), you can't use that Func feature. But with this patch it's still possible to use Contracts in general, as long as you don't use Func checks.

 


 

P.S. Maybe we can fix this with even less effort. (this patch is -2 +2 = 0 lines and the other patch is -1 +2 = +1 lines)

But I'm not sure if it's ok to assign the new lambda to the existing blk variable. Guess it's fine, but someone else may double check!

--- a/lib/contracts/call_with.rb
+++ b/lib/contracts/call_with.rb
@@ -81,8 +81,8 @@ module Contracts
                  method.call(*args, &blk)
                else
                  # original method name reference
-                 added_block = blk ? lambda { |*params| blk.call(*params) } : nil
-                 method.send_to(this, *args, &added_block)
+                 blk = lambda { |*params| blk.call(*params) } if blk && blk.is_a?(Contract)
+                 method.send_to(this, *args, &blk)
                end

       unless @ret_validator[result]