Skip to content

Wrong closure scope class reported for static methods #8932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nicolas-grekas opened this issue Jul 6, 2022 · 1 comment
Closed

Wrong closure scope class reported for static methods #8932

nicolas-grekas opened this issue Jul 6, 2022 · 1 comment

Comments

@nicolas-grekas
Copy link
Contributor

nicolas-grekas commented Jul 6, 2022

Description

The following code:

class A
{
    public static function b()
    {
        echo static::class;
    }
}

class B extends A
{
}

$c = ['B', 'b'];

$d = \Closure::fromCallable($c);

$r = new \ReflectionFunction($d);

var_dump($r->getClosureScopeClass());

$d();

Resulted in this output:

object(ReflectionClass)#41 (1) {
  ["name"]=>
  string(1) "A"
}
B

But I expected this output instead:

object(ReflectionClass)#41 (1) {
  ["name"]=>
  string(1) "B"
}
B

Right now, there is no way to turn back a named closure into a regular callable because reflection doesn't give back the "called class" ("B" here.)

See code running at https://3v4l.org/OpEA9

PHP Version

PHP >= 7.1

Operating System

No response

@cmb69
Copy link
Member

cmb69 commented Jul 6, 2022

Yeah, that looks like a bug. The problem is that we're loosing the called_scope during construction of the ReflectionFunction, and it seems there is currently no way to retrieve it later.

cmb69 added a commit to cmb69/php-src that referenced this issue Jul 6, 2022
`ReflectionFunction::getClosureScopeClass()` needs to report the
`called_scope`, if there is one.

Unfortunately, it is not yet possible to retrieve that value from the
closure object, so we need to introduce a getter.
nicolas-grekas pushed a commit to nicolas-grekas/php-src that referenced this issue Aug 11, 2022
fabpot added a commit to twigphp/Twig that referenced this issue Aug 12, 2022
…s (bennothommo)

This PR was merged into the 2.x branch.

Discussion
----------

Allow inherited magic method to still run with calling class

This is #3719 ported to 2.x and improved a bit.

I also reported php/php-src#8932 because the underlying behavior of PHP is broken to me.

If a static method cannot be resolved to the calling class, but the calling class has, or inherits, a `__callStatic` handler, this allows the `__callStatic` handler to be used with the calling class, and not the inherited class as would occur with reflection. This allows systems such as Laravel facades to still work.

Fixes #3716

Commits
-------

d1457a4 Allow inherited magic method to still run with calling class
arnaud-lb added a commit that referenced this issue Sep 2, 2022
* PHP-8.0:
  [ci skip] NEWS
  Add tests
  Fix GH-8932: Provide a way to get the called-scope of closures (#9299)
arnaud-lb added a commit that referenced this issue Sep 2, 2022
* PHP-8.1:
  [ci skip] NEWS
  [ci skip] NEWS
  Add tests
  Fix GH-8932: Provide a way to get the called-scope of closures (#9299)
arnaud-lb added a commit that referenced this issue Sep 2, 2022
* PHP-8.2:
  [ci skip] NEWS
  [ci skip] NEWS
  [ci skip] NEWS
  Add tests
  Fix GH-8932: Provide a way to get the called-scope of closures (#9299)
fabpot added a commit to twigphp/Twig that referenced this issue Dec 13, 2022
This PR was merged into the 2.x branch.

Discussion
----------

Fix optimizing closures callbacks

Follows #3722 and php/php-src#8932

Uses a dedicated method added to PHP 8.1.11 (and 8.0.24, but I feel like there is no need to make the check too complex for an outdated version. We just need a marker to make the code simpler when we'll bump to PHP >= 8.1.11.)

Commits
-------

406b3e5 Fix optimizing closures callbacks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@nicolas-grekas @cmb69 and others