Changing the generated name for anon classes View as Markdown

Nikita,

Just to chime in here: would it be wise to add an interface instead? (this
would automatically be added by the engine) so that the consumer of a
generated class could really know based on the interface that it's a
generated anonymous class instead of string type checking the classname?
Relying on some interface seems te be more future proof than just
changing the string to look up ;)

Keep up the good work!

Regards
Robin

Op do 6 feb. 2020 om 12:21 schreef Nikita Popov nikita.ppv@gmail.com:

Hi internals,

Based on a suggestion by Nicolas Grekas,
https://github.com/php/php-src/pull/5153 changes the generated name for
anonymous classes to include the name of the parent class or first
interface. So instead of just class@anonymous, you'll see something like
EventHandler@anonymous in error messages, for example.

There's a minor BC break here, for code checking for a "class@anonymous"
prefix, which should be easy to rectify by checking for "@anonymous"
instead.

What do people think about doing this change?

Regards,
Nikita

Hi,

Nikita Popov wrote:

Hi internals,

Based on a suggestion by Nicolas Grekas,
https://github.com/php/php-src/pull/5153 changes the generated name for
anonymous classes to include the name of the parent class or first
interface. So instead of just class@anonymous, you'll see something like
EventHandler@anonymous in error messages, for example.

There's a minor BC break here, for code checking for a "class@anonymous"
prefix, which should be easy to rectify by checking for "@anonymous"
instead.

What do people think about doing this change?

Regards,
Nikita

Perhaps it would make sense to include the namespace used by the code
which defines/instantiates the class? That might make it easier, when
looking at a mysterious anonymous class using var_dump() deep in a big
project perhaps, to find out where it came from.

Andrea

Le ven. 7 févr. 2020 à 14:36, Nikita Popov nikita.ppv@gmail.com a écrit :

Hi,

Nikita Popov wrote:

Hi internals,

Based on a suggestion by Nicolas Grekas,
https://github.com/php/php-src/pull/5153 changes the generated name for
anonymous classes to include the name of the parent class or first
interface. So instead of just class@anonymous, you'll see something
like
EventHandler@anonymous in error messages, for example.

There's a minor BC break here, for code checking for a "class@anonymous
"
prefix, which should be easy to rectify by checking for "@anonymous"
instead.

What do people think about doing this change?

Regards,
Nikita

Perhaps it would make sense to include the namespace used by the code
which defines/instantiates the class? That might make it easier, when
looking at a mysterious anonymous class using var_dump() deep in a big
project perhaps, to find out where it came from.

That would be a possible alternative. This is similar to what we do for
closures.

As we need the generated name to be reasonably concise, we can only do one
or the other though. I don't know which one is more useful... Maybe Nicolas
wants to chime in.

The proposed names are the most useful from my experience, take e.g. these
lines, found in the same class:

throw new class() implements ServerExceptionInterface {...};
[...]
throw new class() implements ClientExceptionInterface {...};

The debugging experience is better mentioning ServerExceptionInterface or
ClientExceptionInterface than the namespace of the surrounding class. The
file + line info will fo straight to this surrounding class also, so the
proposal maximizes the amount of useful info from my experience.

Nicolas

Hi,

Nikita Popov wrote:

Hi internals,

Based on a suggestion by Nicolas Grekas,
https://github.com/php/php-src/pull/5153 changes the generated name for
anonymous classes to include the name of the parent class or first
interface. So instead of just class@anonymous, you'll see something
like
EventHandler@anonymous in error messages, for example.

There's a minor BC break here, for code checking for a "class@anonymous
"
prefix, which should be easy to rectify by checking for "@anonymous"
instead.

What do people think about doing this change?

Regards,
Nikita

Perhaps it would make sense to include the namespace used by the code
which defines/instantiates the class? That might make it easier, when
looking at a mysterious anonymous class using var_dump() deep in a big
project perhaps, to find out where it came from.

That would be a possible alternative. This is similar to what we do for
closures.

As we need the generated name to be reasonably concise, we can only do one
or the other though. I don't know which one is more useful... Maybe Nicolas
wants to chime in.

I've scrolled through anon class uses in popular composer packages, and
that convinced me that parent/interface is probably the most useful
information here. Adding the namespace prefix leaves you with something
like "Amp\class@anonymous", which doesn't really tell you anything at all,
while adding the interface gives you "Amp\CancellationToken@anonymous",
which is fairly meaningful. Many more examples like this.

Nikita