christopheradams / elixir_style_guide

A community driven style guide for Elixir
4.37k stars 300 forks source link

`Alias` should have higher priority then `use` and `import` #141

Closed virtual-light closed 7 years ago

virtual-light commented 7 years ago

Why do you think that iii. use iv. import v. alias is a rational hierarchy?

I think that alias should go first because unlike the use or import it can affect on other declarations in module including use and import. We all saw that case with GenStage when it was in experimental namespace:

alias Experimental.GenStage

use GenStage

so result of use or import statements could depend on alias statement. Order of alias declarations have a semantic sense that affects on other declarations. This is not only question of style.

christopheradams commented 7 years ago

Related to #117, it would be helpful to document why this module order may or may not be preferred.

My intuition is that @behaviour and use are listed first because they are very strong indicators of how the module is going to behave, whereas import and alias are mere conveniences.

The example you point to seems like the exception that proves the rule. The Announcing GenStage article puts the alias at the top of the file, not the module (possibly to account for multiple modules in a file or modules that already refer to GenStage everywhere; but more likely I think to highlight how unusual it is):

alias Experimental.GenStage

defmodule A do
  use GenStage
end
virtual-light commented 7 years ago

My intuition is that @behaviour and use are listed first because they are very strong indicators of how the module is going to behave

That is correct but only for "working" code in module. It has no affect on the declarations (like import or alias) like alias has.

The example you point to seems like the exception that proves the rule.

My example just illustrates that use or import depends on alias but alias doesn't depends on them. There could be a lot of variants of such behaviour. Aliases work for use or import statements just the same as for other code in file. Aliases are frequently used to define shortcuts so you can call shortened module name in use after you declared alias (before use). It you have A that not depends on B and C declarations, but B and C depends on A declaration this is rational that A have higher priority.

christopheradams commented 7 years ago

Aliases are frequently used to define shortcuts so you can call shortened module name in use after you declared alias

I would suggest a countermanding rule that you should never use aliases in any of the directives (use, require, etc.). Rather, they should only be used below that, in the module body and its functions.

I guess the use of alias Experimental.GenStage was only a temporary requirement, with the idea that you could remove that single line when the final package was released.

But I'm open to seeing examples to the contrary.

virtual-light commented 7 years ago

I would suggest a countermanding rule that you should never use aliases in any of the directives (use, require, etc.). Rather, they should only be used below that, in the module body and its functions.

Why we should have this restriction when language provides such ability?

ericmj commented 7 years ago

That is correct but only for "working" code in module. It has no affect on the declarations (like import or alias) like alias has.

This is not true since use can bring in aliases and other things that affect things.

christopheradams commented 7 years ago

Why we should have this restriction when language provides such ability?

The styles here are only recommendations and common practices, not restrictions. For example, you can scope an alias inside a function, but almost no one does that, and so I would not recommend it in this guide.

This is not true since use can bring in aliases and other things that affect things.

Exactly. This supports my initial argument, that use presages how a module will work and what role it will play, and so it's natural to list it first.

The main purpose of aliases is to act as short cuts in function calls and pattern matches, in order to shorten lines and improve readability. This is less of a concern in module directives themselves, which are only ever as long as the module name (plus options), and our understanding benefits from having the full module written out. Only in exceptional circumstances (such as Experimental.GenStage), would I recommend otherwise.

If anyone can help formulate justifications for how module attributes/directives should be ordered, or point out problems/pitfalls, it would be welcome addition to the guide.

virtual-light commented 7 years ago

This is not true since use can bring in aliases and other things that affect things.

It can affect "declaration part" code only by bringing alias about what we talking here.

The styles here are only recommendations and common practices, not restrictions.

It is looks like you invented a rule (without any reasons expect "style") and when it was shown where it is not works you invented a new one with no reasons just to reinforce the first rule.

virtual-light commented 7 years ago

The main purpose of aliases is to act as short cuts

So what the reasons to not use shortcuts for "declarations part" code if the language provides ability to do this? And as you saw it with Experimental.GenStage it can do not only shortcuts but selecting a specific namespace.

This is not true since use can bring in aliases and other things that affect things.

As I said, alias can affect use but the converse is not true. So alias should have higher priority.

christopheradams commented 7 years ago

I hear what you're saying @virtual-light. You might want to alias a module first and then use it.

I did a search across almost 1000 modules from the standard library and popular packages, with either a use or an alias. I found that use is almost universally listed first, before alias.

The only counter-examples I could find were in Credo, e.g.:

  alias Credo.Check.Design.TagHelper

  use Credo.Check, base_priority: :high

https://github.com/rrrene/credo/blob/f93e2bb1c00b20c372228371c46eeffa5a3a1ef5/lib/credo/check/design/tag_fixme.ex#L22

But even there, the placement of alias has no effect on use.

Although it is mostly arbitrary, the preferred style is to have use first and to write out complete module names in the directives (so no aliases).

Thanks for kicking off this discussion, but we are not considering changing the recommended order at this time.

mustafaturan commented 5 years ago

@christopheradams majority of the developers follow this guideline, it could be the reason not to see many projects use the ‘alias’ first. The ‘alias’ clearly helps not to duplicate full module name writing on any of the others including ‘use’, ‘import’, ‘require’ and ‘@behaviour’

cdeyoung commented 5 years ago

I've always used this order: use require import alias

Use seems to do the most complex things and alias seems to do the least complex things, so the order feels right to me in that regard. It also seems to be common throughout the third-party code I use. Some code is organized differently, but most are this way.

I wouldn't consider the example of Experimental.GenStage a ubiquitous one. It seems more a clever use of the language, to be able to use an experimental module and then switch to the release version without having to change any code (other than deleting the alias line, of course). It's not a pattern I'd expect to use day-to-day. Personally, if it were me, I'd have written:

use Experimental.GenStage alias Experimental.GenStage

for the lake of clarity. Yes, it is redundant, but I don't have to think about it when reading the code.

christopheradams commented 5 years ago

Good reasoning @cdeyoung.