Allow specifying (non-permanent) module and class names.

[ioquatix]

Enables:

• Class.new(superclass, name)
• Module.new(name)

such that #name returns the given name UNTIL it's assigned to a constant.

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

[jeremyevans]

I don't think it's a good idea to modify initialize. If we support this at all, I think it should be via a separate method (e.g. Module#set_temporary_name).

[ioquatix]

I don't think it's a good idea to modify initialize

Why not?

I don't think this is something that should be a new method, because it is not something that should be invoked more than once, and should be set before any internal callbacks are invoked for the class/module.

[jeremyevans]

I don't think it's a good idea to modify initialize

Why not?

Because it is almost never needed. It also requires you modify two methods instead of a single method.

I don't think this is something that should be a new method, because it is not something that should be invoked more than once, and should be set before any internal callbacks are invoked for the class/module.

What callbacks are you referring to? For Module.new, there don't appear to be any, all it does is yield to the provided block, and you could call the new method at the top of the block. For Class#new, the only internal callback appears to be to inherited.

In the places where you plan to use this feature, is there a reason it couldn't be a separate method? If so, can you provide an example?

[ioquatix]

The class name is cached, if you define child classes in a callback, e.g. #inherited, they will generate the wrong name. Assuming that you have a method to set it AFTER the class is constructed, you will get the results shown below. I didn't implement a method, but instead I moved the logic to set the class path AFTER the callbacks from initialize are called, which is the order of operations you are proposing:

class Base
  def self.inherited(derived)
    nested = derived.const_set(:Nested, Class.new)
    nested.name # force full class name to generated
  end
end

class Child1 < Base
end
pp Child1 # => Child1
pp Child1::Nested # Child1::Nested

child2 = Class.new(Base, "Child2")
pp child2 # => Child2
pp child2::Nested # => #<Class:0x00007f1dd9e23850>::Nested

I'm against introducing a method for this feature as it's not something that should be set after the class is created and the semantics of setting it afterwards are confusing at best.

Just to be super clear, here is the expected behaviour (and the one given by this PR):

child2 = Class.new(Base, "Child2")
pp child2 # => Child2
pp child2::Nested # => Child2::Nested

[jeremyevans]

Assuming you wanted this working in Class#inherited (and the code example you provided for that seems unlikely to happen in non-contrived situations), you could still use a separate method that created the module/class, such as Module#new_with_temp_name/Class#new_with_temp_name. IMO, it's a bad idea to change the method signature for Module#new/Class#new to implement a feature that will be so rarely used.

[ioquatix]

There are over 100 usage of labeled_class + labeled_module in Ruby's own test suite alone. I don't see why it's a bad idea to introduce this optional feature. If you think it's a bad idea, please explain why - e.g. compatibility issues, etc. I personally think the method names you proposed are clunky when an optional argument works just fine. Most people creating anonymous modules and classes Module.new and Class.new are not assigning them to constants (otherwise you can just use the module/class X form), but it is incredibly useful from the POV of error handling to have useful names.

The current implementations in EnvUtil are actually not completely correct and leave edge cases where the name is not correctly used. So if you are willing to stretch a little, you might be able to call this PR a bug fix.

I'm tired of seeing things like:

irb(main):001:0> c = Class.new
=> #<Class:0x00007f0a7d5ee118>
irb(main):002:0> c.new
=> #<#<Class:0x00007f0a7d5ee118>:0x00007f0a7d623c50>

This proposal makes it much nicer:

irb(main):001:0> c = Class.new(Object, "C")
=> C
irb(main):002:0> c.new
=> #<C:0x00007ff6b38f23f0>

Finally, it's unfair to say "so rarely used" since up until the point where this interface is introduced, it's impossible to do such a thing correctly anyway. It would only be apparent some years after introduction whether it's "so rarely used" in practice. And evidence given by labeled_class an labeled_module suggest that the feature would be used in a non-trivial number of cases, especially when dynamically building classes and modules (which itself is not uncommon). I have at least several places where I would find this functionality extremely useful.

As one more data point, searching for ruby set anonymous class name reveals that this is not an uncommon request and there are various strategies, none of which is completely correct and/or flexible (i.e. using an arbitrary string as a class name).

[eregon]

I think this needs discussion in a dev meeting first.
And also what does it solve that is not possible otherwise?

[eregon]

This proposal makes it much nicer:

irb(main):001:0> c = Class.new(Object, "C")
=> C
irb(main):002:0> c.new
=> #<C:0x00007ff6b38f23f0>

The problem is obvious, isn't it?
With that output, anyone would expect there is a constant C pointing to that class.
But there isn't, or if there is it might point to another class.
That's terrible, it causes extreme confusion and waste of time, because Module#name is a sort of reflection API and it should reflect the real state.
(yes this can also happens with remove_const but that's very rarely used and overall Module#name being how to reach the module holds in almost all cases).

[eregon]

Ah and BTW ruby/spec doesn't use any labeled_class.
It either uses anonymous modules or it names them properly in fixtures files. So IMO there is no need for any such hacks in tests.

[nobu]

Since envutil.rb is used by many default gems which may support earlier versions, it should not use the syntax only available on the head.

[ioquatix]

We have decided not to go ahead with this approach at this time.