Method definitions must not be nested catches singleton methods in minitest

[nateberkopec]

e.g.:

def setup
  @client = Client.new
  def @client.gateway
    @gateway ||= TestGateway.new
  end
end

I could use define_method and a proc, but that just takes longer to type.

[searls]

lmao I didn't even know this was valid. Neat.

[nateberkopec]

I do this all the time. It's part of my stubbing toolkit in minitest. This + minitest/mock and you're set.

[nateberkopec]

I imagine this wouldn't be caught if I used the it "does a thing" rails-style tests, right?

[saturnflyer]

This is a neat trick for tests but is probably unwise to depend on it. A friend asked Matz directly about this behavior and he was told to not depend on it. But for a more concrete quote: https://bugs.ruby-lang.org/issues/11665

But at least, the current behavior of nested method definition is useless. It should be made obsolete to open up the future possibility (I'd vote for warning).

[nateberkopec]

What I'm doing is not really a nested function as discussed in that thread, it's just defining a singleton method inside of another method.

[nateberkopec]

One resolution here would be to ignore any def whatever inside of a method if whatever contains .

[saturnflyer]

@nateberkopec that's a good point. def @client is different than def although that's still a def inside a def so that seems to land within Matz's comment about not allowing that

[BrianHawley]

This restriction on defining singleton methods with def inside of other methods is going to cause problems with it comes to BasicObject and Ractor compatibility. Also, the restriction in question is just going to make code worse, because the need to define singleton methods in other methods is still there, and the other alternatives that people would be forced to do to get around this restriction are uglier and harder to understand.

For reference, the alternatives to def object. for defining singleton methods are object.define_singleton_method, object.singleton_class.class_exec { def ... }, object.singleton_class.class_eval "def ...", and more. Instance methods have similar alternatives, like define_method.

Starting with the Ractor issue: If you use define_method or define_singleton_method, this can cause a closure, where variables referenced in outer methods are accessible in the methods you're defining. If you try to run your new methods in another Ractor, the closed variables won't be sharable, so referencing them in the methods would fail. Methods defined with def can't access outer variables, except for instance variables and methods defined in the object that the method is defined on, so they don't have as many sharability issues.

Because of this, when the code in the built-in gems have been updated to be Ractor compatible, they've generally switched from using define_method to using class_eval and source code in interpolated strings with def to generate the new methods. This method of code generation ensures that no data is shared between Ractors, and the resulting code is very friendly to the JIT when it gets around to evaluating the resulting methods.

Instance methods of the type that you would define with define_method can't be defined with def in other instance methods easily because of https://bugs.ruby-lang.org/issues/11665 - which I'm not disagreeing with - but you could work around this with code like this:

def foo
  ...
  self.class.class_exec do
    def bar
      ...
    end
  end
end

It's worth pointing out that self.class part though. Using an instance method to define things at a class level is a good sign that you really should be using a class method here. This is why https://bugs.ruby-lang.org/issues/11665 makes sense: It's not because of the syntax or even the nested def; it's a matter of the instance method stepping out of its bounds of appropriate behavior, intruding on the territory of class methods. Also, self.class won't work on objects which aren't derived from Object, because BasicObject doesn't define a class method, so the entire idea of defining class methods inside of instance methods is going to be restricted for those, which really emphasizes how much of a bad idea it is.

For singleton methods, this is a different issue though. In that case, you are defining a method on a specific object that you have a reference to in your method. This is why defining a singleton method on an object is a really common way to define a localized extension of behavior, especially if you don't want it to be defined on all objects of that type. Not just in Minitest: Also in many other gems, including in Rails, and a lot of private code.

The most common way that I've seen singleton methods defined is with def object., even inside other methods, but some people use define_singleton_method. If you define a singleton method with define_singleton_method, you are going to run into the same Ractor sharing issues, plus BasicObject restrictions because it doesn't have that method either, so it's really best to use def object. style when you don't have to do custom generation of the methods.

For example, here are the alternatives for adding the same method every time:

# Defining a non-customized singleton method using def style directly:
def foo(object)
  ...
  def object.bar
    ... # This code is Ractor-safe if object is made sharable or transferred.
  end
  object
end

# Defining a non-customized singleton method without nesting def, strictly because of the standard restriction:
def foo(object)
  ...
  object.singleton_class.class_eval do
    def bar
      ... # This code is Ractor-safe if object is made sharable or transferred.
    end
  end
  object
end

# But wait, BasicObject doesn't define singleton_class either.
# So let's try again to meet the standard restriction.
def foo(object)
  ...
  # BasicObject has instance_eval or instance_exec, which are the same for blocks.
  object.instance_exec do |obj|
    # Still don't have access to singleton_class, so we have to use def obj. here and hope standard allows it.
    def obj.bar
      ... # This code is Ractor-safe if object is made sharable or transferred.
    end
  end
  object
end

# If standard complains about the def obj. in the nested block, there's no other way to define a
# singleton method that works with BasicObject, so we have to use code in a string so standard
# won't recognize it as being code:
def foo(object)
  ...
  object.instance_eval(<<~RUBY, __FILE__, __LINE__ + 1)
    # This will definitely work with the standard restrictions, because it's a string.
    def self.bar
      ... # This code is Ractor-safe if object is made sharable or transferred.
    end
  RUBY
  object
end

For generating custom singleton methods, you can't directly use def object. anyway, but I'll cover this too:

# Using define_singleton_method (doesn't work with BasicObject):
def foo(object)
  ...
  object.define_singleton_method(:bar) do
    ... # This code can have Ractor sharing issues, even if object is made sharable or transferred.
  end
  object
end

# Using singleton_class (doesn't work with BasicObject):
def foo(object)
  ...
  object.singleton_class.class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
    def bar
      ... # This code is Ractor-safe if object is made sharable or transferred.
    end
  RUBY
  object
end

# The only way to do custom methods that works with BasicObject is that instance_eval string code from before:
def foo(object)
  ...
  object.instance_eval(<<~RUBY, __FILE__, __LINE__ + 1)
    # This will definitely work with the standard restrictions, because it's a string.
    def self.bar
      ... # This code is Ractor-safe if object is made sharable or transferred.
    end
  RUBY
  object
end

I bring up the BasicObject stuff because delegator classes are usually derived from BasicObject, and it pops up in other places.

Nonetheless, you can see that for the case where the same singleton method is added to the object every time, the cleanest code is to use the def object. style. Any restriction on using that style makes the code harder to understand and less pleasant to read and write.

For what it's worth, I really think that Matz's comment was specifically not about defining singleton methods, as he didn't mention that at all; he only mentioned instance methods. The fact that there is no other way than def object. to define singleton methods on BasicObject objects is a sign that it might be intentional that they allow them to be defined this way in method code. Don't take my word for it though.