Skip function-like and anonymous class nodes when checking if a function or method is a generator

[sebkehr]

Fixes: phpstan/phpstan#12462.

As is, when checking if a function (or method) is a generator (via PhpFunctionFromParserNodeReflection::isGenerator()) all subnodes are recursively checked for containment of a yield expression. Hence any function or method itself returning a yielding closure or arrow function is falsely determined to be a generator which in turn leads to incorrect return type checks as demonstrated in https://phpstan.org/r/5970e2e2-0aad-4927-8938-3f7d83e73a95.

The proposed fix therefore simply skips FunctionLike nodes in the above mentiond recursive check.

[ondrejmirtes]

Please also submit the same fix to https://github.com/Roave/BetterReflection: https://github.com/Roave/BetterReflection/blob/d262672652b9b70a54246adadf340587b535f549/src/Reflection/ReflectionFunctionAbstract.php#L349-L380

[ondrejmirtes]

Would be great to also not descend into anonymous classes.

[sebkehr]

@ondrejmirtes Anonymous class nodes should now be skipped as well. I submitted the fix via Roave/BetterReflection#1479.

[ondrejmirtes]

I just realized there's one more place that needs fixing. Check out CleaningParserTest. The job of CleaningParser is to erase function bodies of not-analyzed code (like in vendor/) before the AST is cached into memory.

I worry the code is faulty and it might do something like this:

Before:

function foo() {
    $fn = fn () => yield;
};

After:

function foo() {
    yield;
};

Changing function foo() from non-generator to a generator.

Thank you!

[sebkehr]

I just realized there's one more place that needs fixing. Check out CleaningParserTest. The job of CleaningParser is to erase function bodies of not-analyzed code (like in vendor/) before the AST is cached into memory.

Ah ok, I'll look into it.

[sebkehr]

@ondrejmirtes

I added two more examples to tests/PHPStan/Parser/data/cleaning-1-before.php. Please also have a look on the expected outcome after cleaning in tests/PHPStan/Parser/data/cleaning-1-after.php.

With the proposed changes to src/Parser/CleaningVisitor.php the closure in ContainsClosure::doFoo() will now be erased not leading to any left-over yield statements just like the yielding array-function in ContainsClosure::doBar(). Is this the intended behavior?

I also added Baz::someGenerator3() containing yield as a subexpression which will now lead to a yield expression after cleaning.