contracts.ruby icon indicating copy to clipboard operation
contracts.ruby copied to clipboard

module_function ignores contracts

Open kliuless opened this issue 10 years ago • 5 comments
trafficstars

Tested on 0.7. It's possible that module_function is too magical to try to work around. In that case the docs should say not to use it. (It's probably not feasible to redefine module_function to raise an error, because it might be called before including contracts.)

module OverloadCase
  include Contracts
  include Contracts::Modules
  #extend self     # this works   
  module_function  # this doesn't 

  Contract 1 => 1
  def fact(n) 1 end

  Contract Integer => Integer
  def fact(n)
    n * fact(n-1)
  end
end

module MinCase
  include Contracts
  include Contracts::Modules
  #extend self     # this works
  module_function  # this doesn't

  Contract 1 => 1
  def fact(n) 1 end
end

if __FILE__ == $0
  p OverloadCase.fact(6) # => stack too deep
  p MinCase.fact(6)      # => violation is ignored, so 1 is returned
end

kliuless avatar Feb 23 '15 19:02 kliuless

Interesting observation:

module X
  module_function

  def hello
    :world
  end
end

class Y
  include X
end

class Z
  extend X
end

# expected
X.hello   #=> :world

# expected
Y.hello   #=> NoMethodError: undefined method `hello' for Y:Class

# unexpected
Y.new.hello   #=> NoMethodError: private method `hello' called for #<Y:0x000001022b1940>

# expected
Z.hello  #=> NoMethodError: private method `hello' called for Z:Class

# expected
Z.new.hello   #=> NoMethodError: undefined method `hello' for #<Z:0x000001021eea30>

alex-fedorov avatar Feb 23 '15 19:02 alex-fedorov

It is definitely different from extend self in its behaviour:

module X
  extend self

  def hello
    :world
  end
end

class Y
  include X
end

class Z
  extend X
end

X.hello     #=> :world
Y.hello     #=> undefined method
Y.new.hello   #=> :world      # this is different, with module_function it is private
Z.hello     #=> :world            # this is different, with module_function it is private
Z.new.hello    #=> undefined method

alex-fedorov avatar Feb 23 '15 19:02 alex-fedorov

Documentation says:

Module functions are copies of the original, and so may be changed independently.

alex-fedorov avatar Feb 23 '15 19:02 alex-fedorov

I have played around a bit with it and the only solution I see:

module M
  include Contracts
  include Contracts::Modules
  include Contracts::ModuleFunction
  module_function

  # ...
end

Because the way these methods get defined is following:

singleton_method_added singleton_method_added
method_added fact
method_added __contracts_ruby_original_fact_i6i9rqcm8jze
method_added fact
singleton_method_added fact

First definition of fact consumes all applied decorators (read: Contract ... calls). So definition of self.fact made by module_function goes unnoticed.

What we want is to duplicate decorator(s) in this case, but we need to do it only if module_function is applied, but I don't see any way to detect it. Even redefining module_function doesn't work, because it is not a method at all, looks like just syntax construction. Because this doesn't work:

module M
  def self.module_function
    puts "gotcha"
    super
  end

  module_function

  def hello
    :world
  end
end
#=> "gotcha"

M.hello    #=> undefined method 'hello'

And I even afraid to think about the behaviour in jruby/rbx.

But if we force usage by include Contracts::ModuleFunction we will avoid the problem entirely. On the other hand amount of these include-s can easily go only up and up with these temp.

Other idea is somehow mark contract to extend over the next definition:

  Contract::ModuleFunction 1 => 1
  def fact(1) 1 end

But I don't see how it is better than explicit include.

alex-fedorov avatar Feb 23 '15 19:02 alex-fedorov

I think the explicit include is acceptable. Remember also to test when module_function is used with method names.

Sidenote: I did some testing on module_function and it's actually an instance method of Module. It only seems to be magical when called without args. In that case, it acts on the lexical scope.

module M1
end

$M1_mod_func = lambda {|*args| M1.send(:module_function, *args) }
$M1_mod_func_eval = lambda {|binding_| eval 'module_function', binding_ }

module M1
  $M1_mod_func.call  # no names
  # even this doesn't work on :hello, probably due to lexical scoping
  #$M1_mod_func_eval.call(binding)

  def hello
    p 'M1 hello'
  end
end
module M1
  def goodbye
    p 'M1 goodbye'
  end
  $M1_mod_func.call :goodbye  # explicit names
end

module M2
  # ??? if you change this to M1.send, the no-arg call still acts on M2 !
  M2_mod_func = lambda {|*args| M2.send(:module_function, *args) }
  M2_mod_func.call  # no names

  def hello
    p 'M2 hello'
  end
end


M1.hello   rescue p "M1 hello fail: #{$!}"   # fail
M1.goodbye rescue p "M1 goodbye fail: #{$!}" # succeed
M2.hello   rescue p "M2 hello fail: #{$!}"   # succeed

kliuless avatar Feb 23 '15 23:02 kliuless