Skip to content

ReflectionFunction::getNamespaceName() is still broken for closures in PHP 8.4 #18070

Closed as not planned

Description

@jlherren

Description

The following code:

<?php
namespace TestNamespace;
$ref = new \ReflectionFunction(function (Other\Factory $factory) {
});
echo $ref->getNamespaceName(), "\n";

Resulted in this output:


But I expected this output instead (as output by PHP 8.1, 8.2 and 8.3):

TestNamespace

Also see possibly related #16122.

PHP Version

PHP 8.4.5

Operating System

Official docker image php:8.4-cli

Activity

  1. iluuu1994 commented on Mar 14, 2025

    @iluuu1994
    Member

    Hi @jlherren. This isn't really a bug. Namespaces in PHP are really just prefixes to class/function/constant names. Since closures are anonymous, they don't have a meaningful name, and thus don't have a prefix that is needed for context/disambiguation.

    Nonetheless, in what context would this be relevant to you?

  2. jlherren commented on Mar 14, 2025

    @jlherren
    Author

    I see how it makes sense that closures don't really have a namespace. But it's relevant because it breaks code that relied on the previous behavior present all the way back to PHP 5.3. If you say that the previous behavior was unintended or unsupported, then it may not be classified as a bug, but it still represents a rather unfortunate backward compatibility break.

    One thing it breaks is laravel/serializable-closure, see laravel/serializable-closure#111 which in turn breaks php-di/php-di compiled containers for me.

  3. iluuu1994 commented on Mar 14, 2025

    @iluuu1994
    Member

    @jlherren Oh, I missed that the behavior was changed in 8.4.

    @TimWolla This is caused by #13550. It explicitly dropped the namespace prefix, but I don't think we sufficiently thought about the consequences.

  4. TimWolla commented on Mar 14, 2025

    @TimWolla
    Member

    but I don't think we sufficiently thought about the consequences.

    Indeed. It's an unfortunate and unanticipated backwards compatibility break - exactly for the reason that closures do not have a meaningful name by definition.

    But I'm not sure if this is fixable now that PHP 8.4 is released and folks already adapted their code to support the closure naming format.

    laravel/serializable-closure already uses the tokenizer to parse the file and look up the use statements, adding support for parsing the namespace should be a small incremental change. Generally I would expect any tool using the tokenizer to require adjustments with every new PHP version to correctly understand syntax and semantics and to me this change is no different.

  5. TimWolla commented on Mar 16, 2025

    @TimWolla
    Member

    Generally I would expect any tool using the tokenizer to require adjustments with every new PHP version to correctly understand syntax and semantics and to me this change is no different.

    To give an example: The library also fails to correctly serialize property hooks. This example:

    <?php
    
    use Laravel\SerializableClosure\SerializableClosure;
    
    require_once __DIR__ . '/vendor/autoload.php';
    
    $closure = function () {
    	return new class {
    		public string $foo {
    			get {
    				return 'test';
    			}
    		}
    	};
    };
    
    $serialized = serialize(new SerializableClosure($closure));
    
    echo "Serialized: $serialized\n";
    
    $closure = unserialize($serialized)->getClosure();
    
    echo 'Received: ', $closure(), "\n";

    results in:

    Serialized: O:47:"Laravel\SerializableClosure\SerializableClosure":1:{s:12:"serializable";O:46:"Laravel\SerializableClosure\Serializers\Native":5:{s:3:"use";a:0:{}s:8:"function";s:100:"function () {
            return new class {
                    public string $foo {
                            \get {
                                    return 'test';
                            }
                    }
            };
    }";s:5:"scope";N;s:4:"this";N;s:4:"self";s:32:"00000000000000020000000000000000";}}
    
    Parse error: syntax error, unexpected fully qualified name "\get", expecting identifier in laravel-serializable-closure://function () {
            return new class {
                    public string $foo {
                            \get {
                                    return 'test';
                            }
                    }
            };
    } on line 5
    

    … adding an extra \ in front of the get. So the only safe solution for tools using the tokenizer is to explicitly add support for new PHP versions after verifying compatibility.


    The fact that we are already at PHP 8.4.5 and other libraries already adapted their code to the new closure naming format of PHP 8.4, changing this in the PHP 8.4.x cycle would result in a(nother) breaking change. Combined with the fact that this issue only affects libraries that try to do relative symbol lookups, which already need the tokenizer to find out about use statements, adding namespace extraction support to the tokenization logic (together with support for all the other PHP 8.4 syntax) should overall be the less impactful change.

    I'm therefore closing this issue as a “Won't fix”.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions