[onk]

module_function makes the instance method private and puts a copy on the module itself. The module in this example is a namespace that nothing includes, so it carried a private instance method for every class method the bad example defined.

With def self., the good example differs from the bad one only in the class keyword.

The module_function rule below is unaffected. It applies when a module's methods are
instance methods and you want them callable on the module too, which is what
module_function is for.

Style/StaticClass enforces this rule and its autocorrect had the same problem;
rubocop/rubocop#15770 changes it to correct the same way.

[pirj]

They serve different purposes.

module A
  module_function
  def a
    1
  end
end

module B # composed interface
  extend A
  # extend SomeOtherModule

  module_function
  def b
    a + a
  end
end

B.b # => 2

module A
  def self.a
    1
  end
end

module B
  extend A

  def self.b
    a + a
  end
end

B.b # 'B.b': undefined local variable or method 'a' for module B (NameError)

More on that in https://practicingruby.com/articles/uses-for-modules-3a

Even though the guideline is about not using classes for cases when they are not purposed to be instantiated, I'm reluctant to accepting a change that can result in surprises like above.

[onk]

@pirj

This example uses module_function for something that is not a module function. That's the whole of what I want to fix.

module_function is for module functions. The docs define them as methods that are "private AND singleton methods of the module" — the point being that you can reach them both ways (doc/extension.rdoc):

Math.sqrt(4)

# or
include Math
sqrt(4)

Module#module_function says the same: "These functions may be called with the module as a receiver, and also become available as instance methods to classes that mix in the module. The instance-method versions are made private."

Nothing includes SomeModule, so the second half never applies.

It's true that extend only adds a module's instance methods, so def self. methods aren't picked up. But that's a different question from whether module_function should be used for something that is not a module function, which is all this change is about.

Could you reopen this?

[pirj]

From the docs:

The instance-method versions are made private.

Please correct me if I’m mistaken that this is when a class includes a module that defines a module function. This is not the case described in the guideline.

This example uses module_function for something that is not a module function.

Can you please elaborate? What is your perspective of a module function?

The docs define them as methods that are "private AND singleton methods of the module"

I struggle to see this from the doc. Can you please point me to where it is?

This example uses module_function for something that is not a module function.

What makes you think so? A.a works. And, equally, you can call a from where you include A.

As I briefly mentioned the example in my previous comment shows the usage of such modules that go a bit beyond just using them as standalone module function bags: for building a larger interface out of smaller ones. Think of this:

module Math
  extend Algebra
  extend Trigonometry
  extend Logarithms 
  extend Roots
  extend Gamma
end

Can you build it with def self.?

whether module_function should be used for something that is not a module function, which is all this change is about

I agree that this change does affect the section that is focused on something that will be a receiver of class methods only. Not necessarily included or used to extend something.
And yes, def self., as you propose, can be used for this sole purpose.
And there’s another guideline beneath it that recommends using module_function instead of extend self to turn them into class methods. Without ever saying why.
Doesn’t this make you think that those two would be contradicting each other after your suggested change?

[onk]

Answering your questions in order.

(1) module_function creates an instance method

The instance-method versions are made private.

Please correct me if I'm mistaken that this is when a class includes a module that defines a module function. This is not the case described in the guideline.

The private instance method is created on the module itself at the moment module_function runs. No include is involved.

module A
  module_function
  def a; end
end

A.private_instance_methods(false) # => [:a]
A.singleton_methods               # => [:a]

module_function is implemented as rb_mod_modfunc.
It makes the method private on the module itself, then copies it to the module's singleton class.

https://github.com/ruby/ruby/blob/v4.0.7/vm_method.c#L3315

The C API counterpart, rb_define_module_function, is the same pair.

https://github.com/ruby/ruby/blob/v4.0.7/class.c#L2848-L2849

I struggle to see this from the doc. Can you please point me to where it is?

It is in the documentation for writing extensions. I brought it up because it uses Math.sqrt as the example, which I thought made the shape easier to see.

https://github.com/ruby/ruby/blob/v4.0.7/doc/extension.rdoc#L411-L426

At last, rb_define_module_function defines a module function,
which are private AND singleton methods of the module.

(2) What is a module function?

What is your perspective of a module function?

A module function is a method that can be called with the module as the receiver, and is also available as an instance method wherever the module is mixed in. Math.sqrt in the doc above is exactly that.

Math.sqrt(4)

# or
include Math
sqrt(4)

module Math
  extend Algebra
  extend Trigonometry
  extend Logarithms
  extend Roots
  extend Gamma
end

Can you build it with def self.?

No. extend mixes a module into the receiver's singleton class, so methods defined with def self. are not picked up by extend.

That said, what this needs is not module_function. A plain def is enough.

module MyAlgebra
  def sqrt(x) = "MyAlgebra#sqrt(#{x})"
end

module MyMath
  extend MyAlgebra
end

MyMath.sqrt(4) # => "MyAlgebra#sqrt(4)"

module_function is what you need when you want to offer both call forms shown above.

"Modules vs Classes" covers the case where only one of the two is ever used. A class cannot be included or extended in the first place, so there are no mixin users before the conversion. The reason to make it a module is that we never instantiate it, not that we want to mix it in. Using module_function here adds one instance method per class method that the original class never had.

(3) Do the guidelines contradict each other?

Doesn't this make you think that those two would be contradicting each other after your suggested change?

I don't think they contradict. The two guidelines start from different premises, and there is no code where both apply at once.

The "module_function" guideline is conditioned on "when you want to turn a module's instance methods into class methods". It assumes instance methods already exist, and tells you to pick module_function over extend self.
"Modules vs Classes" is about a class that has only class methods. There are no instance methods, and nothing to mix it into.

The only connection between them is that they are written one after the other, and two adjacent guidelines are not required to cover the same case.

[onk]

module_function is needed only when both call forms are actually used. That means the module is a namespace and a mixin at the same time, which puts two design intents in one module and is not the usual shape.

The module in the "Modules vs Classes" example is only ever a receiver. The instance-method half is never reached, so showing module_function there presents the uncommon case as the default shape for a module with class methods.

Is there a reason to show module_function in that example that I'm not seeing?

[pirj]

This is an argument against the other guideline:

Prefer the use of module_function over extend self when you want to turn a module's instance methods into class methods.

that doesn’t even bother mentioning a totally reasonable class << self as an option, nor to provide an example usage where the module is included.

I don’t have sufficient historical context to answer why it is so. But my assumption is that there are just too many possible usage scenarios, and with at least those three (and a half with def self.foo) options, and the dense format of the guide, the most practical way was picked as recommended.

From README:

The guidelines … are also meant to reflect real-world usage of Ruby instead of a random ideal. When we had to choose between a very established practice and a subjectively better alternative we’ve opted to recommend the established practice.

I clearly see and totally agree with you that what you propose is a better option in certain scenarios.
My memory also tells me that I see module_function less frequently than class << self, but more frequently than extend self.
Personally, I try to avoid making modules to serve double purpose, and opt for class << self.

But none of this is sufficient to change the recommended style.

Do you want to harvest the real-world-ruby/real-world-rails meta-repos to gather information on usage, purpose and design to show that the change that you propose is the real-world practice?

[onk]

I haven't argued against the module_function guideline. Nothing in this PR or in my comments compares module_function with extend self. I don't have a preference between the two, and I'm not proposing to change that guideline.

This PR is only about the code example under "Modules vs Classes". My question from my previous comment is still open: is there a reason to show module_function in that example?

[onk]

You mentioned class << self. I agree it's a good way to write the "Modules vs Classes" example too. It doesn't add instance methods, which is what I care about here.

But I think def self. is a little better. The methods are written the same way as in the bad example, so the only difference is class vs module. If you like class << self more, I can change it.

[pirj]

I clearly see what you mean now.
You want to improve parity between the bad and the good code and shift the emphasis towards the class -> module change. In the current form, the guideline has two changes 1) class -> module 2) def self. -> module_function.

Is my understanding correct?

[onk]

Yes, that's right. Thank you!

And module_function also adds instance methods.

[onk]

@pirj Any update?