Bug #17374: Refined methods aren't visible from a refinement's module

• Status: Rejected
• Assignee: shugo (Shugo Maeda)
• Target version: -
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

[marcandre (Marc-Andre Lafortune)]

[marcandre (Marc-Andre Lafortune)]

• Subject changed from Refined methods aren't visible from a refinementRefinements that include/prepend module to Refined methods aren't visible from a refinement's module

Is this intentional?

class Foo
  module Extension
    module Implementation
      def foo
        super(bar) # << Can not see bar
      end

      def bar # << Bar is in the same module!
        42
      end
    end

    refine Foo do
      prepend Implementation
    end
  end

  def foo(value = :none)
    p value
  end
end

Foo.new
Foo.new.foo # => :none (ok)

# Does not work: undefined local variable or method `bar'
using Foo::Extension
Foo.new.foo rescue :error # => :error

# Works:
Foo.prepend Foo::Extension::Implementation
Foo.new.foo # => 42

[Eregon (Benoit Daloze)]

I think so. Refined methods only see each other if they are directly defined in the refine.

Using prepend/include is probably not recommended anyway, it leads to many complications in the semantics.

[marcandre (Marc-Andre Lafortune)]

Eregon (Benoit Daloze) wrote in #note-2:

I think so. Refined methods only see each other if they are directly defined in the refine.

They can be defined in different refine blocks and it still works:

class Foo
  module Extension
    refine Foo do
      def foo
        super(bar)
      end
    end

    refine Foo do
      def bar
        42
      end
    end
  end

  def foo(value = :none)
    p value
  end
end

using Foo::Extension
Foo.new.foo # => 42

Using prepend/include is probably not recommended anyway, it leads to many complications in the semantics.

It's not clear to me why they are allowed if they are supported this way.

I know of no other case in Ruby where two methods in a module can not see each other.

I know of no other case in Ruby where you can not extract some code in a new method (assuming no naming conflict)

Let's say a class does not define any methods; I know of no other case in Ruby where include Mod or copy-pasting Mod's methods is different.

In my mind, include/prepend inserts a layer inside a Module and brings in those methods in it. I can not explain mentally what refine { include } does currently.

Here's a simplified example, where a method does not see itself:

module Extension
  module Implementation
    def foo(value = nil)
      return value.to_s if value

      foo(42) # => NoMethodError!?
    end
  end

  refine Object do
    include Implementation
  end
end

using Extension
:x.foo(:y) # => 'y' (ok)
:x.foo # => NoMethodError

Finally, my use case is that I think it would be worthwhile to provide a way to write libraries that can be used either as a refinement, or as a apply everywhere:

# Either
using MySuperGem # => everywhere you use the refinement

# Or
SomeClass.prepend MySuperGem # => once

This is much harder to write with the current semantics.

[Eregon (Benoit Daloze)]

• Assignee set to shugo (Shugo Maeda)

marcandre (Marc-Andre Lafortune) wrote in #note-3:

They can be defined in different refine blocks and it still works:

Yes, because they are refine under the same "refinement namespace" module.
And each refine is basically using all other refine in the same "refinement namespace" module.

The set of refinements that apply are captured at def time.

In the original example, there is no way to know that Implementation will be followed by refine, and that the methods declared there should be affected.

What you are asking is I think dynamic rebinding, because what should happen if:

module R1
  refine Foo do
    prepend Foo::Implementation
  end
end

module R2
  refine Foo do
    prepend Foo::Implementation
  end
end

A given (lexical) call site should have a fixed set of used refinements (important for reasoning and performance).
That wouldn't hold for Foo::Extension::Implementation#foo, it could be either of R1 or R2 for the same call site.

[marcandre (Marc-Andre Lafortune)]

Eregon (Benoit Daloze) wrote in #note-4:

In the original example, there is no way to know that Implementation will be followed by refine, and that the methods declared there should be affected.

What you are asking is I think dynamic rebinding

I might be confused as to what dynamic rebinding is; for me the constraint was that there could not be later redefinitions of refinements, that everything had to had been already defined (which is the case).

When refine { include Mod }, I would like the include to "duplicate" the current methods of Mod, just as if I had written them with def at that point. Ideally they would be in a different layer than the current refinement, so you could use prepend, def and include on the same method, but at this point that is minor.

Note that I tried refine { define_method :foo, Mod.instance_method(:foo) } and that does not work either.

because what should happen if:

module R1
  refine Foo do
    prepend Foo::Implementation
  end
end

module R2
  refine Foo do
    prepend Foo::Implementation
  end
end

More or less the same as if you replaced the prepend with the defs of Foo::Implementation as they were defined at the time.

[jeremyevans0 (Jeremy Evans)]

I do not think this is a bug. The question to ask yourself is, at the point of call, is the refinement active?. So in method Implementation#foo, we are talking about the call to bar:

      def foo
        super(bar) # << Can not see bar
      end

No, the refinement is not active at that point. You can make it active, though doing so is probably not obvious:

class Foo
  module Extension
    module Implementation
    end

    refine Foo do
      prepend Implementation
    end

    module Implementation
      def foo
        super(bar) # << Can not see bar
      end

      def bar # << Bar is in the same module!
        42
      end
    end
  end

  def foo(value = :none)
    p value
  end
end

Foo.new
Foo.new.foo # => :none (ok)

# Does not work: undefined local variable or method `bar'
using Foo::Extension
Foo.new.foo rescue (p :error) # => :error

# Works:
Foo.send(:prepend, Foo::Extension::Implementation)
Foo.new.foo # => 42

This is because refine implicitly activates the refinement, so the refinement is active at the point bar is called. This approach works back to Ruby 2.0, with output:

:none
:none
42

[marcandre (Marc-Andre Lafortune)]

jeremyevans0 (Jeremy Evans) wrote in #note-6:

This is because refine implicitly activates the refinement, so the refinement is active at the point bar is called. This approach works back to Ruby 2.0, with output:

:none
:none
42

Thanks for trying it out.
This is not the desired output, though. The last using has no effect. Desired output is:

:none # No refinement active yet
42 # using was activated
42 # extend works too (assuming the `using` part is commented out...

My understanding is that the refine simply has no effect, since the module is empty at that point, no?

[jeremyevans0 (Jeremy Evans)]

marcandre (Marc-Andre Lafortune) wrote in #note-7:

jeremyevans0 (Jeremy Evans) wrote in #note-6:

This is because refine implicitly activates the refinement, so the refinement is active at the point bar is called. This approach works back to Ruby 2.0, with output:

:none
:none
42

Thanks for trying it out.
This is not the desired output, though. The last using has no effect. Desired output is:

:none # No refinement active yet
42 # using was activated
42 # extend works too (assuming the `using` part is commented out...

Ah, thank you for clarifying. I still think the behavior is expected, even if my fix was bad, because the refinement is not active at the point of call. However, only @matz (Yukihiro Matsumoto) can decide whether this is a bug or expected behavior.

After playing around with it, I found a way to get it to work without duplicating the literal method definitions. It's not much better than calling class_eval with the same string in two places, though:

class Foo
  module Extension
    RefinedImplementation = refine Foo do
      def foo(value=:none)
        super(bar) # << Can not see bar
      end

      def bar # << Bar is in the same module!
        42
      end
    end
    
    module Implementation
      RefinedImplementation.public_instance_methods(false).each do |m|
        define_method(m, RefinedImplementation.instance_method(m))
      end
    end
  end

  def foo(value = :none)
    p value
  end
end

I'd probably use class_eval with the same string instead of this.

[shugo (Shugo Maeda)]

It's intended behavior.

Refinements are activated either in scope where using is called or in blocks given to refine.
I don't recommend module inclusion to define refined methods.

[marcandre (Marc-Andre Lafortune)]

jeremyevans0 (Jeremy Evans) wrote in #note-8:

After playing around with it, I found a way to get it to work without duplicating the literal method definitions.

Ah! I had tried the other way (doesn't work), but from refinement module to the module it works, thanks for figuring this out 👍

It's not much better than calling class_eval with the same string in two places

I think it is. A string isn't parsable (e.g. RuboCop, AST-based search tools, ...), nor syntax-highlightable, nor as debuggable, won't be detected by help generation tools, etc...

This way I can write what I wanted:

class Foo
  module Extension
    import refine(Foo) {
      def foo
        super(bar) # << Can not see bar
      end

      def bar # << Bar is in the same module!
        42
      end
    }
  end

  def foo(value = :none)
    p value
  end
end

# Does not work: undefined local variable or method `bar'
if ENV['USING']
  using Foo::Extension
else
  Foo.prepend Foo::Extension
end
Foo.new.foo rescue (p :error) # => 42, works both ways

With this refinement active...

refine Module do
  def import(refined_mod)
    refined_mod.instance_methods(false).each do |m|
      define_method(m, refined_mod.instance_method(m))
    end
  end
end

[marcandre (Marc-Andre Lafortune)]

shugo (Shugo Maeda) wrote in #note-9:

I don't recommend module inclusion to define refined methods.

Indeed!

The question remains: if it's a bad idea to include/prepend modules inside a refine block, could we not make it a good idea? Would it be technically difficult to duplicate the methods in the included module, import them and active them in the refinement?

[jeremyevans0 (Jeremy Evans)]

marcandre (Marc-Andre Lafortune) wrote in #note-10:

jeremyevans0 (Jeremy Evans) wrote in #note-8:

It's not much better than calling class_eval with the same string in two places

I think it is. A string isn't parsable (e.g. RuboCop, AST-based search tools, ...), nor syntax-highlightable, nor as debuggable, won't be detected by help generation tools, etc...

As commonly used, you are correct. However, one possibility is:

  path = File.expand_path('implementation.rb', __dir__)
  code = File.read(path)
  
  refine(Foo){class_eval(code, path)}
  Implementation = Module.new{class_eval(code, path)}

This should be parsable and syntax-highlightable at least. I'm less sure about debuggability and interaction with help generation tools, though. Loss of code locality is definitely a downside.

[Eregon (Benoit Daloze)]

• Status changed from Open to Rejected

marcandre (Marc-Andre Lafortune) wrote in #note-11:

The question remains: if it's a bad idea to include/prepend modules inside a refine block, could we not make it a good idea? Would it be technically difficult to duplicate the methods in the included module, import them and active them in the refinement?

That actually means duplicating the inline caches and so probably also the bytecode, etc.
So it doesn't seem a good idea at least from a memory footprint point of view.

Given shugo is AFAIK responsible for the refinements specification and he confirmed it's intended behavior, I'll close this issue (but feel free to continue the discussion).

[shugo (Shugo Maeda)]

marcandre (Marc-Andre Lafortune) wrote in #note-11:

shugo (Shugo Maeda) wrote in #note-9:

I don't recommend module inclusion to define refined methods.

Indeed!

The question remains: if it's a bad idea to include/prepend modules inside a refine block, could we not make it a good idea? Would it be technically difficult to duplicate the methods in the included module, import them and active them in the refinement?

In addition to implementation issues described by eregon, it may be confusing because it changes the semantics of include/prepend to code duplication like Module#mix proposed by Matz before.
Anyway, your proposal should be filed as a feature ticket.

[Eregon (Benoit Daloze)]

• Related to Feature #17380: Useful `include/prepend` in `refine` added

[Dan0042 (Daniel DeLorme)]

I tried testing this and immediately ran into this issue: (in latest master)

class Foo
  module Extension
    module Implementation
      def foo
        super(bar)
      end

      def bar
        42
      end
    end

    refine Foo do
      prepend Implementation
    end
  end

  def foo(value = :none)
    p value
  end
end

Foo.new.foo # => :none (ok)

Foo.prepend Foo::Extension::Implementation
Foo.new.foo # => :none ... not 42 as expected

So it appears like Foo.prepend Foo::Extension::Implementation has no effect. But if I just comment out the prepend Implementation line it works as expected. Note that there's no using anywhere anyway. I'm at a loss to explain what's going on here. But I think more than ever that module inclusion to define refined methods is not just "not recommended", it should be prevented by raising an error. Or at least output a warning.

[marcandre (Marc-Andre Lafortune)]

Yup, that's a bug, good catch.
I believe is the same as #17379 because if you comment the first Foo.new.foo then it works as expected. I added a simplified version to that issue

[jeremyevans0 (Jeremy Evans)]

• Related to Bug #17429: Prohibit include/prepend in refinement modules added