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

ReflectionFunction::getNamespaceName() is broken for closures with PHP8.4 #16122

Closed
takaram opened this issue Sep 29, 2024 · 5 comments
Closed

Comments

@takaram
Copy link

takaram commented Sep 29, 2024

Description

The following code:

<?php

namespace Foo;

class Bar
{
    public static function testClass(): \Closure
    {
        return function(){};
    }
}

function testFunc(): \Closure
{
    return function(){};
}

var_dump((new \ReflectionFunction(Bar::testClass()))->getNamespaceName());
var_dump((new \ReflectionFunction(testFunc()))->getNamespaceName());

Resulted in this output:

string(12) "{closure:Foo"
string(12) "{closure:Foo"

But I expected this output instead:

string(3) "Foo"
string(3) "Foo"

https://3v4l.org/QjjJK/rfc

PHP Version

PHP 8.4.0RC1

Operating System

No response

@iluuu1994
Copy link
Member

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

@iluuu1994
Copy link
Member

Oh, I see this only changed in 8.4.

@iluuu1994
Copy link
Member

iluuu1994 commented Sep 29, 2024

The name was previously Foo\{closure}, and is now {closure:Foo\Bar::testClass():9}. It seems this is related to #13550. Hence, the logic in ReflectionFunctionAbstract::getNamespaceName() is now broken, as it's looking for the first \ and, if present, returning everything up to it. /cc @TimWolla

zend_string *name = fptr->common.function_name;
const char *backslash = zend_memrchr(ZSTR_VAL(name), '\\', ZSTR_LEN(name));
if (backslash) {
RETURN_STRINGL(ZSTR_VAL(name), backslash - ZSTR_VAL(name));
}
RETURN_EMPTY_STRING();

@TimWolla
Copy link
Member

Related to #14001 (which fixed it for getShortName()) and #14087 which follow-up fixed that for FCC. I'll send a PR tomorrow.

@TimWolla
Copy link
Member

This also affects inNamespace(). My PR will fix both.

TimWolla added a commit to TimWolla/php-src that referenced this issue Sep 30, 2024
jorgsowa pushed a commit to jorgsowa/php-src that referenced this issue 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 issue 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

No branches or pull requests

4 participants