Checking that Closure is static

php.internals

Дмитрий Елисеев

5 years ago
Hello! When I use a static anonymous function as an HTTP-action in some frameworks like: $app = new App(); $app->get('/', static function () { return new Response('Hello!); }) I get a warning "Cannot bind an instance to a static closure" in every $closure->bind($container) call inside of the framework. But right now I cannot prevent this warning because there is no native way to check that closure is static before calling the bind method. I can only suppress the warning with @ or set/restore_error_handler pair. So I propose to add a method like Closure::isStatic (or Closure::canBeBound, etc.) for ability of manual checking it before bind call: if (!$closure->isStatic()) { $closure->bind($object); } The method can contain simple statement: return func->common.fn_flags & ZEND_ACC_STATIC; It will be helpful if somebody needs to support dynamic and static callables both without warnings. -- Best, Dmitry

Дмитрий Елисеев

5 years ago
18 june 2021 at 17:35, Dmitry Eliseev <elisdnru@gmail.com>:
> > Hello! When I use a static anonymous function as an HTTP-action in some frameworks like: > > $app = new App(); > $app->get('/', static function () { > return new Response('Hello!); > }) > > I get a warning "Cannot bind an instance to a static closure" in every $closure->bind($container) call inside of the framework. > > But right now I cannot prevent this warning because there is no native way to check that closure is static before calling the bind method. I can only suppress the warning with @ or set/restore_error_handler pair. > > So I propose to add a method like Closure::isStatic for ability of manual checking it before bind call: > > if (!$closure->isStatic()) { > $closure->bind($object); > } > > The method can contain simple statement: > > return func->common.fn_flags & ZEND_ACC_STATIC; > > It will be helpful if somebody needs to support dynamic and static callables both without warnings.
I sent Pull Request https://github.com/php/php-src/pull/7193 with a simple implementation. P.S. Sorry for PR without RFC, but I have not got any reply to my proposal yet.

Claude Pache

5 years ago
> Le 18 juin 2021 à 16:35, Дмитрий Елисеев <elisdnru@gmail.com> a écrit : > > Hello! When I use a static anonymous function as an HTTP-action in > some frameworks like: > > $app = new App(); > $app->get('/', static function () { > return new Response('Hello!); > }) > > I get a warning "Cannot bind an instance to a static closure" in every > $closure->bind($container) call inside of the framework. > > But right now I cannot prevent this warning because there is no native > way to check that closure is static before calling the bind method. I > can only suppress the warning with @ or set/restore_error_handler > pair. > > So I propose to add a method like Closure::isStatic (or > Closure::canBeBound, etc.) for ability of manual checking it before > bind call: > > if (!$closure->isStatic()) { > $closure->bind($object); > } > > The method can contain simple statement: > > return func->common.fn_flags & ZEND_ACC_STATIC; > > It will be helpful if somebody needs to support dynamic and static > callables both without warnings. > > -- Best, Dmitry
I think that the Warning should be removed entirely, and the `bindTo()` method should be a no-op when applied to a static closure. That the closure is implemented as static, or as non-static without actually using `$this`, is an implementation detail that the user of the closure doesn’t need to be aware. As a precedent, it is perfectly legal to call a static class method on an instance of that class: https://3v4l.org/5QKsN <https://3v4l.org/5QKsN> —Claude

Дмитрий Елисеев

5 years ago
чт, 24 июн. 2021 г. в 17:38, Claude Pache <claude.pache@gmail.com>:
> Le 18 juin 2021 à 16:35, Дмитрий Елисеев <elisdnru@gmail.com> a écrit : > > Hello! When I use a static anonymous function as an HTTP-action in > some frameworks like: > > $app = new App(); > $app->get('/', static function () { > return new Response('Hello!); > }) > > I get a warning "Cannot bind an instance to a static closure" in every > $closure->bind($container) call inside of the framework. > > But right now I cannot prevent this warning because there is no native > way to check that closure is static before calling the bind method. I > can only suppress the warning with @ or set/restore_error_handler > pair. > > So I propose to add a method like Closure::isStatic (or > Closure::canBeBound, etc.) for ability of manual checking it before > bind call: > > if (!$closure->isStatic()) { > $closure->bind($object); > } > > The method can contain simple statement: > > return func->common.fn_flags & ZEND_ACC_STATIC; > > It will be helpful if somebody needs to support dynamic and static > callables both without warnings. > > I sent Pull Request https://github.com/php/php-src/pull/7193 with a > simple implementation. > > -- Best, Dmitry > > > I think that the Warning should be removed entirely, and the `bindTo()` method should be a no-op when applied to a static closure. > > That the closure is implemented as static, or as non-static without actually using `$this`, is an implementation detail that the user of the closure doesn’t need to be aware. > > As a precedent, it is perfectly legal to call a static class method on an instance of that class: https://3v4l.org/5QKsN
Yes, removing this warning can be a better solution because bindTo() already returns FALSE on failure. But the bindTo() method is more general by design. It can be used for binding/unbinding/associating closures and it triggers different warnings in each case. So without warnings we lose information about the concrete reason for failure. And in quiet no-op mode without catching and logging these warnings we can miss real problems with incorrect binding in source code of our projects. Throwing an exception is a strict solution, but it is a BC-breaking change for the next major release.