[ruby-core:101639] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[ruby-core:101639] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been reported by shugo (Shugo Maeda).

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:101642] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[matz]

Issue #17429 has been updated by matz (Yukihiro Matsumoto).


I basically agree. Combination of refinement and `include/prepend` only cause confusion.

Matz.


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89433

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:101646] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


+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`.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89436

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:101663] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[marcandre-ruby-core]

Issue #17429 has been updated by marcandre (Marc-Andre Lafortune).


Is there a proposal to import modules in refinements?

Something like this?

```ruby
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.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89452

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:101664] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[daniel]

Issue #17429 has been updated by Dan0042 (Daniel DeLorme).


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:
> ```ruby
> refine Object, import: Code do
>   # extra methods
> end
> ```

That looks pretty good I think.

> I still think that `include` and `prepend` within the `refine` 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."

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89483

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:101943] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[merch-redmine]

Issue #17429 has been updated by jeremyevans0 (Jeremy Evans).


I've added a pull request for this: https://github.com/ruby/ruby/pull/4029

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89790

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102058] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[ronnie]

Issue #17429 has been updated by matsuda (Akira Matsuda).


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?

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89916

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102059] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


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.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89920

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102081] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been updated by shugo (Shugo Maeda).


marcandre (Marc-Andre Lafortune) wrote in #note-3:
> Is there a proposal to import modules in refinements?
> 
> Something like this?
> 
> ```ruby
> 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.


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89940

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102093] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[daniel]

Issue #17429 has been updated by Dan0042 (Daniel DeLorme).


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.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-89950

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102179] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been updated by shugo (Shugo Maeda).


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 use `import` 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?

```ruby
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
```


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-90027

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102181] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


Having a Module subclass for Refinements seems nice and useful :+1:

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-90029

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102198] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[marcandre-ruby-core]

Issue #17429 has been updated by marcandre (Marc-Andre Lafortune).


I like @shugo'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

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-90045

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102247] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been updated by shugo (Shugo Maeda).

Assignee set to matz (Yukihiro Matsumoto)

@eregon @marcandre Thanks for your feedback.

The remaining issue is the name of the new method.
I came up with the following options:

1. Refinement#import
2. Refinement#mix
3. Refinement#include (different behavior from Module#include)

@matz Which do you like, or do you have another option in mind?


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-90095

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102296] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[ko1]

Issue #17429 has been updated by ko1 (Koichi Sasada).


I like `Module#mix` for all modules (not only for refimement).

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-90151

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102298] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


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?

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-90153

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104502] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


One more issue reported due these confusing semantics of include/prepend inside `refine`: #18021.

I suggest we warn in 3.1, raise in 3.2.
And I suggest to add `Refinement#import`, because:
* `mix` seems to imply other things, and if it's ever added to Module with different semantics we will just get more confusion.
* `include` seems likely to cause confusion because the behavior would not be Module#include-like. It is also potentially backward-incompatible, raises the question about what would happen for `prepend` and consistency. And finnaly `include` would not longer mean "define higher in the ancestor" for this context, i.e., defining the same method in the refine block would replace, not just override).

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92772

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104503] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


I'll mention it here as it may be helpful.
If you want to define the same method in multiple refinements before this is fixed, the current workaround is to use `class_eval/module_eval` inside `refine` and have those shared methods in a String.
Not so pretty, but it works and it's simple.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92773

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104651] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been updated by shugo (Shugo Maeda).


Eregon (Benoit Daloze) wrote in #note-20:
> One more issue reported due these confusing semantics of include/prepend inside `refine`: #18021.
> 
> I suggest we warn in 3.1, raise in 3.2.
> And I suggest to add `Refinement#import`, because:
> * `mix` seems to imply other things, and if it's ever added to Module with different semantics we will just get more confusion.
> * `include` seems likely to cause confusion because the behavior would not be Module#include-like. It is also potentially backward-incompatible, raises the question about what would happen for `prepend` and consistency. And finnaly `include` would not longer mean "define higher in the ancestor" for this context, i.e., defining the same method in the refine block would replace, not just override).

I've implemented Refinement#import in https://github.com/shugo/ruby/pull/3

In the current implementation, the module in cref is replaced with the refinement like Module#dup, so constants in the imported module are not accessible from the copied methods.
It would be possible to support constant access with a hack on cref, but I'm afraid it may bring new problems.

Maybe Refinement#import_methods is a better name if we keep the current behavior.


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92960

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104664] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


shugo (Shugo Maeda) wrote in #note-22:
> In the current implementation, the module in cref is replaced with the refinement like Module#dup, so constants in the imported module are not accessible from the copied methods.

Could you show an example that would not work due to that?
Methods from the imported module should be able to access constants from the imported module, otherwise I think it is very surprising.
They should not be able to access constants from the refinement module, that's fine they were declared in the imported module.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92976

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104676] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been updated by shugo (Shugo Maeda).


Eregon (Benoit Daloze) wrote in #note-23:
> shugo (Shugo Maeda) wrote in #note-22:
> > In the current implementation, the module in cref is replaced with the refinement like Module#dup, so constants in the imported module are not accessible from the copied methods.
> 
> Could you show an example that would not work due to that?
> Methods from the imported module should be able to access constants from the imported module, otherwise I think it is very surprising.
> They should not be able to access constants from the refinement module, that's fine they were declared in the imported module.

For me, it's surprising if the imported methods cannot access constants of the refinement.
However, constant assignments in refine block define constants not in the refinement but the outer scope, so it may not be a problem actually.

```
module Ext
  refine Object do
    X = 1 # defines Ext::X
    const_set(:Y, 2) # defines #<refinement:Object@Extension>::X
  end
end
```

Is it enough that the imported methods can access only constants in the original context?


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92992

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104680] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


Yes, I think that's completely fine.

In code, this should work:
```ruby
module Shared
  A = 1
  def foo
    A
  end
end

refine SomeClass do
  import Shared
end

SomeClass.new.foo # => 1
```

And this should not:
```ruby
module Shared
  def foo
    A
  end
end

refine SomeClass do
  self::A = 1
  import Shared
end

SomeClass.new.foo # => NameError
```

That would be the equivalent of dynamic rebinding or so, I think nobody expects that, the constant scope has always been lexical (+ ancestors of the first enclosing module).

Could you add tests (or better, specs under spec/ruby) for that?
Then I think it should be good to go.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92995

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104681] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[eregontp]

Issue #17429 has been updated by Eregon (Benoit Daloze).


In other words, Refinement#import copies methods but does not attempt to change anything lexical, except which refinements are applied to these methods.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-92996

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:104690] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo]

Issue #17429 has been updated by shugo (Shugo Maeda).


I've changed the behavior and have added tests in https://github.com/shugo/ruby/pull/3/commits/3aaca9217f958640495c14ac11b08948960d1f30

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93005

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105189] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo (Shugo Maeda)]

Issue #17429 has been updated by shugo (Shugo Maeda).


In the current implementation:
* Refinement#import raises an ArgumentError if the specified module has methods written in C.
   Should it import C methods without refinements activation?
* Only methods defined directly in the specified module are imported.
  Importing ancestors' methods  may be confusing because Refinement#import doesn't work with super.



----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93592

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105302] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[mame (Yusuke Endoh)]

Issue #17429 has been updated by mame (Yusuke Endoh).


In today's dev meeting, matz accepted the concept, but wanted to take some time to consider the name `import`.

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93712

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105303] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[mame (Yusuke Endoh)]

Issue #17429 has been updated by mame (Yusuke Endoh).


BTW, the change seems to add a top-level new constant `::Refinement`. I'm not against the addition, but unsure about the impact. Is it okay?

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93713

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105318] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo (Shugo Maeda)]

Issue #17429 has been updated by shugo (Shugo Maeda).


mame (Yusuke Endoh) wrote in #note-30:
> BTW, the change seems to add a top-level new constant `::Refinement`. I'm not against the addition, but unsure about the impact. Is it okay?

I found a gem named refinement....

https://github.com/square/refinement/blob/master/lib/refinement.rb


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93733

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105326] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo (Shugo Maeda)]

Issue #17429 has been updated by shugo (Shugo Maeda).


shugo (Shugo Maeda) wrote in #note-31:
> mame (Yusuke Endoh) wrote in #note-30:
> > BTW, the change seems to add a top-level new constant `::Refinement`. I'm not against the addition, but unsure about the impact. Is it okay?
> 
> I found a gem named refinement....
> 
> https://github.com/square/refinement/blob/master/lib/refinement.rb

I've created an issue on the project: <https://github.com/square/refinement/issues/71>.

By gem-codesearch, I've found another gem which defines ::Refinement.

* https://github.com/rkh/refine/blob/master/lib/refine.rb

However, it's a gem for very old Ruby versions without Refinements, so I believe there's no problem.


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93740

* Author: shugo (Shugo Maeda)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105475] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo (Shugo Maeda)]

Issue #17429 has been updated by shugo (Shugo Maeda).

Status changed from Open to Assigned

shugo (Shugo Maeda) wrote in #note-32:
> > I found a gem named refinement....
> > 
> > https://github.com/square/refinement/blob/master/lib/refinement.rb
> 
> I've created an issue on the project: <https://github.com/square/refinement/issues/71>.

The maintainer of the gem agreed with introducing the built-in class Refinement.

Matz, is the method name import is OK?


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-93924

* Author: shugo (Shugo Maeda)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105712] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[matz (Yukihiro Matsumoto)]

Issue #17429 has been updated by matz (Yukihiro Matsumoto).


I agreed with `import_methods`, which is more descriptive and clear.

Matz.


----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-94210

* Author: shugo (Shugo Maeda)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105713] [Ruby master Bug#17429] Prohibit include/prepend in refinement modules

[shugo (Shugo Maeda)]

Issue #17429 has been updated by shugo (Shugo Maeda).

Assignee changed from matz (Yukihiro Matsumoto) to shugo (Shugo Maeda)

----------------------------------------
Bug #17429: Prohibit include/prepend in refinement modules
https://bugs.ruby-lang.org/issues/17429#change-94211

* Author: shugo (Shugo Maeda)
* Status: Assigned
* Priority: Normal
* Assignee: shugo (Shugo Maeda)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/