Repository: shepmaster/snafu (rust)

Issue:
`source(from)` could imply `source(true)`
In #109, when considering the field attributes that can work together, there was some agreement that `source(true)` and `source(from)` make sense together on a field.  `source(true)` indicates that a field is the source even though it can't be named "source".  `source(from(...))` tells Snafu how to transform another type into the type indicated in the field.  (See https://docs.rs/snafu/0.4.3/snafu/guide/attributes/index.html#transforming-the-source).  There's no conflict in these intentions.

Moreover, if you specify `source(from)` on a field, you almost certainly mean for that to imply `source(true)` - you've already said "source" once, and it's a conflict to specify `source(false)` and `source(from)`.  It's not the case today, though - the `source(from)` attribute would be ignored if there weren't some other signal that the field is a source.

It'd reduce line count and cognitive overhead to have `source(from)` imply that the field is a source.  `source(true)` on the same field would still be valid, just redundant.

Pull request description:
Make `source(from)` imply `source(true)`
Fixes #124 - based on discussion from #109 about which attributes should be valid together.

This change makes `source(from(...))` imply `source(true)`.  This means you don't need two `source`-type attributes on a field that you intend to be a source but can't be named `source`.  (It's still valid to specify `source(from(...))` and `source(true)`, just redundant.)

The `attributes` section of the user's guide is updated with a note on this behavior.

---

**Testing done:**

Updated compile-fail results show that we catch more duplicates than we could before, because more fields are recognized as sources.

An updated unit test also shows this behavior in a more positive light.  :)

Patch:
diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs
index f4919fc..5dd3af4 100644
--- a/snafu-derive/src/lib.rs
+++ b/snafu-derive/src/lib.rs
@@ -508,6 +508,7 @@ fn parse_snafu_enum(
                                         }
                                         source_attrs.add((), name.span());
                                         transformations.add((t, e), name.span());
+                                        is_source = Some(true);
                                     }
                                 }
                             }
diff --git a/src/guide/attributes.md b/src/guide/attributes.md
index 64ed15f..42d143b 100644
--- a/src/guide/attributes.md
+++ b/src/guide/attributes.md
@@ -141,6 +141,11 @@ enum Error {
 struct ApiError(Box<Error>);
 ```
 
+Note: If you specify `#[snafu(source(from(...)))]` then the field
+will be treated as a source, even if it's not named "source" - in
+other words, `#[snafu(source(from(...)))]` implies
+`#[snafu(source)]`.
+
 ## Controlling backtraces
 
 If your error enum variant contains a backtrace but the field


Test patch:
diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr b/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr
index 2e217c0..1ae1b87 100644
--- a/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr
+++ b/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr
@@ -16,6 +16,12 @@ error: Multiple 'backtrace' attributes are not supported on one field; found a d
 34 |             backtrace3: String,
    |             ^^^^^^^^^^
 
+error: Multiple 'source' attributes are not supported within an error variant; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:18:13
+   |
+18 |             source2: Box<EnumError>,
+   |             ^^^^^^^
+
 error: Multiple 'source' attributes are not supported within an error variant; found a duplicate here:
   --> $DIR/attribute-duplication.rs:21:13
    |
@@ -40,6 +46,12 @@ error: Multiple 'backtrace' attributes are not supported within an error variant
 34 |             backtrace3: String,
    |             ^^^^^^^^^^
 
+error: Multiple 'backtrace(delegate)' attributes are not supported within an error variant; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:18:13
+   |
+18 |             source2: Box<EnumError>,
+   |             ^^^^^^^
+
 error: Multiple 'backtrace(delegate)' attributes are not supported within an error variant; found a duplicate here:
   --> $DIR/attribute-duplication.rs:21:13
    |
diff --git a/tests/source_attributes.rs b/tests/source_attributes.rs
index 1199602..79306b4 100644
--- a/tests/source_attributes.rs
+++ b/tests/source_attributes.rs
@@ -27,6 +27,11 @@ mod enabling {
             cause: InnerError,
         },
 
+        FromImpliesTrue {
+            #[snafu(source(from(InnerError, Box::new)))]
+            cause: Box<InnerError>,
+        },
+
         ExplicitFalse {
             #[snafu(source(false))]
             source: i32,
@@ -36,6 +41,7 @@ mod enabling {
     fn example() -> Result<(), Error> {
         inner().context(NoArgument)?;
         inner().context(ExplicitTrue)?;
+        inner().context(FromImpliesTrue)?;
         ExplicitFalse { source: 42 }.fail()?;
         Ok(())
     }