Repository: shepmaster/snafu (rust)

Issue:
`#[snafu(source, backtrace)]` fails to parse
```
error: expected parentheses
  --> tests/source_and_backtrace.rs:19:23
   |
19 |         #[snafu(source, backtrace)]
   |                       ^

```

In fact, I'd guess the same would happen with any type that parses using `MyParens`, when placed in a non-final slot in the list of attributes.

Found in #141, missed during fix of multiple attributes in #137.

Pull request description:
Fix parsing of source/backtrace with no parens
If a plain `source` or `backtrace` attribute were specified in a non-final
position in an attribute list, we would fail to parse them because we assumed
`input` would be empty if we were at the end of an attribute.  However, `input`
is the parse stream for the whole attribute list, not just the attribute; it
could contain a comma and further attributes.

This change checks more carefully whether we're at the end of an attribute by
looking for empty input or a comma that signifies another attribute is coming.
Error messages are also improved, because it uses `Lookahead1` to let syn track
the tokens we want to allow.

Fixes #146

---

**Testing done:**

A new compile-fail test is added to show the handling of invalid identifiers inside `source(...)` using `Lookahead1`, and the multiple-attributes test was updated to include the `snafu(source, backtrace)` case which wouldn't parse before this PR.

---

Note that there's a slight timing issue with #141, and I'll rebase one on top of the other as shepmaster prefers.  (GitHub won't let me layer them by creating a PR in shepmaster/snafu with a base branch on my own fork.)

Either this PR should change `backtrace(delegate)` to `backtrace` if #141 is merged first, or #141 should change `source(true)` to `source` if this is merged first.  (Or shepmaster can edit one while merging, I suppose.)

Patch:
diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs
index b6a1e5f..2285243 100644
--- a/snafu-derive/src/lib.rs
+++ b/snafu-derive/src/lib.rs
@@ -967,6 +967,7 @@ enum SnafuAttribute {
 
 impl syn::parse::Parse for SnafuAttribute {
     fn parse(input: syn::parse::ParseStream) -> SynResult<Self> {
+        use syn::token::{Comma, Paren};
         use syn::{Expr, Ident, Visibility};
 
         let input_tts = input.cursor().token_stream();
@@ -985,18 +986,24 @@ impl syn::parse::Parse for SnafuAttribute {
                 .map_or_else(private_visibility, |v| Box::new(v) as UserInput);
             Ok(SnafuAttribute::Visibility(input_tts, v))
         } else if name == "source" {
-            if input.is_empty() {
+            let lookahead = input.lookahead1();
+            if input.is_empty() || lookahead.peek(Comma) {
                 Ok(SnafuAttribute::Source(input_tts, vec![Source::Flag(true)]))
-            } else {
+            } else if lookahead.peek(Paren) {
                 let v: MyParens<List<Source>> = input.parse()?;
                 Ok(SnafuAttribute::Source(input_tts, v.0.into_vec()))
+            } else {
+                Err(lookahead.error())
             }
         } else if name == "backtrace" {
-            if input.is_empty() {
+            let lookahead = input.lookahead1();
+            if input.is_empty() || lookahead.peek(Comma) {
                 Ok(SnafuAttribute::Backtrace(input_tts, Backtrace::Flag(true)))
-            } else {
+            } else if lookahead.peek(Paren) {
                 let v: MyParens<Backtrace> = input.parse()?;
                 Ok(SnafuAttribute::Backtrace(input_tts, v.0))
+            } else {
+                Err(lookahead.error())
             }
         } else {
             Err(syn::Error::new(


Test patch:
diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs
new file mode 100644
index 0000000..423c511
--- /dev/null
+++ b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs
@@ -0,0 +1,17 @@
+use snafu::Snafu;
+
+#[derive(Debug, Snafu)]
+enum InnerError {
+    InnerVariant,
+}
+
+#[derive(Debug, Snafu)]
+enum Error {
+    AVariant {
+        // Invalid identifier, and not a boolean; should error
+        #[snafu(source(5))]
+        a: InnerError,
+    },
+}
+
+fn main() {}
diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr
new file mode 100644
index 0000000..27d7c19
--- /dev/null
+++ b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr
@@ -0,0 +1,5 @@
+error: expected boolean literal or identifier
+  --> $DIR/attribute-unparseable.rs:12:24
+   |
+12 |         #[snafu(source(5))]
+   |                        ^
diff --git a/tests/multiple_attributes.rs b/tests/multiple_attributes.rs
index 000b273..44358f6 100644
--- a/tests/multiple_attributes.rs
+++ b/tests/multiple_attributes.rs
@@ -1,13 +1,25 @@
 extern crate snafu;
 
+use snafu::{ResultExt, Snafu};
+
+#[derive(Debug, Snafu)]
+enum InnerError {
+    InnerVariant,
+}
+
 mod error {
+    use super::InnerError;
     use snafu::Snafu;
 
     #[derive(Debug, Snafu)]
     pub(super) enum Error {
         // We'll test both of these attributes inside `#[snafu(...)]`
         #[snafu(visibility(pub(super)), display("Moo"))]
-        Alpha,
+        Alpha {
+            // Ensure we can have multiple field attributes as well
+            #[snafu(source, backtrace(delegate))]
+            cause: InnerError,
+        },
     }
 }
 
@@ -17,9 +29,13 @@ fn is_visible() {
     let _ = error::Alpha;
 }
 
+fn example() -> Result<u8, InnerError> {
+    InnerVariant.fail()
+}
+
 // Confirm `display("Moo")` is applied to the variant
 #[test]
 fn has_display() {
-    let err = error::Error::Alpha;
+    let err = example().context(error::Alpha).unwrap_err();
     assert_eq!(format!("{}", err), "Moo");
 }