Bug #12514: Refinements: when including Module as refinement, can't call other module methods

• Status: Rejected
• Assignee: -
• Target version: -
• Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN

[chucke (Tiago Cardoso)]

Very simple script which reproduces the problem:

module Extensions
  def vegetables ; potatoe ; end
  def potatoe ; "potatoe" ; end
end

module Refinary
  refine String do
    # this doesn't work
    include Extensions
    # this would work...
    # def vegetables ; potatoe ; end
    # def potatoe ; "potatoe" ; end
  end
end

using Refinary

puts "tomatoe".vegetables

#=> in `': undefined method `vegetables' for "tomatoe":String 

failing from ruby 2.0 to 2.3

[shugo (Shugo Maeda)]

• Status changed from Open to Rejected

#=> in ': undefined methodvegetables' for "tomatoe":String

On my box, the following exception is raised instead:

t.rb:2:in `vegetables': undefined local variable or method `potatoe' for "tomatoe":String (NameError)
	from t.rb:18:in `<main>'

And this is an expected behavior because String is not refined in the definition of Extensions.

[chucke (Tiago Cardoso)]

potatoe is not a variable, but a method call. The method is defined right after. That should be the whole point of it, right?

[chucke (Tiago Cardoso)]

the error message in the example is wrong, my bad, it is "potatoe" the undefined method, not "vegetables".

[shugo (Shugo Maeda)]

Tiago Cardoso wrote:

potatoe is not a variable, but a method call. The method is defined right after. That should be the whole point of it, right?

The potatoe is defined in Extension, but String is only refined in the scope where using is called or the block of refine.
The definition of Extensions#vegetables is out of either scope, and thus the method call of potatoe fails.

[chucke (Tiago Cardoso)]

Don't get me wrong, but you're trying to explain me why it doesn't work. I'm making the point it should work. Why?

If I reopen the string class and include the extensions module, it just works. This is the expected meta-behaviour, as both methods are added to the scope, and lookup is resolved in runtime.

This doesn't work with Refinements, and I categorize it as a bug. Infact, I effectively can't use it. Imagine I write a library with a module which injects methods in a core class. In a pre-refinement world, I'll just include the module in runtime and live and dy by the monkey-patch everywhere this core class is used. If I could limit the inclusion with refinements, I'd safely compartimentalize the behaviour injection. I could also provide support for ruby <2 this way. The way this is currently implemented, it's a bit unusable.

[shugo (Shugo Maeda)]

Tiago Cardoso wrote:

Don't get me wrong, but you're trying to explain me why it doesn't work. I'm making the point it should work. Why?

If I reopen the string class and include the extensions module, it just works. This is the expected meta-behaviour, as both methods are added to the scope, and lookup is resolved in runtime.

This doesn't work with Refinements, and I categorize it as a bug. Infact, I effectively can't use it. Imagine I write a library with a module which injects methods in a core class. In a pre-refinement world, I'll just include the module in runtime and live and dy by the monkey-patch everywhere this core class is used. If I could limit the inclusion with refinements, I'd safely compartimentalize the behaviour injection. I could also provide support for ruby <2 this way. The way this is currently implemented, it's a bit unusable.

If you don't like the current behavior, please file another ticket as a feature request.

A feature called local rebinding, which was rejected by Matz in the past, would solve your problem.
I'm not sure whether it can be solved without local rebinding.

[chucke (Tiago Cardoso)]

Thx, will do. Do you have a link to the reject ticket? Maybe it would be wise to read about the reasons against before filling a similar ticket.

[shugo (Shugo Maeda)]

Tiago Cardoso wrote:

Thx, will do. Do you have a link to the reject ticket? Maybe it would be wise to read about the reasons against before filling a similar ticket.

local rebinding was discussed in #4085.

Today, Matz stated that he wouldn't like to introduce local rebinding because it's unpredictable (https://twitter.com/yukihiro_matz/status/748000755353784321).
Another reason was performance issues.