Skip to content
This repository has been archived by the owner on Apr 27, 2021. It is now read-only.

Render parens on zero arity fun definitions #51

Closed
lpil opened this issue Aug 1, 2017 · 5 comments
Closed

Render parens on zero arity fun definitions #51

lpil opened this issue Aug 1, 2017 · 5 comments

Comments

@lpil
Copy link
Owner

lpil commented Aug 1, 2017

https://github.com/lexmag/elixir-style-guide#fun-parens

@rgreenjr
Copy link
Contributor

I'll start work on this one.

@rgreenjr
Copy link
Contributor

This looks like it will be fairly straightforward (famous last words).

However I've run into a problem. The semantic tests in Semantic.ExfmtProjectTest fail anytime parentheses are added to zero arity function definitions.

For example, changing the macro in Exfmt.Ast.Infix from:

defmacro infix_ops do
    @infix_ops
end

to:

defmacro infix_ops() do
    @infix_ops
end

produces this error:

** (Exfmt.SemanticsError) Error: The semantic meaning of the source code differs
     between the input and the formatted output! We are unable
     to continue as formatting may break your code.

I'll dig into this, but wanted to post this in case you knew of an easy fix.

@rgreenjr
Copy link
Contributor

The AST produced by a zero arity function definition without parentheses:

quote do
  def one do
    1
  end
end
{:def, [context: Elixir, import: Kernel],
 [{:one, [context: Elixir], Elixir}, [do: 1]]}

differs more than I expected from the same function definition with parentheses:

quote do
  def one() do
    1
  end
end
{:def, [context: Elixir, import: Kernel],
 [{:one, [context: Elixir], []}, [do: 1]]}

The semantic check is getting tripped up by this difference.

@OvermindDL1
Copy link

'def' is just a macro invocation, and the change is from going from a 'binding' ast to a 'call' ast, which is definitely a big change. Internally the 'def' is a macro (as might other things be too) that can accept both (design fault in my opinion...).

@rgreenjr
Copy link
Contributor

Thanks @OvermindDL1, that's very helpful.

rgreenjr added a commit to rgreenjr/exfmt that referenced this issue Aug 17, 2017
rgreenjr added a commit to rgreenjr/exfmt that referenced this issue Aug 17, 2017
@lpil lpil closed this as completed Nov 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants