elichai / log-derive

A procedural macro for auto logging output of functions
https://docs.rs/log-derive
Apache License 2.0
189 stars 12 forks source link

Attribute parsing allows weird input with unexpected results #3

Closed TedDriggs closed 5 years ago

TedDriggs commented 5 years ago

Consider the following code:

#[logfn(INFO, fmt = "fibonacci() -> {}", ok = "Trace", Warn)]
fn fibonacci(n: u32) -> u32 {
    match n {
        0 => 1,
        1 => 1,
        _ => fibonacci(n - 1) + fibonacci(n - 2),
    }
}

What level of log message will this emit?

The answer turns out to be trace, because the ok = "Trace" overrules the INFO and Warn elements, despite the return value not being a result.

Removing the ok clause causes these to become warnings: The Warn word overwrites the earlier INFO word.

There are a few things I was expecting to be different here.

  1. Specifying the general level more than once would produce an error spanned to each subsequent attempt to redeclare. This is how serde handles it (example).
  2. Attempting to use the shorthand for log-level as not the first element in the attribute would produce an error

While I get the desire to be permissive with input here, I also sort of wish the library was more particular about casing and positioning, requiring:

  1. Attribute names to be snake_case to match the standard Rust style
  2. Level to appear first, or possibly allowing it to appear later as level = "warn" with the first position being a magic shorthand.
elichai commented 5 years ago

I was expecting the first thing to be right too, I'll look into this. About the second one I'm not sure if that's needed.

what exactly isn't snake_case here? and I'm thinking maybe to throw an Error if ok/err is used on non Result function, this might be in combination with #2

elichai commented 5 years ago

Fixed By your PR #8