Formatter inconsistency with typespecs

[Vincent Siliakus]

Hi,

Maybe there's a good reason for the current behaviour, so I'm posting it here instead of opening an issue on Github.

Consider the following typespecs:

@type strkey_kv :: [{String.t, term}]

@type kv :: strkey_kv | map | Keyword.t

When these typespecs get processed by Elixir's new format option, the result is:

@type strkey_kv :: [{String.t(), term}]

@type kv :: strkey_kv | map | Keyword.t()

Note the added braces to String.t and Keyword.t. Now I don't have a strong opinion on writing types with or without using braces, but I don't think adding braces only to external types is good.

Can this be considered a bug or is there a reason for the current behaviour?

Best,
Vincent

[Eric Meadows-Jönsson]

Typespecs are following the same rules as all other code because the formatter only sees AST and can't discern typespecs from executed code.

We can't add parenthesis to nullary local function calls because they have the same AST representation as variables so adding parenthesis would break code.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/a53d03cb-dd9e-4945-a5d1-c2f23f540c7e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Eric Meadows-Jönsson

[Vincent Siliakus]

I understand the reason that parenthesis can't be added to nullary local functions and that if you walk the AST without keeping any state the formatter can't discern a local type from a local nullary function call.

However, when the formatter does keep some state while walking the tree it should be possible for the formatter to understand which AST nodes are part of a typespec?

Given the following typespec:

@type my_type :: my_other_type

The corresponding AST should be something like:

{:@, [line: 1],
 [
   {:type, [line: 1],
    [
      {:::, [line: 1],
       [{:my_type, [line: 1], nil}, {:my_other_type, [line: 1], nil}]}
    ]}
 ]}

So the formatter encounters an attribute node {:@, ...}, updates it state that if the next node starts with :type, we are at the start of a typespec, etc.

Is interpreting the AST like this considered too fragile/brittle, or am I missing something else?

Best,
Vincent

Op maandag 5 februari 2018 15:10:05 UTC+1 schreef Eric Meadows-Jönsson:

Typespecs are following the same rules as all other code because the formatter only sees AST and can't discern typespecs from executed code.

We can't add parenthesis to nullary local function calls because they have the same AST representation as variables so adding parenthesis would break code.

--

Eric Meadows-Jönsson

[Michał Muskała]

The problem is that any AST can be intercepted by macros before it reaches the compiler. For example, you could easily have a macro that can be used like:

    my_macro @type t :: other_type

And that rewrites the @type annotation to something completely different. Changing other_type to other_type() would change what AST the my_macro receives and it has potential to break code. That's why one of the core principles of the formatter is to never change the AST.

Perhaps there should be a way to allow formatter some of those "unsafe" transformations, but I think this is somewhat a separate discussion.

Michał.

--

You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/f1efdaf8-c8b8-4197-b1f2-622bd5b2190e%40googlegroups.com.

[Vincent Siliakus]

Ah, I see. Thanks for insight, I didn't consider this and appreciate the better safe than sorry approach regarding AST transformations.


Op maandag 5 februari 2018 16:43:14 UTC+1 schreef Michał Muskała: