basho/lager — pull request #54 by Vagabond (2012-12-13 06:44:03+00:00)
Title: Parse transform can't handle `Attr' as a variable

Log statements with `Attr` bound to a variable, causes the parse transform to fail with a `function_clause`. To reproduce, try out the following:

Attr = [{a, alpha}, {b, beta}],
lager:info(Attr, "Hello ~s", ["world!"]).

Crashes during compile with something like the following (cropped).

{function_clause, [
{lager_transform, concat_lists, [
{var,24,'Attr'}, ...

The cause is clear, it is expected that the `DefaultAttrs` can be appended to the passed property list of attributes, so there's no clause for a `var` in `concat_lists`.

**But** - It's already noted in `lager_transform.erl` that transformation of logging statements is ambiguous for calls with arity 3 (2 arguments). Sadly I think it's not only that, but actually also broken for arity 2 if `Attr` is a variable - the match for a property list doesn't work. Actually, Is is at all possible to walk the AST in a parse transform, when the variable is passed from outside the module? To me it seems that this could be the real bringer of bad news.

I like the general idea of lager, having log statements transformed in order to add implicit meta-data, but I think that the way the API is designed right now, it's very hard to support a more _free_ way of using, and abusing, it.

To be on the safe side I guess the API has to use stricter ordering for optional parameters i.e. attributes are always arity 3 and always on the end of the call. Another option could be to use tagged parameters.

Perhaps a solution for lager, and this API version, is to simply amend the documentation and more clearly state that the attributes, must be defined as an inline property list.

Or does someone else have a solution for this?

With kind regards
/O