DynamicFunctionReturnTypeExtension / DynamicFunctionThrowTypeExtension not executed for anonymous functions

[olsavmic]

Feature request

Since PHPStan does not provide FunctionReflection for anonymous functions, DynamicFunctionThrowTypeExtension / DynamicFunctionReturnTypeExtension are not applied.

This limits the possibilities for rules like forbidCheckedExceptionInCallable (shipmonk-rnd/phpstan-rules#210).

• In short, this rule is exposing @throw clauses from the anonymous function to the "outside" to the point where the function is invoked, thus improving the PHPStan-native checked exceptions support.

Given that ReflectionFunction is defined for anonymous functions (https://www.php.net/manual/en/reflectionfunction.isanonymous.php) and all the type information is available during the PHPStan runtime (as part of the analyzed AST), I believe it would make sense to provide FunctionReflection for anonymous functions, making dynamic extensions working.

I understand though this is pretty much an edge case :)

Did PHPStan help you today? Did it make you happy in any way?

Thank you for your time and support of this amazing tool, making PHP better every day :)

[ondrejmirtes]

What I think would be a much better solution: to inform PHPStan about everything about closures passed into parameters. Currently we don't know:

• If the closure gets execute immediately or later
• How many times the closure gets executed

We could have something like @phpstan-closure-param-invoke-immediate $cb and @phpstan-closure-param-invoke-count $cb PHPDocs. That would allow the analysis to be informed more precisely about closures passed into functions.

Because the closure "itself" doesn't throw exceptions. Only functions/methods called from its body. And it depends when the body is actually called.

[olsavmic]

I definitely like the proposal! It would cover the whole functionality of the rule we're using now + it would have better DX.

Anyway though, the issue was not meant to be about the rule itself. I'm sorry for not making myself clear :)
I'll try to describe the issue better below:

Context

The main limitation of the current solution (but also the proposed solution) is the fact that closures do not support @throws declarations.

Not only the annotation is not supported (which is a different issue, I don't want to propose a syntax for that), but mainly the throw types are not inferred automatically.

What we're doing in the ImmediatelyCalledCallableThrowTypeExtension is that we traverse the inner nodes and extract the throwTypes - it works and it could for sure be automated.

Request

From the quick check of the phpstan codebase regarding this issue, it seems like if the FunctionReflection was provided for Closures, the auto-infering of throws would start working out of the box.

Originally, I though that we'd still need to register a DynamicFunctionThrowTypeExtension but after second look, it seems like it would not be needed.

If FunctionReflection with throwTypes is provided, then the whole throwPoints functionality is going to work out-of-the box and our custom rule for ImmediatelyCalledCallableThrowTypeExtension will get to 1/3 the size and will be more reliable :)

---

TLDR;

We need to solve this:

$result = (static fn ($val) => throw new CheckedException($val))(); // PHPStan should know it throws!

$fun = (static fn ($val) => throw new CheckedException($val))(); // $fun should know that it throws!

$fun(); // should throw here, it's the exact same type!

function throwingMatch() {
    /** @throws CheckedException */ // This should not be needed but it is now as PHPStan is not able to tell that the match may throw

    return match ($format) {
        ShippingLabelFormat::Zpl => ...,
        ShippingLabelFormat::Pdf => (function () use ($format): LabelResult {
            ...        
            throw new CheckedException();
            
            })(),
        };
}

before the following (proposed) can work properly:

/**
 * @phpstan-closure-param-invoke-immediate $cb
 * @param callable(): void $cb
 */
function map(callable $cb) {...}

[ondrejmirtes]

The main issue here is that in $fun(); this gets resolved in Type::getCallableParametersAcceptors(): ParametersAcceptor[].

But ParametersAcceptor does not contain information about throwing exceptions, that's only on FunctionReflection and MethodReflection.

We could add it on ParametersAcceptorWithPhpDocs (it's a bad name, it should be called ExtendedParametersAcceptor).

[ondrejmirtes]

i figured out a way how to do this in PHPStan API pretty nicely, because during the work on impure points (which enable checks like this one https://phpstan.org/r/a57ed38d-2412-4d58-ab2a-084c1e7f8fc4) I realized I'm gonna need them for closures/callables as well. And it's suitable for throw points as well! :)

So the idea is to narrow the type of Type::getCallableParametersAcceptors() to allow passing additional information like throw points and impure points: phpstan/phpstan-src@7b2bd09

Of course this is just the initial commit, we have to add methods on CallableParametersAcceptor to make it useful.

Another idea I have - for functions we can assume that callable passed as an argument is always immediately called. I think it's a reasonable default, otherwise the function does something nasty like saving the callback in a global state.

For methods we shouldn't assume that, it's very common an object saves a callback to be called later.

/cc @janedbal

[ondrejmirtes]

Alright, with this commit phpstan/phpstan-src@74c02de you'll no longer need to configure functions in immediatelyCalledCallables.

So now the plan is to add @param-immediately-invoked-callable $a for methods (opt-in), and @param-later-invoked-callable $a for functions (opt-out).

Comments welcome :)

[janedbal]

I'd like to test it, will that be part of 1.10.x?

[ondrejmirtes]

No, part of 1.11.x. You can still test it if you simply install 1.11.x-dev :)

[janedbal]

I'd say the underlying problem is now solved, so maybe close this?

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.