php / php-src

The PHP Interpreter
https://www.php.net
Other
37.97k stars 7.72k forks source link

Incorrect `ReflectionClass::getNamespaceName` for anonymous class #12895

Open kubawerlos opened 9 months ago

kubawerlos commented 9 months ago

Description

The following code:

<?php

namespace Foo {
    interface InterfaceFoo {}
}

namespace Bar {
    new class implements \Foo\InterfaceFoo {};
}

namespace {
    foreach (get_declared_classes() as $class) {
        if (!str_contains($class, '@anonymous')) {
            continue;
        }
        echo 'Class: ', $class, PHP_EOL;
        echo 'Namespace: ', (new ReflectionClass($class))->getNamespaceName(), PHP_EOL;
    }
}

Resulted in this output:

Class: Foo\InterfaceFoo@anonymousD:\a\php-anonymous-class-namespace\php-anonymous-class-namespace\test.php:8$0
Namespace: Foo\InterfaceFoo@anonymousD:\a\php-anonymous-class-namespace\php-anonymous-class-namespace

at Windows and

Class: Foo\InterfaceFoo@anonymous/home/runner/work/php-anonymous-class-namespace/php-anonymous-class-namespace/test.php:8$0
Namespace: Foo

at Ubuntu

But I expected this output instead:

Class: Foo\InterfaceFoo@anonymousD:\a\php-anonymous-class-namespace\php-anonymous-class-namespace\test.php:8$0
Namespace: Bar

Reproduced at https://github.com/werlos/php-anonymous-class-namespace (see the only action there).

PHP Version

8.3.0

Operating System

No response

iluuu1994 commented 9 months ago

Hi @kubawerlos. I don't understand, why do you expect backslashes for paths on Ubuntu? The slashes are not namespace separators, its a path appended to the class name to avoid naming conflicts of anonymous classes. The class name of anonymous classes is essentially an implementation detail.

Edit: Oh, sorry. I missed that this is talking about getNamespaceName, not the slashes.

nielsdos commented 9 months ago

This is because Windows paths use backslashes, but the function also uses backslashes to determine where to "cut off" the string: https://github.com/php/php-src/blob/ec79fc9d9c266e85b94fba46b1c347068f565d6f/ext/reflection/php_reflection.c#L5456-L5459

Tested on my Windows 10 VM I get the same behaviour as OP.

nielsdos commented 9 months ago

And getShortName is also messed up as it'll return "foo.php:8$0".

iluuu1994 commented 9 months ago

Indeed. I think OP would also expect Unix to yield Bar, but because the name is prefixed with the parent class (or first implemented interface) rather than the namespace it's also wrong.

nielsdos commented 9 months ago

Okay, so fixing this would consist of two parts:

WDYT?

iluuu1994 commented 9 months ago

That sounds reasonable. namespace@anonymouspath:line$counter should do. I don't know what the point of including the parent class is.

nielsdos commented 9 months ago

I don't know what the point of including the parent class is.

72bd55902d1908857f47555ad69458861e1acd94

"This is intended to display a more useful class name in error messages and stack traces, and thus make debugging easier."

I guess it doesn't really matter that much, but I did find this.

nielsdos commented 9 months ago

@iluuu1994 How do you want to proceed here? I have a working Windows development environment and I already know how to tackle the second bullet point.

EDIT: or we could split the issue between us: 2nd bullet point for me and 1st for you? Just let me know so we at least don't do double work ;)

iluuu1994 commented 9 months ago

@nielsdos However you want, I'll focus on something else tomorrow. I'm certainly not mad if you take care of either or both. ^^ I don't think this is high priority though.

nielsdos commented 9 months ago

Okay, will tackle this tomorrow.

Wirone commented 6 months ago

Let's code PHP like there's no tomorrow! 😀