lexmag/elixir-style-guide

An opinionated Elixir style guide

elixir-style-guide's Issues

Multi-line struct assignment

I'd appreciate verification on the preferred way to wrap multi-line struct assignments. The three possible scenarios are depicted below.

Thanks in advance!

1. Left hand side fits, but right doesn't:

datetime =
  %NaiveDateTime{calendar: Calendar.ISO, day: 1, month: 12, year: 2017}

2. Right hand side fits, but left doesn't:

%NaiveDateTime{calendar: calendar, day: day, month: month, year: year} =
  datetime_struct

3. Neither right nor left hand side fits:

%NaiveDateTime{
  calendar: calendar,
  day: day,
  hour: hour,
  microsecond: microsecond,
  minute: minute,
  month: month,
  second: second,
  year: year} = 
  %NaiveDateTime{
    calendar: Calendar.ISO,
    day: 1,
    hour: 23,
    microsecond: {0, 0},
    minute: 59,
    month: 1,
    second: 59,
    year: 2017
  }

Code style linter

Add rule that expected result goes on the right of ExUnit assertion

"Blocks" inside pipes

I don't know if calling it blocks would be the correct way, but basically I'd like to know what would be the preferred way of organizing cases, (), {} and [] inside pipes.
The situation would be something like this:

Repo.get(App, "app_id")
|> case do
     nil -> %App{}
     app -> app
   end
|> App.changeset(%{
     id: "new_app_id",
     name: "The name of this app is something kinda long so it needs a line for that"
   })
|> Repo.insert_or_update!

Right now I'm using the way shown above because it makes sense to me to close it in the same indentation level of the beginning of the "block", but... it's a little bit weird and totally not clean.

Add rule about defstruct/1

# Bad
defstruct [year: nil, month: nil, day: nil, calendar: Calendar.ISO]

# Good
defstruct [:year, :month, :day, calendar: Calendar.ISO]

How to split long function specs?

Let's say I have a very long function spec:

@callback handle_call(request :: term, state :: term) :: {:ok, reply, new_state} | {:ok, reply, new_state, :hibernate} | {:remove_handler, reply}

How do we split this?

How to split guard clause attached to a case condition?

I have this piece of code with a very long guard clause attached to a case condition:

case ast do
  {sym, meta1, [{fun, meta2, nil} | rest]} when sym in [:def, :defp, :defmacro, :defmacrop, :defdelegate] ->
    {sym, meta1, [{fun, meta2, []} | rest]}
  _ ->
    ast
end

I split it after the when operator with extra indentation like so:

case ast do
  {sym, meta1, [{fun, meta2, nil} | rest]} when
      sym in [:def, :defp, :defmacro, :defmacrop, :defdelegate] ->
    {sym, meta1, [{fun, meta2, []} | rest]}
  _ ->
    ast
end

Is this correct?

How to indent multiline guard clause?

Context: uohzxela/ex_format#63

Style 1:

  defp parenless_capture?({atom, _, _})
       when is_atom(atom) and
       atom not in unquote(@unary_ops) and
       atom not in unquote(@binary_ops) do
    true
  end

Style 2:

  defp parenless_capture?({atom, _, _})
       when is_atom(atom) and
            atom not in unquote(@unary_ops) and
            atom not in unquote(@binary_ops) do
    true
  end

Which one is preferred?

Some hints that was found when working on examples for Glavin001/atom-beautify

1. @lexmag: commented about: true-in-cond rule in Glavin001/atom-beautify#1421.

Now I see, the rule wasn't clear, it meant the we should always use true and not any other atom since cond allows any:

cond do
  :any -> :ok
end

2. When parsing README.md I found unneeded (?) link in pipeline-operator. As I can see all other rules have links only at end of first paragraph for rule (list element in list for header level 3 - h3 - element):

<a name="needless-pipeline"></a>

How is it different from christopheradams/elixir_style_guide?

It would be very useful to pinpoint the differences from the most popular https://github.com/christopheradams/elixir_style_guide guide in the readme file.

Review other style guides and refer to their rules

There are lots of other style guide like:

• The Elixir Style Guide by @christopheradams
• Credo's Elixir Style Guide by @rrrene

Do you interested in comparing them and add/modify rules (of course in our own way) here?
Can I take it and create a PR's for each style guide, so we can discuss about possible changes?

Parenthesis around anonymous function arguments

Good or bad?

`import` best practices

I have a few questions about best practices when using import, and the like.

When using import, should I simply import the entire module and not bother with the :only option?

# ...

import App.Web.SessionHelpers
import Comeonin.Bcrypt

This approach certainly looks the cleanest, but what if the module being imported has a large number of public functions? With that I risk polluting the namespace with a bunch of unused functions. It makes sense to only import what one needs, yes?

Additionally, when a function is called within a module which was imported from a different module, how will I or future maintainers know where it came from (easily, anyway)? I know I've seen this in code before, I'll see a function like clear_user_password(user) and the function doesn't exist as a function within the current module and there are like ten imports (oy!). Using :only can help point future maintainers in the right direction, as it makes what is used and from where very clear.

# ...

import App.Web.SessionHelpers, only: [clear_current_user: 1, get_current_user_safe: 1, set_current_user: 2 ]
import Comeonin.Bcrypt, only: [checkpw: 2]

But like above, this can easily get out of hand. So is it better in this case to stack them, like below?

# ...

import App.Web.SessionHelpers, only: [
  clear_current_user: 1,
  get_current_user_safe: 1,
  set_current_user: 2,
]

import Comeonin.Bcrypt, only: [
  checkpw: 2,
]

Or is there no "right" way, because it depends heavily on the context. E.g., if I'm using the majority of the functions from an imported module I should just import it without :only, but use :only if I'm just borrowing a function or two?

Please advise, thank you :)

Document the correct parens usage with non zero-arity types

From this section, we know that with zero-arity types, we must use parens. This is the case for the following examples:

type ::
      any()                     # the top type, the set of all terms
      | none()                  # the bottom type, contains no terms
      | atom()
      | map()                   # any map
      | pid()                   # process identifier
      | port()                  # port identifier
      | reference()
      | tuple()                 # tuple of any size
      | float()
      | integer()
      | neg_integer()           # ..., -3, -2, -1
      | non_neg_integer()       # 0, 1, 2, 3, ...
      | pos_integer()           # 1, 2, 3, ...

but what about optional types in the following cases?

type ::
      | list(type)                                                    # proper list ([]-terminated)
      | nonempty_list(type)                                           # non-empty proper list

or custom types exposing the same behaviour? Should we use parens or not?

Question about multiline expression assignment

With reference to this rule, we have an example as follows:

{found, not_found} =
  Enum.map(files, &Path.expand(&1, path))
  |> Enum.partition(&File.exists?/1)

My formatter output:

{found, not_found} = Enum.map(files, &(Path.expand(&1, path)))
|> Enum.partition(&(File.exists?() / 1)) 

Is it stylistically correct?

Here's another example with binary operator:

result =
  op_to_string(left, fun, :::, :left) <>
  "::" <>
  bitmods_to_string(right, fun, :::, :right)

result = op_to_string(left, fun, :::, :left) <>
"::" <>
bitmods_to_string(right, fun, :::, :right)

Question about parenthesis on chained zero arity function calls

Is there a standard for parenthesis on chained zero arity function calls? For example,

Mix.shell.info("hello")

versus

Mix.shell().info("hello")

The style guide generally seems to prefer parenthesis for clarity, but I didn't see this particular case addressed.

I ask because I'm working on the exfmt project and need to implement the rules for this scenario.

Thanks!

Mention "warnings as errors" option

Could be set via --warnings-as-errors CLI switch, or Code.compiler_options(warnings_as_errors: true) call.

Add more examples and "Bad" versions of examples to rules

Lots of rules don't have a sample code and most of rules also don't have a "Bad" version of example.

Some beginners do not know perfectly English (how translation tools work I do not need to explain - there are also not perfect) and other developers (like me) can misunderstand some rules. To avoid this I suggest to add/extend examples.

When I created an examples for atom-beautify I added more examples and all have "Bad version" (original) and "Good" version (expected). There shows why this rules are important and there were already reviewed by @lexmag, so they do not need lots of edits to add them here.

Here is my PR: Glavin001/atom-beautify#1421

Do you interested in this? What do you think about it? Can I take it and add PR based on examples from atom-beautify?

Parenthesizing captures

Today there's not written rule in the style guide, but we do write captures like this:

&not(&1 or &2)
&if(&1, do: &2, else: :default)

instead of how the style guide would suggest (especially for things like not):

&(not (&1 or &2))

While I like the first version better, I think we should go with the second version for consistency. It would also make a formatter's job easier. Wdyt @lexmag and @josevalim?

Specify multi-line struct format

%Pet{name: "Felix",
     species: "Cat"}

%Pet{
  name: "Felix",
  species: "Cat",
}

Which is preferred? :)

Elixir Style reference card format

I was a little bit bored and I made this reference card as an adaptation of the Elixir Style Guide. It was also a good way to remember a few things from the style guide :)

I already printed and pinned this refcard and looks good on my desk, if you agree I plan to publish the source code (LaTeX) in https://github.com/milmazz/elixir-style-refcard under the same license (CC BY 4.0)

@lexmag Please let me know what do you think? Do you have any suggestion?

Use markdown headers instead of explicit link

Nice guide, but is there a reason to add all the links at the end of the bullet points?
GitHub markdown automatically creates deep links for headers if you change the format a bit from:

## Linting

* <a name="pipeline-operator"></a>
  Favor the pipeline operator `|>` to chain function calls together.
  <sup>[[link](#pipeline-operator)]</sup>

to:

## Linting

### Pipeline Operator
Favor the pipeline operator `|>` to chain function calls together.

Zero arity fn -> spacing

Which is correct?

# 1
fn-> :ok end

# 2
fn -> :ok end

Related to: lpil/exfmt#13