Skip to content

Strengthen lifetime tests to exercise non-trivial lifetime relationships in lifetime_conversion and lifetime_duration modules #62

@sourcery-ai

Description

@sourcery-ai

Summary

The current tests for lifetime-related derives/macros (notably in the lifetime_conversion and lifetime_duration modules) only use values that share the same lifetime. As a result, the tests do not actually exercise or validate the intended lifetime relationships encoded in the where-clauses (e.g., 'pass: 't, 'fail: 'e, 't: 'o, 'e: 'f, 'input: 't / 'input: 'e).

This issue tracks improving those tests to construct references with different lifetimes (including nested scopes and separate stack frames) so that:

  • Over-constrained derives (that accidentally extend lifetimes too far) are caught by the compiler, and
  • Under-constrained derives (that fail to enforce required lifetime relationships) are also caught.

Problem

In the existing lifetime_conversion and lifetime_duration tests:

  • All &i32 values used in assertions are created within a single scope per test.
  • This means all references effectively have the same lifetime.
  • Constraints like 'pass: 't, 'fail: 'e, 't: 'o, 'e: 'f, and 'input: 't/'input: 'e are only exercised in the degenerate “all lifetimes are equal” situation.

Because of that, the tests cannot:

  • Detect if a derive accidentally allows a reference to outlive its source, or
  • Verify that a derive correctly encodes relationships such as one lifetime strictly outliving another.

Desired Behavior

Tests should:

  1. Create references (okval, errval, etc.) with different, nested lifetimes so that one clearly outlives the other.
  2. Use helper functions in a separate stack frame to model realistic uses and force the compiler to check the lifetime constraints as encoded by the macros/derives.
  3. Wire these helpers to the actual conversion/result types used by the macros (e.g., Try / Try_ConvertResult or custom enums), not just trivial tuples.
  4. Store extracted references in outer-scope variables so the compiler detects any lifetime over-/under-extension.

Suggested Approach

Below is a sketch of the kind of tests that should be added to lifetime_conversion (and analogously to lifetime_duration). This is intentionally written in terms of simple tuples but should be adapted to use the real types in this crate.

mod lifetime_conversion {
    use super::*;

    // Helper functions to exercise lifetime relationships across stack frames.
    // Replace the return types with the real conversion/derive types used in
    // this module (e.g., Try / Try_ConvertResult / custom enums).
    fn convert_ok_err<'t, 'e>(ok: &'t i32, err: &'e i32) -> (&'t i32, &'e i32) {
        (ok, err)
    }

    fn convert_err_ok<'t, 'e>(ok: &'t i32, err: &'e i32) -> (&'e i32, &'t i32) {
        (err, ok)
    }

    #[test]
    fn ok_outlives_err_nested_scope() {
        // 'outer: lifetime of okval
        let ok_storage = 1;
        let okval = &ok_storage;

        // Bind in outer scope so any returned references must be valid for 'outer.
        let (out_ok, out_err): (&i32, &i32);

        {
            // 'inner: lifetime of errval, strictly shorter than 'outer
            let err_storage = 2;
            let errval = &err_storage;

            // This should reflect constraints like 't: 'e or 'input: 't, depending
            // on the actual conversion types.
            let (ok_res, err_res) = convert_ok_err(okval, errval);

            assert_eq!(*ok_res, 1);
            assert_eq!(*err_res, 2);

            out_ok = ok_res;
            out_err = err_res;
        }

        // out_ok must still be valid here (okval outlives errval).
        assert_eq!(*out_ok, 1);

        // out_err is only sound here if the conversion did NOT extend the
        // lifetime of errval beyond its scope. Once wired to the real types,
        // the compiler should reject any incorrect derive that over-constrains.
        assert_eq!(*out_err, 2);
    }

    #[test]
    fn err_outlives_ok_nested_scope() {
        // 'outer: lifetime of errval
        let err_storage = 10;
        let errval = &err_storage;

        let (out_ok, out_err): (&i32, &i32);

        {
            // 'inner: lifetime of okval, strictly shorter than 'outer
            let ok_storage = 20;
            let okval = &ok_storage;

            // Exercises the opposite direction of the lifetime relationship.
            let (err_res, ok_res) = convert_err_ok(okval, errval);

            assert_eq!(*ok_res, 20);
            assert_eq!(*err_res, 10);

            out_ok = ok_res;
            out_err = err_res;
        }

        // out_err must still be valid here (errval outlives okval).
        assert_eq!(*out_err, 10);

        // When wired to the real conversion types, any under-constrained derive
        // that fails to enforce 'fail: 'e-like relationships should be rejected.
        assert_eq!(*out_ok, 20);
    }
}
``

### Integration Steps
To fully exercise the actual macros/derives instead of simple tuples:
1. **Replace helper return types**
   - Change `convert_ok_err` / `convert_err_ok` to return the real conversion/result types used in `lifetime_conversion` (e.g., a type implementing `Try` or a custom enum with the `derive` under test).
   - Ensure these types preserve the `'t` and `'e` lifetimes in their generic parameters as intended.

2. **Construct real conversion values inside helpers**
   - In `convert_ok_err` and `convert_err_ok`, construct the actual conversion/derive types using `ok` and `err` and return those.

3. **Adjust assertions**
   - Instead of directly destructuring tuples, pattern-match on the real types or use accessor methods to extract the `ok`/`err` references.
   - Store the extracted references in `out_ok` / `out_err` (bound in the outer scope) so the compiler is forced to verify the lifetime constraints.

4. **Mirror for `lifetime_duration`**
   - Add analogous tests in the `lifetime_duration` module that:
     - Create `okval` and `errval` (or analogous values) in nested scopes with differing lifetimes.
     - Use helper functions in separate stack frames.
     - Use the real duration-related types/macros being tested there.

## Action Items
- [ ] Identify all lifetime-sensitive test modules (at least `lifetime_conversion` and `lifetime_duration`).
- [ ] Add nested-scope tests for the case where the `ok` value outlives the `err` value.
- [ ] Add nested-scope tests for the case where the `err` value outlives the `ok` value.
- [ ] Implement helper functions that:
  - Take references with explicit lifetime parameters (`'t`, `'e`, etc.).
  - Return the actual macro/derive-generated types that encode those lifetimes.
- [ ] Update assertions to operate on the real conversion/duration types, not tuples.
- [ ] Ensure the compiler catches incorrect lifetime constraints by verifying that the tests fail to compile (or require `compile_fail` tests) if derives are over- or under-constrained.

By implementing these changes, the test suite will validate the true lifetime behavior encoded by the macros/derives instead of only the trivial case where all references share the same lifetime.

---

I created this issue for @MusicalNinjaDad from https://github.com/MusicalNinjaDad/try_v2/pull/61#discussion_r3068136567.

<details>
<summary>Tips and commands</summary>

#### Getting Help

- [Contact our support team](mailto:support@sourcery.ai) for questions or feedback.
- Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information.
- Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).

</details>

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions