arjan / decorator

Function decorators for Elixir
MIT License
386 stars 21 forks source link

Elixir 1.5 Errors #11

Closed asummers closed 7 years ago

asummers commented 7 years ago

Hi there! On Elixir 1.5-rc.0, this library seems to be misbehaving on rescue blocks when a function is @decorated. It can be pretty simply reproduced with the following code, most of which is from the README.

defmodule PrintDecorator do
  use Decorator.Define, [print: 0]

  def print(body, context) do
    quote do
      IO.puts("Function called: " <> Atom.to_string(unquote(context.name)))
      unquote(body)
    end
  end

end

defmodule Testing do
  use PrintDecorator

  @decorate print()
  def hello do
    :world
  rescue
    e -> IO.inspect e
  end

end

When you try to compile, it errors with:

== Compilation error in file lib/testing.ex ==
** (CompileError) lib/testing.ex:24: unhandled operator ->
    (stdlib) lists.erl:1354: :lists.mapfoldl/3
    (stdlib) lists.erl:1355: :lists.mapfoldl/3
    /Users/asummers/Code/testing/lib/testing.ex:13: Decorator.Decorate.before_compile/1

I'm looking around to see if I see anything immediate but wanted to open this issue for visibility / to see if you had any ideas off the top of your head of what might be causing this. It's also entirely possible this is a regression in Elixir itself.

arjan commented 7 years ago

Thanks for the report. I never use rescue on the function level. It seems it always worked magically, but in Elixir 1.5 they changed the AST for these constructs. I've updated the code.

josevalim commented 7 years ago

Hi @arjan, just to clarify, the change was a bug fix as it was impossible to distinguish a bodyless clause from a function that returned nil. This could even be the source of a bug in this lib. For example, if someone did:

@decorator Bla
def foo(bar, baz)

def foo(baz, baz) do
  ...
end

Note however that not only do/rescue is possible, but do and any of rescue/catch/after. So you probably want to check the Elixir version for v1.5+ and do nothing if the body is nil and, if a keyword list, you traverse all pairs.

arjan commented 7 years ago

Thanks I'll check that out and make sure it has coverage!

asummers commented 7 years ago

Thank you @josevalim and @arjan ! ❤️