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

self doesn't work as expected in combination with late static binding #3293

Open
GuySartorelli opened this issue Mar 26, 2024 · 10 comments
Open

Comments

@GuySartorelli
Copy link

Description

The following code:

<?php
class foo {
    static public function getclass() {
        var_dump(get_called_class());
    }
    static public function test() {
        self::getclass();
    }
}

class bar extends foo {}

bar::test();

Resulted in this output:

string(3) "bar"

But I expected this output instead:

string(3) "foo"

Additional context

In the docs about late static binding it says

Static references to the current class like self:: or __CLASS__ are resolved using the class in which the function belongs, as in where it was defined

This leads me to believe that get_called_class() should be returning foo, because that is the class where the self:: was defined, and get_called_class() is called downstream from that.

It's possible that everything's working as it's supposed to, but in that case I believe the documentation for late static binding needs to be updated to include this edge case as it's extremely unintuitive - it feels like self is just a synonym for static in this scenario.

PHP Version

PHP 8.1.27

Operating System

No response

@GuySartorelli GuySartorelli added bug Documentation contains incorrect information Status: Needs Triage labels Mar 26, 2024
@GuySartorelli
Copy link
Author

I've just had this pointed out to me which increases my confusion:

class foo {
    static public function getclass() {
        var_dump(self::class);
    }
    static public function test() {
        static::getclass();
    }
}
class bar extends foo {}

// will output "foo", not "bar"
bar::test();
class foo {
    static public function getclass() {
        var_dump(static::class);
    }
    static public function test() {
        self::getclass();
    }
}
class bar extends foo {}

// will output "bar", not "foo"
bar::test();

There's definitely something very unexpected going on with the relationship between self and late static binding which at best needs clearer documentation.

@GuySartorelli GuySartorelli changed the title self is synonymous with static when used upstream of get_called_class() self doesn't work as expected in combination with late static binding Mar 26, 2024
@damianwadley
Copy link
Member

get_called_class is working as intended: it was created before LSB was implemented, and it was that era's solution to the "how do I know what class this static method was called on?" problem. So it is supposed to return the name of the class that was first called.

For self vs static, the former always refers to the class in which the code is written, while the latter is resolved at runtime to be the called class - the same as get_called_class. So in your first example, because self::class was written within foo so it will evaluate to "foo", while static::class will evaluate to "bar" because that chain of static method calls began using bar.

@GuySartorelli
Copy link
Author

GuySartorelli commented Mar 27, 2024

That makes some sense.
I think most of my confusion is that I have trained myself to see self to mean "the class where this code is defined", which matches how it's described in the documentation about late static binding.
You even said yourself:

For self vs static, the former always refers to the class in which the code is written

This means that self in the foo class refers to the foo class...

So with the foo class in the issue description, I'd expect these two be identical:

static public function test() {
    self::getclass();
}
static public function test() {
    foo::getclass();
}

But they're not identical when late static binding comes into play.

It sounds like that's intended - so my recommendation is that the documentation for late static binding makes this scenario explicitly clear. I'd go so far as to recommend it discourage calling static methods with self, since it can produce unexpected results.

@GuySartorelli
Copy link
Author

GuySartorelli commented Mar 27, 2024

Also.... does the "Status: Needs Feedback" label mean you were waiting for a response from me? If so, in future could you please explicitly ask for me (or whoever you're waiting on) to respond? "needs feedback" could mean (and I assumed it did mean, until the label changed again) that you're waiting for feedback from additional maintainers. I almost didn't respond at all.

@damianwadley
Copy link
Member

But they're not identical when late static binding comes into play.

LSB kicks in when the first static method call to a class hierarchy is made, and it stays the same while you remain within that hierarchy. If you go outside the hierarchy, a new layer of LSB starts.
https://3v4l.org/TRP70

self and static stay within the hierarchy so the LSB remains intact - it doesn't start up a new one just because you switched keywords. The difference between the two is which particular version of a method should be executed: the one defined in the code's own class, or the one defined by the LSB class.

In other words, it's the same kind of inheritance that has always been available to class instances, with LSB being equivalent to the concept of a class instance, and static:: being equivalent to $this->.

Also.... does the "Status: Needs Feedback" label mean you were waiting for a response from me?

It means that I was confident you were going to reply, even if I didn't explicitly write a "So does that make sense?" at the end. And also that I didn't want to leave the issue as "needing triage" (it's not a bug), nor did I want to close it (maybe there is a bug and I don't see it).


So does that make sense? 😉

@GuySartorelli
Copy link
Author

It makes sense in that I understand what is happening (and will avoid calling static methods with self from now on to avoid this type of problem), yes.

I still maintain that this is a scenario that should be more clearly documented, since the mention of self in the docs about late static binding initially reinforced the feeling that something was wrong, where it should instead make it clear that what's happening is expected (at least from a "this is what the maintainers of PHP have decided should happen" sense, if not from the perspective of the developer using it :p)

@damianwadley
Copy link
Member

I'll move this over to the docs repo, but it would be very helpful if you could think of what kinds of changes that would like to see - what changes would have helped you if you had been able to read them earlier. Because it can be quite hard for people who are familiar with the subject matter to write good documentation for people who aren't as familiar with it...

@damianwadley damianwadley removed bug Documentation contains incorrect information Status: Needs Triage labels Mar 27, 2024
@damianwadley damianwadley transferred this issue from php/php-src Mar 27, 2024
@GuySartorelli
Copy link
Author

GuySartorelli commented Jun 5, 2024

The main change that I think would clear it up for me is after this statement:

Static references to the current class like self:: or __CLASS__ are resolved using the class in which the function belongs, as in where it was defined

Like I said before, in my initial example the above wording leads me to believe that get_called_class() should be returning foo, because that is the class where the self:: was defined.

The Limitations of self:: section should ideally explicitly give an example like those in this issue with an explanation about what is actually happening there, since the behaviour noted in this issue seems to contradict what's said in that section currently.

@ntd
Copy link

ntd commented Jun 19, 2024

LSB kicks in when the first static method call to a class hierarchy is made, and it stays the same while you remain within that hierarchy. If you go outside the hierarchy, a new layer of LSB starts.
https://3v4l.org/TRP70

I slightly modified your example and the new LSB layer is started even remaining inside the same hierarchy. It is the explicit class call that triggers it regardless:

class Four {
    public static function four() {
        var_dump(static::class);
    }
}

class Three extends Four {
    public static function three() {
        var_dump(static::class);
        Four::four();
    }
}

class Two extends Three {
    public static function two() {
        var_dump(static::class);
        static::three();
    }
}

class One extends Two {
    public static function one() {
        var_dump(static::class);
        self::two();
    }
}

$one = new One();
$one->one();

and the output is the same:

string(3) "One"
string(3) "One"
string(3) "One"
string(4) "Four"

@GuySartorelli
Copy link
Author

GuySartorelli commented Jul 29, 2024

FWIW using self::class::myMethod() does what I originally expected self::myMethod() to do.

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