add unwrap method

[MusicalNinjaDad]

No description provided.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The generated unwrap currently panics with an empty panic!(); consider emitting a more informative message (similar to Result::unwrap) and, if necessary, extending the derive’s bounds (e.g., Debug) so you can include the enum and residual value in the panic message.
• Since impl_try_methods only uses name, output_variant_name, and output_type, you could avoid binding the other tuple elements from split_for_impl() (or add targeted #[allow(unused)] attributes) to reduce the need for the broad #[allow(unused_variables)] on the whole tuple.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The generated `unwrap` currently panics with an empty `panic!()`; consider emitting a more informative message (similar to `Result::unwrap`) and, if necessary, extending the derive’s bounds (e.g., `Debug`) so you can include the enum and residual value in the panic message.
- Since `impl_try_methods` only uses `name`, `output_variant_name`, and `output_type`, you could avoid binding the other tuple elements from `split_for_impl()` (or add targeted `#[allow(unused)]` attributes) to reduce the need for the broad `#[allow(unused_variables)]` on the whole tuple.

## Individual Comments

### Comment 1
<location path="src/lib.rs" line_range="477" />
<code_context>
+        where_clause,
+    ) = tryenum.split_for_impl();
+
+    // TODO: Better error message: needs where ... Debug
+    let impl_extraction = quote! {
+        impl #impl_generics #name #ty_generics #where_clause {
</code_context>
<issue_to_address>
**issue:** The TODO about `Debug` bounds seems stale relative to the current implementation.

`unwrap` doesn’t use `Debug`, so this TODO is now misleading. Please either remove/update it or add a `Debug`-based error message and extend `#where_clause` to include the appropriate bound.
</issue_to_address>

### Comment 2
<location path="tests/test_usage.rs" line_range="448-452" />
<code_context>
+        ValidationFailed(E),
+    }
+
+    #[test]
+    fn unwrap() {
+        let x: Validated<_, String> = Validated::Valid(2);
+        assert_eq!(x.unwrap(), 2);
+    }
+}
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests that verify `unwrap` panics for non-output variants to fully validate the derived method behavior.

The current `methods::unwrap` test only checks `Validated::Valid`. Since the derive-generated `unwrap` should panic for all non-output variants, please also add `#[should_panic]` tests for `Validated::Invalid` and `Validated::ValidationFailed` to cover those branches and guard against regressions in the pattern matching.

```suggestion
    #[test]
    fn unwrap() {
        let x: Validated<_, String> = Validated::Valid(2);
        assert_eq!(x.unwrap(), 2);
    }

    #[test]
    #[should_panic]
    fn unwrap_panics_on_invalid() {
        let x: Validated<i32, String> = Validated::Invalid;
        // This should panic because `Invalid` is a non-output variant
        let _ = x.unwrap();
    }

    #[test]
    #[should_panic]
    fn unwrap_panics_on_validation_failed() {
        let x: Validated<i32, String> = Validated::ValidationFailed("error".to_owned());
        // This should panic because `ValidationFailed` is a non-output variant
        let _ = x.unwrap();
    }
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[MusicalNinjaDad]

@sourcery-ai review

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The Try_Methods derive unconditionally adds a Debug bound to all type parameters, which can silently tighten generic constraints and break existing users; consider restricting this to only the type parameters actually used in the panic message or gating the derive behind an explicit Debug requirement in the user’s definition.
• The unwrap_panic_unit test’s #[should_panic(expected = ...)] string does not match the actual variant (Invalid) that will be printed by Debug, so the expectation should be updated to reflect the real panic message.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `Try_Methods` derive unconditionally adds a `Debug` bound to all type parameters, which can silently tighten generic constraints and break existing users; consider restricting this to only the type parameters actually used in the panic message or gating the derive behind an explicit `Debug` requirement in the user’s definition.
- The `unwrap_panic_unit` test’s `#[should_panic(expected = ...)]` string does not match the actual variant (`Invalid`) that will be printed by `Debug`, so the expectation should be updated to reflect the real panic message.

## Individual Comments

### Comment 1
<location path="tests/test_usage.rs" line_range="455" />
<code_context>
+    }
+
+    #[test]
+    #[should_panic(expected = "called `unwrap()` on a short-circuiting value: ValidationFailed(2)")]
+    fn unwrap_panic_fields() {
+        let x: Validated<i32, _> = Validated::ValidationFailed(2);
</code_context>
<issue_to_address>
**issue (testing):** The expected panic message for the `unwrap_panic_unit` test does not match the actual variant being used and will not reflect the real panic output.

In `unwrap_panic_unit`, the enum value is `Validated::Invalid`, so `Debug` formatting will produce something like `Invalid`, not `ValidationFailed(2)`. The current `#[should_panic(expected = ...)]` string looks copied from `unwrap_panic_fields` and won’t match the real panic. Please update the expected substring to reflect the `Invalid` variant’s `Debug` output (e.g. `"called `unwrap()` on a short-circuiting value: Invalid"`).
</issue_to_address>

### Comment 2
<location path="src/lib.rs" line_range="461" />
<code_context>
+    impl_try_methods(input.into()).into()
+}
+
+fn impl_try_methods(input: TokenStream2) -> DiagnosticStream {
+    let ast: DeriveInput = syn::parse2(input).expect("derive macro");
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying `impl_try_methods` by avoiding custom generic-bound manipulation and instead adding a `Debug` bound directly on `unwrap` (or its impl).

You can drop the extra generics machinery and separate `impl` entirely by putting a `Debug` bound directly on the method, reusing the existing `split_for_impl()` output.

This removes:

- `TypeParamBound`
- `tryenum.generics` callback
- `debug_generics.split_for_impl()` and the second impl block

while keeping the same behavior of `unwrap` (it still only compiles when `self` is `Debug`-printable for the panic).

Suggested change:

```rust
fn impl_try_methods(input: TokenStream2) -> DiagnosticStream {
    let ast: DeriveInput = syn::parse2(input).expect("derive macro");

    let tryenum = TryEnum::parse(&ast)?;
    let (
        name,
        output_variant_name,
        output_type,
        residual_type_name,
        residual_type,
        impl_generics,
        ty_generics,
        where_clause,
    ) = tryenum.split_for_impl();

    let impl_extraction = quote! {
        impl #impl_generics #name #ty_generics #where_clause {
            pub fn unwrap(self) -> #output_type
            where
                Self: std::fmt::Debug,
            {
                let #name::#output_variant_name(val) = self else {
                    panic!("called `unwrap()` on a short-circuiting value: {:?}", self);
                };
                val
            }
        }
    };

    Ok(impl_extraction)
}
```

If you prefer the bound at the `impl` level instead of per-method:

```rust
let impl_extraction = quote! {
    impl #impl_generics #name #ty_generics
    where
        #where_clause
        Self: std::fmt::Debug,
    {
        pub fn unwrap(self) -> #output_type {
            let #name::#output_variant_name(val) = self else {
                panic!("called `unwrap()` on a short-circuiting value: {:?}", self);
            };
            val
        }
    }
};
```

(You’d need to merge `#where_clause` carefully if it is `Option<WhereClause>`.)
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}