Skip to content

Conversation

@joelbutcher
Copy link
Collaborator

This PR makes the E passed to the Result PHP return doc for the attempt helper function a generic of type Throwable, allowing for improved PHPStan reporting with functions that already return typed Results in PHP doc:

  * @return Result<MyObj, CustomThrowable>

This will fix handling match or orElse calls where strongly typing the known exception type in the passed closure is preferred over fn(Throwable $t) => handler(...)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modifies the attempt helper function to make its error type parameter E generic (bounded by Throwable) instead of fixed to Throwable. This change aims to allow developers to specify more precise exception types in their return type annotations, enabling better type inference in downstream code when using methods like match or orElse.

Key changes:

  • Added @template E of Throwable to the attempt function's PHPDoc
  • Changed return type annotation from Result<T, Throwable> to Result<T, E>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 33 to 35
* @template E of Throwable
* @param callable(): T $f
* @return Result<T, Throwable>
* @return Result<T, E>
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of @template E of Throwable with return type Result<T, E> introduces a type soundness concern. Since PHPStan cannot infer E from the callable parameter (which has no exception specification), and the function body catches all Throwable exceptions, this allows potentially incorrect type narrowing.

For example:

/** @return Result<int, RuntimeException> */
function foo(): Result {
    // Type checks, but could throw LogicException
    return attempt(fn() => throw new LogicException());
}

While this pattern allows developers to manually specify exception types they know their code throws (via type hints or annotations), it shifts the responsibility for correctness from the type system to the developer.

Consider adding a documentation comment explaining that developers must ensure their callable only throws exception types compatible with their declared return type, or consider keeping the original @return Result<T, Throwable> for guaranteed type safety.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants