Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alias_method should be preferred to alias #821

Open
marcandre opened this issue Jun 13, 2020 · 4 comments
Open

alias_method should be preferred to alias #821

marcandre opened this issue Jun 13, 2020 · 4 comments
Assignees
Labels

Comments

@marcandre
Copy link
Contributor

marcandre commented Jun 13, 2020

TLDNR: The styleguide and RuboCop should always prefer the use of alias_method.

Obscure difference

Ask 100 Rubyists to provide an example where alias and alias_method act differently. I would be surprised if a single one was able to.

I admit I couldn't. My friend and co-author of DeepCover couldn't.

The example given in the style guide isn't a good one either (use alias and it works in the same way).

I had to look it up

alias sometimes wrong, sometimes not possible, alias_method always works

The style guide admits that alias can be confusing in some cases and thus says to use it in other contexts.

It is implicitly admitted that there is never a case where alias actually does something better than alias_method does.

alias also doesn't accept a receiver, or variable arguments

invalid reasons

The reason given in the guide is:

Prefer alias when aliasing methods in lexical class scope

Please ask 100 rubyists what is a "lexical class scope".

as the resolution of self in this context

It is not the resolution of self, but the resolution of the "currently opened class". The self is never lexical. Take the example of the blog, wrap the alias in a instance_eval and things remain the same. Wrap it instead in a class_eval and things work; both change the self (here doing nothing as the receiver is self) but they affect the "currently opened class" differently. I find it quite obscure.

is also lexical

How is that an advantage? In these cases, the self and the currently opened class are always the same anyways. The existence of attr_reader, etc., make it such that we are used to think about the current self way more than the current open class.

and it communicates clearly to the user that the indirection of your alias will not be altered at runtime or by any subclass unless made explicit.

IIC, the idea behind the rule is that only uses of alias_method can affect a subclass (say within a def self.inherit?) and that alias won't since its usage is restricted to the simple cases. If so... why is that of any importance? When is that really a concern? When is it not obvious that an alias_method is impacting another class than the current class?

Complicated "when to apply"

Asking Rubyists to sometimes use alias and sometimes use alias_method overcomplicates things for no gain.

It makes some refactorings awkward:

class MyClass < Struct.new(:foo)
  alias foo? foo

  def bar
  end
end

# Refactored to:
MyClass = Struct.new(:foo) do
  alias_method :foo?, :foo  # How is that meaningfully different?

  def bar
  end
end

Note: I'm not sure at all what a "lexical class scope" means, I'm assuming that RuboCop's cop is correct here.

In short, I feel that alias was a language design mistake, an unnecessary keyword that is best avoided.

Any rule that attempts to distinguish when to use one or the other adds a complex cognitive load and will make distinctions when there aren't (as in example above). Note that a rule that would say "use alias unless alias_method acts differently" would not make unnecessary distinctions (by definition) but then falls under the "so complex that nobody knows when to apply it".

The one rule

The rule "always prefer alias_method" is straightforward, always applicable and can not be simpler. It should be the official rule of this guide and the default of RuboCop.

I will gladly provide a PR to fix this guide and RuboCop if it is accepted.

Notes:

See also this issue where @Ajedi32 puts forth similar arguments.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 12, 2020

TLDNR: The styleguide and RuboCop should always prefer the use of alias_method.

If memory serves - it was like this in the beginning, but then someone complained that alias is useful in some cases. I'm have to dig through the history here for details.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 12, 2020

Seems this is why things got changed #377 Probably the guide can be updated and everything on alias can just to go some sidebar for posterity's sake.

@dferrazm
Copy link

I'm also for alias_method instead of alias, since it's less confusing and with less surprises, especially when dealing with inheritance and mixins.

But after reading the discussions (on #377 and others) I feel like alias vs alias_method is a bikeshed like any other: '/ ", space / tabs, etc. Just with more trade-offs and particularities involved.

In this case, perhaps Rubocop should simply drop any sort of default for this one and have a neutral recomendation in its docs. Leaving the decision for the devs using it.

@pirj
Copy link
Member

pirj commented Feb 25, 2021

Magnificent explanation!

As a practical experiment I've replaced all alias with alias_method in rspec-mocks's source code, and specs passed just fine.

@pirj pirj added the Approved label Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants