You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ReflectionFunction::getNamespaceName() is still broken for closures in PHP 8.4 #18070
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?
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.
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.
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:
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”.
Activity
iluuu1994 commentedon Mar 14, 2025
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?
jlherren commentedon Mar 14, 2025
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 breaksphp-di/php-dicompiled containers for me.iluuu1994 commentedon Mar 14, 2025
@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.
TimWolla commentedon Mar 14, 2025
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
usestatements, adding support for parsing thenamespaceshould 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.TimWolla commentedon Mar 16, 2025
To give an example: The library also fails to correctly serialize property hooks. This example:
results in:
… adding an extra
\in front of theget. 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
usestatements, addingnamespaceextraction 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”.