Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reflection: Fix the return value of ReflectionFunction::{getNamespaceName,inNamespace}() for closures #16129

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented Sep 30, 2024

The output is definitely bad, but closures don't have a namespace. Not sure what the correct output would be.

Given the (reasonable) assumption that namespacename + shortname = name, the only option is to return an empty namespace and return false when asking if the closure is inside of a namespace.


Fixes GH-16122

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate we have an unnoticed BC break after RC, but I agree with your assessment that "namespacename + shortname = name" should be true.

ext/reflection/php_reflection.c Outdated Show resolved Hide resolved
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate we have an unnoticed BC break after RC

Well, the current output is just broken, so needs to be fixed anyway.

@TimWolla TimWolla merged commit a1cc091 into php:PHP-8.4 Sep 30, 2024
9 of 10 checks passed
TimWolla added a commit that referenced this pull request Sep 30, 2024
* PHP-8.4:
  reflection: Fix the return value of ReflectionFunction::{getNamespaceName,inNamespace}() for closures (#16129)
@TimWolla TimWolla deleted the gh-16122-closure-reflection branch September 30, 2024 14:35
jorgsowa pushed a commit to jorgsowa/php-src that referenced this pull request Oct 1, 2024
…Name,inNamespace}() for closures (php#16129)

* reflection: Fix the return value of ReflectionFunction::{getNamespaceName,inNamespace}() for closures

Fixes phpGH-16122

* reflection: Clean up implementation of `ReflectionFunctionAbstract::inNamespace()`

* reflection: Clean up implementation of `ReflectionFunctionAbstract::getNamespaceName()`
jorgsowa pushed a commit to jorgsowa/php-src that referenced this pull request Oct 1, 2024
…Name,inNamespace}() for closures (php#16129)

* reflection: Fix the return value of ReflectionFunction::{getNamespaceName,inNamespace}() for closures

Fixes phpGH-16122

* reflection: Clean up implementation of `ReflectionFunctionAbstract::inNamespace()`

* reflection: Clean up implementation of `ReflectionFunctionAbstract::getNamespaceName()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants