Bug #17429
Prohibit include/prepend in refinement modules
Description
include/prepend in refinement modules has implementation difficulties such as #17007 and #17379, and tends to be misleading like #17374.
How about to prohibit it in future versions?
Method copy like #17380 may be more convenient, but it's confusing to use names include and prepend because semantics is different from the original ones.
Related issues
Updated by matz (Yukihiro Matsumoto) 2 months ago
I basically agree. Combination of refinement and include/prepend
only cause confusion.
Matz.
Updated by Eregon (Benoit Daloze) 2 months ago
+1 from me!
I think a new Module method to copy all methods to another Module could be useful.
Something like A.copy_methods(B)
.
It seems there is no need to copy constants, because the constant scope of "copied" methods would still be A
(and lexical parents).
Actually the method would only do a shallow copy of each method, i.e., still use the same bytecode, etc, so maybe another name than copy_methods
would be clearer.
The docs of Module#append_features
make it sounds like #append_features
would do that, but it doesn't.
Actually, #append_features adds the given module in the ancestors chain (include = append_features + included).
Would be a good occasion to clarify the docs of #append_features
.
Updated by marcandre (Marc-Andre Lafortune) 2 months ago
Is there a proposal to import modules in refinements?
Something like this?
module Code
# ...
end
refine Object, import: Code do
# extra methods
end
I still think that include
and prepend
within the refine
block could have that function.
I think that having a nice way to implement methods that can be used with include
or using
would help adoption of refinements for gems.
Updated by Dan0042 (Daniel DeLorme) 2 months ago
I agree the current situation needs to be fixed, and prohibiting include/prepend is the simplest way. But I also think there has to be a way to achieve what Marc-Andre was trying in #17374.
marcandre (Marc-Andre Lafortune) wrote in #note-3:
refine Object, import: Code do # extra methods end
That looks pretty good I think.
I still think that
include
andprepend
within therefine
block could have that function.
But there's not much benefit to that is there? Having a different name such as import
feels cleaner. Using include
or prepend
within a refine block could result in a warning/error along the lines of "include
is not supported in refinements but you can use the almost-equivalent import
argument."
Updated by jeremyevans0 (Jeremy Evans) about 2 months ago
I've added a pull request for this: https://github.com/ruby/ruby/pull/4029
Updated by jeremyevans0 (Jeremy Evans) about 2 months ago
- Related to Bug #17007: SystemStackError when using super inside Module included and lexically inside refinement added
Updated by jeremyevans0 (Jeremy Evans) about 2 months ago
- Related to Bug #17374: Refined methods aren't visible from a refinement's module added
Updated by jeremyevans0 (Jeremy Evans) about 2 months ago
- Related to Bug #17379: Refinement with modules redefinition bug added
Updated by matsuda (Akira Matsuda) about 2 months ago
Calling include in refinement modules has certin use cases.
Here's an actual example.
https://github.com/tomykaira/rspec-parameterized/blob/v0.4.2/lib/rspec/parameterized/table_syntax.rb#L27-L61
This gem uses Module#include to avoid code repetition, which to me looks quite natural and basic usage of Module.
If we prohibit include in refinement modules, can this code still be written equally simply?
Updated by Eregon (Benoit Daloze) about 2 months ago
With https://bugs.ruby-lang.org/issues/17429#note-3 it would.
I think it makes sense adding such functionality at the same time as no longer allowing include
for refinement modules.
Updated by shugo (Shugo Maeda) about 2 months ago
marcandre (Marc-Andre Lafortune) wrote in #note-3:
Is there a proposal to import modules in refinements?
Something like this?
module Code # ... end refine Object, import: Code do # extra methods end
I prefer the following way, but I'm not sure about the name import.
refine Object do import Code end
The behavior is similar to Module#mix proposed by Matz before.
Updated by Dan0042 (Daniel DeLorme) about 2 months ago
shugo (Shugo Maeda) wrote in #note-11:
The behavior is similar to Module#mix proposed by Matz before.
Visually it's a more pleasing API than the import
keyword, but would it be available in any module or just refinements?
If any module, we'd now have three mixin mechanisms: include, prepend, import. IMO that's overly complex.
If just refinements, it feels inconsistent. IMO we'll have people asking why they can't use import
in classes and modules.
As a keyword it's clear this is a refinement-only behavior.
Updated by shugo (Shugo Maeda) about 2 months ago
Dan0042 (Daniel DeLorme) wrote in #note-12:
shugo (Shugo Maeda) wrote in #note-11:
The behavior is similar to Module#mix proposed by Matz before.
Visually it's a more pleasing API than the
import
keyword, but would it be available in any module or just refinements?
If any module, we'd now have three mixin mechanisms: include, prepend, import. IMO that's overly complex.
If just refinements, it feels inconsistent. IMO we'll have people asking why they can't useimport
in classes and modules.
As a keyword it's clear this is a refinement-only behavior.
It's enough to changing the class of a module created by refine to the following subclass of Module, isn't it?
class Refinement < Module
[:include, :prepend].each do |name|
define_method(name) do |*args|
warn("#{name} in a refinement is deprecated; use mix instead", uplevel: 1, category: :deprecated)
super(*args)
end
end
def mix(*args)
# ...
end
end
Updated by Eregon (Benoit Daloze) about 2 months ago
Having a Module subclass for Refinements seems nice and useful :+1:
Updated by marcandre (Marc-Andre Lafortune) about 1 month ago
I like shugo (Shugo Maeda)'s approach too.
For anyone interested, I released the refine_export
gem that makes @jeremyevans' nice hack easy to use:
https://github.com/marcandre/refine_export#usage
Updated by shugo (Shugo Maeda) about 1 month ago
- Assignee set to matz (Yukihiro Matsumoto)
Eregon (Benoit Daloze) marcandre (Marc-Andre Lafortune) Thanks for your feedback.
The remaining issue is the name of the new method.
I came up with the following options:
- Refinement#import
- Refinement#mix
- Refinement#include (different behavior from Module#include)
matz (Yukihiro Matsumoto) Which do you like, or do you have another option in mind?
Updated by ko1 (Koichi Sasada) about 1 month ago
I like Module#mix
for all modules (not only for refimement).
Updated by Eregon (Benoit Daloze) about 1 month ago
ko1 (Koichi Sasada) wrote in #note-17:
I like
Module#mix
for all modules (not only for refimement).
I think they need different semantics.
For refinements, we will need to do a deep copy of the method, or at least of the inline caches, so that the copied methods see the other refined methods of the refinement module.
That's quite expensive in footprint, but it probably makes sense for this use case with refinements.
For a general Module#mix, I don't think that is needed. Also what's the advantage of a general Module#mix over include/prepend?