Function result is `[]` when decorating a function head

[uesteibar]

When decorating a function head, the result is always [] if you return the result of the body:

If you have this:

defmodule DecoratorFunctionHead do
  use DecoratorFunctionHead.PrintDecorator

  @decorate print()
  def hello(first_name, last_name \\ nil)

  def hello(:world, _last_name) do
    :world
  end

  def hello(:earth, _last_name) do
    :earth
  end

  def hello(first_name, last_name) do
    "Hello #{first_name} #{last_name}"
  end
end

Then tests will fail as:

1) test greets the world (DecoratorFunctionHeadTest)
     test/decorator_function_head_test.exs:4
     Assertion with == failed
     code:  assert DecoratorFunctionHead.hello(:world) == :world
     left:  []
     right: :world
     stacktrace:
       test/decorator_function_head_test.exs:5: (test)

I did a little digging and it seems to be because body for a function head is [] (empty AST), so that's what it returns. I didn't have time to dig into it deeper though, I might be able to do it in the following days/weeks. For now we managed to have a workaround and not use a decorator when we have a function head.

I've created this repo to reproduce the issue: https://github.com/uesteibar/decorator_function_head

[arjan]

Thanks for the report!

I think decorating an empty function head should be disallowed, because you are supposed decorate each individual function clause, right?

[uesteibar]

that's what I thought at first, but changing this to:

defmodule DecoratorFunctionHead do
  use DecoratorFunctionHead.PrintDecorator

  def hello(first_name, last_name \\ nil)

  @decorate print()
  def hello(:world, _last_name) do
    :world
  end

  @decorate print()
  def hello(:earth, _last_name) do
    :earth
  end

  @decorate print()
  def hello(first_name, last_name) do
    "Hello #{first_name} #{last_name}"
  end

The test still fails:

see failing test

``` 1) test greets the world (DecoratorFunctionHeadTest) test/decorator_function_head_test.exs:4 Assertion with == failed code: assert DecoratorFunctionHead.hello(:world) == :world left: [] right: :world stacktrace: test/decorator_function_head_test.exs:5: (test) ```

and when running mix compile --warnings-as-errors this comes up:

warning: variable "first_name" is unused (if the variable is not meant to be used, prefix it with an underscore)
  lib/decorator_function_head.ex:4: DecoratorFunctionHead.hello/2

warning: variable "last_name" is unused (if the variable is not meant to be used, prefix it with an underscore)
  lib/decorator_function_head.ex:4: DecoratorFunctionHead.hello/2

warning: def hello/2 has multiple clauses and also declares default values. In such cases, the default values should be defined in a header. Instead of:

    def foo(:first_clause, b \\ :default) do ... end
    def foo(:second_clause, b) do ... end

one should write:

    def foo(a, b \\ :default)
    def foo(:first_clause, b) do ... end
    def foo(:second_clause, b) do ... end

  lib/decorator_function_head.ex:1

warning: this clause cannot match because a previous clause at line 1 always matches
  lib/decorator_function_head.ex:1

Also, looking at https://github.com/arjan/decorator/pull/27 I understand that decorating the function head would be the right thing to do. I see we're trying to ignore empty clauses here, which was also added by that PR

However, after some digging I found out that the body for an empty clause is [] insteado of nil from elixir 1.10.0 to 1.10.2, and that was fixed in 1.10.3 as seen in the changelog:

This can be closed, as updating to the latest elixir version fixes it! If it happens to someone else hopefully they can find this issue :)

[arjan]

Ahh, yes, I noticed that in the 1.10.2 changelog. So we'll leave it as a wontfix then. I'll update the docs to reflect the fact that you should decorate the function head instead of the individual members.

[arjan]

https://github.com/arjan/decorator#functions-with-multiple-clauses done :-)

[uesteibar]

that's great, thanks! 🎉