Repository: shepmaster/snafu (rust)

Issue:
Add a way to construct a leaf error without wrapping it in a Result
Right now, sometimes we end up with code like

```rust
Variant.fail::<()>.unwrap_err()
```

This usually occurs when interoperating with systems that already have a `Result`, such as when used in `Result::map_err`.

We could add a new function that just generates the error, not wrapping it.

---

Related but different from the comment in #46:

>  manually construct an error variant with a source error

Pull request:
Add a way to construct a leaf error without wrapping it in a Result
Closes #221

Patch:
diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs
index 83bf550..88664a5 100644
--- a/snafu-derive/src/lib.rs
+++ b/snafu-derive/src/lib.rs
@@ -1387,22 +1387,29 @@ impl<'a> quote::ToTokens for ContextSelector<'a> {
 
                 let inherent_impl = if source_field.is_none() {
                     quote! {
-                    impl<#(#generic_names,)*> #selector_name
-                    {
-                        #[doc = "Consume the selector and return a `Result` with the associated error"]
-                        #visibility fn fail<#(#original_generics_without_defaults,)* __T>(self) -> core::result::Result<__T, #parameterized_enum_name>
-                        where
-                            #(#where_clauses),*
-                        {
-                            let Self { #(#names),* } = self;
-                            let error = #enum_name::#variant_name {
-                                #backtrace_field
-                                #( #names: core::convert::Into::into(#names) ),*
-                            };
-                            core::result::Result::Err(error)
+                        impl<#(#generic_names,)*> #selector_name {
+                            #[doc = "Consume the selector and return the associated error"]
+                            #[must_use]
+                            #visibility fn build<#(#original_generics_without_defaults,)*>(self) -> #parameterized_enum_name
+                            where
+                                #(#where_clauses),*
+                            {
+                                let Self { #(#names),* } = self;
+                                #enum_name::#variant_name {
+                                    #backtrace_field
+                                    #( #names: core::convert::Into::into(#names) ),*
                                 }
                             }
+
+                            #[doc = "Consume the selector and return a `Result` with the associated error"]
+                            #visibility fn fail<#(#original_generics_without_defaults,)* __T>(self) -> core::result::Result<__T, #parameterized_enum_name>
+                            where
+                                #(#where_clauses),*
+                            {
+                                core::result::Result::Err(self.build())
+                            }
                         }
+                    }
                 } else {
                     quote! {}
                 };
diff --git a/src/guide/the_macro.md b/src/guide/the_macro.md
index 9afc7bd..0e80adb 100644
--- a/src/guide/the_macro.md
+++ b/src/guide/the_macro.md
@@ -68,21 +68,23 @@ where
     P: Into<PathBuf>,
 ```
 
-Otherwise, the context selector will have an inherent method
-`fail` and can be used with the [`ensure`](ensure) macro:
+Otherwise, the context selector will have the inherent methods `build`
+and `fail` and can be used with the [`ensure`](ensure) macro:
 
 ```rust,ignore
 impl<I> UserIdInvalid<I>
 where
     I: Into<i32>,
 {
+    fn build(self) -> Error { /* ... */ }
+
     fn fail<T>(self) -> Result<T, Error> { /* ... */ }
 }
 ```
 
 If the original variant had a `backtrace` field, the backtrace
 will be automatically constructed when either `IntoError` or
-`fail` are called.
+`build`/`fail` are called.
 
 #### `Error`
 


Test patch:
diff --git a/compatibility-tests/compile-fail/tests/ui/visibility.rs b/compatibility-tests/compile-fail/tests/ui/visibility.rs
index 9bb2d2c..595b06e 100644
--- a/compatibility-tests/compile-fail/tests/ui/visibility.rs
+++ b/compatibility-tests/compile-fail/tests/ui/visibility.rs
@@ -16,16 +16,16 @@ mod outer {
     }
 
     fn private_is_applied() {
-        let _ = self::inner::Private.fail::<()>();
+        let _ = self::inner::Private.build();
     }
 }
 
 fn pub_in_path_is_applied() {
-    let _ = self::outer::inner::PubInPath.fail::<()>();
+    let _ = self::outer::inner::PubInPath.build();
 }
 
 fn private_is_applied() {
-    let _ = self::outer::inner::Private.fail::<()>();
+    let _ = self::outer::inner::Private.build();
 }
 
 fn main() {}
diff --git a/compatibility-tests/compile-fail/tests/ui/visibility.stderr b/compatibility-tests/compile-fail/tests/ui/visibility.stderr
index 37609b0..9e71932 100644
--- a/compatibility-tests/compile-fail/tests/ui/visibility.stderr
+++ b/compatibility-tests/compile-fail/tests/ui/visibility.stderr
@@ -1,7 +1,7 @@
 error[E0603]: unit struct `Private` is private
   --> $DIR/visibility.rs:19:30
    |
-19 |         let _ = self::inner::Private.fail::<()>();
+19 |         let _ = self::inner::Private.build();
    |                              ^^^^^^^ this unit struct is private
    |
 note: the unit struct `Private` is defined here
@@ -13,7 +13,7 @@ note: the unit struct `Private` is defined here
 error[E0603]: unit struct `PubInPath` is private
   --> $DIR/visibility.rs:24:33
    |
-24 |     let _ = self::outer::inner::PubInPath.fail::<()>();
+24 |     let _ = self::outer::inner::PubInPath.build();
    |                                 ^^^^^^^^^ this unit struct is private
    |
 note: the unit struct `PubInPath` is defined here
@@ -25,7 +25,7 @@ note: the unit struct `PubInPath` is defined here
 error[E0603]: unit struct `Private` is private
   --> $DIR/visibility.rs:28:33
    |
-28 |     let _ = self::outer::inner::Private.fail::<()>();
+28 |     let _ = self::outer::inner::Private.build();
    |                                 ^^^^^^^ this unit struct is private
    |
 note: the unit struct `Private` is defined here
@@ -34,20 +34,20 @@ note: the unit struct `Private` is defined here
 7  |         #[derive(Debug, Snafu)]
    |                         ^^^^^
 
-error[E0624]: method `fail` is private
+error[E0624]: method `build` is private
   --> $DIR/visibility.rs:19:38
    |
-19 |         let _ = self::inner::Private.fail::<()>();
-   |                                      ^^^^
+19 |         let _ = self::inner::Private.build();
+   |                                      ^^^^^
 
-error[E0624]: method `fail` is private
+error[E0624]: method `build` is private
   --> $DIR/visibility.rs:24:43
    |
-24 |     let _ = self::outer::inner::PubInPath.fail::<()>();
-   |                                           ^^^^
+24 |     let _ = self::outer::inner::PubInPath.build();
+   |                                           ^^^^^
 
-error[E0624]: method `fail` is private
+error[E0624]: method `build` is private
   --> $DIR/visibility.rs:28:41
    |
-28 |     let _ = self::outer::inner::Private.fail::<()>();
-   |                                         ^^^^
+28 |     let _ = self::outer::inner::Private.build();
+   |                                         ^^^^^
diff --git a/tests/backtrace-optional-enabled.rs b/tests/backtrace-optional-enabled.rs
index 77b8f42..a1426cf 100644
--- a/tests/backtrace-optional-enabled.rs
+++ b/tests/backtrace-optional-enabled.rs
@@ -8,6 +8,6 @@ enum Error {
 #[test]
 fn optional_backtrace_is_present_with_environment_variable() {
     std::env::set_var("RUST_LIB_BACKTRACE", "1");
-    let sometimes = BacktraceSometimes.fail::<()>();
-    assert!(ErrorCompat::backtrace(&sometimes.unwrap_err()).is_some());
+    let sometimes = BacktraceSometimes.build();
+    assert!(ErrorCompat::backtrace(&sometimes).is_some());
 }
diff --git a/tests/backtrace-optional.rs b/tests/backtrace-optional.rs
index 5760fbf..c46fd55 100644
--- a/tests/backtrace-optional.rs
+++ b/tests/backtrace-optional.rs
@@ -8,12 +8,12 @@ enum Error {
 
 #[test]
 fn bare_backtrace_is_always_present() {
-    let always = BacktraceAlways.fail::<()>();
-    assert!(ErrorCompat::backtrace(&always.unwrap_err()).is_some());
+    let always = BacktraceAlways.build();
+    assert!(ErrorCompat::backtrace(&always).is_some());
 }
 
 #[test]
 fn optional_backtrace_is_not_present_without_environment_variable() {
-    let sometimes = BacktraceSometimes.fail::<()>();
-    assert!(ErrorCompat::backtrace(&sometimes.unwrap_err()).is_none());
+    let sometimes = BacktraceSometimes.build();
+    assert!(ErrorCompat::backtrace(&sometimes).is_none());
 }
diff --git a/tests/build-leaf-error.rs b/tests/build-leaf-error.rs
new file mode 100644
index 0000000..fe6930c
--- /dev/null
+++ b/tests/build-leaf-error.rs
@@ -0,0 +1,24 @@
+use snafu::Snafu;
+
+#[derive(Debug, Snafu)]
+enum Error {
+    Mine,
+}
+
+type Result<T, E = Error> = std::result::Result<T, E>;
+
+fn other_result() -> Result<i32, ()> {
+    Err(())
+}
+
+fn map_result() -> Result<i32> {
+    other_result().map_err(|_| Mine.build())
+}
+
+#[test]
+fn implements_error() {
+    fn check<T: std::error::Error>() {}
+    check::<Error>();
+
+    map_result().unwrap_err();
+}
diff --git a/tests/visibility.rs b/tests/visibility.rs
index a3daa5e..fe0eaf1 100644
--- a/tests/visibility.rs
+++ b/tests/visibility.rs
@@ -23,16 +23,16 @@ mod outer {
 
     #[test]
     fn can_set_default_visibility() {
-        let _ = self::inner::PubCrate { id: 42 }.fail::<()>();
+        let _ = self::inner::PubCrate { id: 42 }.build();
     }
 
     #[test]
     fn can_set_visibility() {
-        let _ = self::inner::PubInPath { id: 42 }.fail::<()>();
+        let _ = self::inner::PubInPath { id: 42 }.build();
     }
 }
 
 #[test]
 fn can_set_default_visibility() {
-    let _ = self::outer::inner::PubCrate { id: 42 }.fail::<()>();
+    let _ = self::outer::inner::PubCrate { id: 42 }.build();
 }