Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115120
MIME-Version: 1.0
References: <CAMSJsBDdfnbge_digZVPfUme2y1V-XGmstUzpTmk9zJ1R7fdRQ@mail.gmail.com>
 <81EDE991-60C8-4DF8-9D1E-1B2BAE144829@gmail.com>
In-Reply-To: <81EDE991-60C8-4DF8-9D1E-1B2BAE144829@gmail.com>
Date: Thu, 24 Jun 2021 18:33:19 +0300
Message-ID: <CAMSJsBCyxhpgou2orQSptVdWtpQjFttq244uP+Exfv35aofxMQ@mail.gmail.com>
To: Claude Pache <claude.pache@gmail.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Checking that Closure is static
From: elisdnru@gmail.com (Dmitry Eliseev)

=D1=87=D1=82, 24 =D0=B8=D1=8E=D0=BD. 2021 =D0=B3. =D0=B2 17:38, Claude Pach=
e <claude.pache@gmail.com>:
> Le 18 juin 2021 =C3=A0 16:35, =D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9 =
=D0=95=D0=BB=D0=B8=D1=81=D0=B5=D0=B5=D0=B2 <elisdnru@gmail.com> a =C3=A9cri=
t :
>
> Hello! When I use a static anonymous function as an HTTP-action in
> some frameworks like:
>
> $app =3D 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()` m=
ethod should be a no-op when applied to a static closure.
>
> That the closure is implemented as static, or as non-static without actua=
lly using `$this`, is an implementation detail that the user of the closure=
 doesn=E2=80=99t 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.