Closure::bindTo Throwable instead of warning when passing object other class

[LordFireen]

Description:

Currently, when binding a new object to Closure, which is an object of a different class, a "warning" is logged in the log, but the method itself has no effect, which can be confusing.

It seems to me better to add Throwable in such a scenario, it will make it easier to handle the case.

The following code:

<?php

class SomeClass
{
    public function testMethod() {
        echo 123;
    }
}

class OtherClass
{
    public function testMethod() {
        echo 234;
    }
}

$a = new SomeClass();
$b = new OtherClass();

$closure = Closure::fromCallable([$a, 'testMethod']);

try {
    $closure->bindTo($b);
} catch (Throwable $e) {
    echo 'Oh, sorry u cannot do it!';
    return;
}

$closure();

Resulted:

Warning: Cannot bind method SomeClass::testMethod() to object of class OtherClass in /in/VgKd4 on line 23
123

Expected:

Oh, sorry u cannot do it!

Workaround:

As it stands, to be able to handle such a scenario you need to use Reflection as follows:

$reflection = new ReflectionFunction($closure);
$oldThis = $reflection->getClosureThis();
if ($oldThis && $oldThis::class === $b::class) {
    $closure->bindTo($b);
} else {
    echo 'Oh, sorry u cannot do it!';
}

Of course, this code can be simplified, but a simple try/catch is a much better solution.

Especially since I assume that the parser will check again if the object is of the same class when executes bindTo()

PHP Version

PHP 8.2.6

Operating System

Linux

[Girgias]

Thank you for pointing this out, at best this can only be changed in the next minor version (8.3) and I don't know if this would require an RFC.

Could you start a discussion on the PHP Internals list to see if people agree with this change without the need for an RFC, or if one should be done. Considering it takes at minimum 1 month to go through the whole RFC process. Moreover, feature freeze is usually around the end of July, so starting sooner than later on the process is better.

[iluuu1994]

Well, to be fair, this code is broken anyway.

https://3v4l.org/pTDvs

bindTo creates a new closure, so it needs to be assigned to something, or called immediately to have any effect. Moreover, it may return NULL in the case of failure, which may be checked and handled accordingly. I still think that an exception would be better as this is likely a programmer error.

I don't have any strong feelings, a mail to the mailing list might suffice to change this if nobody objects, although we should provide a migration path by first generating a deprecation notice and moving to exceptions in PHP 9.0.

https://www.php.net/mailing-lists.php (Internals list)

[DanielEScherzer]

Added to https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_closure_binding_issues