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

get_class_vars contains a break change #13835

Closed
eerison opened this issue Mar 29, 2024 · 6 comments
Closed

get_class_vars contains a break change #13835

eerison opened this issue Mar 29, 2024 · 6 comments

Comments

@eerison
Copy link

eerison commented Mar 29, 2024

Description

Hello guys, on php 8.1 + contain a break change, it should be the same behaviour for all php 8.x version, shouldn't it?

The following code:
https://3v4l.org/KRkS9#v

<?php
declare(strict_types=1);

class ExampleClass {
    public static bool $_strict = true;
}

ExampleClass::$_strict = false;

$example =  get_class_vars(ExampleClass::class);
var_dump($example);

Resulted in this output:

array(1) {
  ["_strict"]=>
  bool(true)
}

But I expected this output instead:

array(1) {
  ["_strict"]=>
  bool(false)
}

PHP Version

PHP 8.1+

Operating System

No response

@alecpl
Copy link

alecpl commented Mar 29, 2024

Documentation says: "Returns an associative array of declared properties visible from the current scope, with their default value". So, I suppose this was a bug fixed in 8.1.

@eerison
Copy link
Author

eerison commented Mar 29, 2024

Documentation says: "Returns an associative array of declared properties visible from the current scope, with their default value". So, I suppose this was a bug fixed in 8.1.

well I don't think so, if you check the link above, it is working since 7.4, on 8.1 it changed

@iluuu1994
Copy link
Member

@eerison What Aleksander is saying is that the function is a misnomer:

get_class_vars — Get the default properties of the class

This function (apparently) is intended not to return the current values of the static variables, but their default values. This feature probably makes sense more for instance rather than static properties. Anyway, this behavior seems correct according to the documentation.

@eerison
Copy link
Author

eerison commented Mar 30, 2024

@eerison What Aleksander is saying is that the function is a misnomer:

get_class_vars — Get the default properties of the class

This function (apparently) is intended not to return the current values of the static variables, but their default values. This feature probably makes sense more for instance rather than static properties. Anyway, this behavior seems correct according to the documentation.

Ahhhh I got it

Yeah that's make sense, i guess this method was used like not recommended 🥲. Ok no problem I changed the implementation from my side.

But the question is, shouldn't it be used like this for others projects? Shouldn't it have some warning or deprecation message before change the behavior?

As you can see it works like this since 7.4, some informative message could save some hours of debug, it is a really tricky issue to find out 😅.

Maybe it should have deprecated on 7.4, but i guess no one was aware of it.

Well keep the same behavior makes the upgrades for minor versions smooth, But on the other hand I don't know if it worth the effort.

@iluuu1994
Copy link
Member

@eerison I had another look and I think you might be right after all. It doesn't seem like this change was intentional, but implicitly changed as part of 4b79dba. We don't have a test like your test-case, which is likely why it was missed.

Nonetheless, given that the implementation has been like this for years at this point, and it matches the documentation, I'm not sure if fixing it doesn't cause more trouble than it's worth.

@eerison
Copy link
Author

eerison commented Mar 30, 2024

Maybe we could wait if someone else complain about this issue, if yes then we could reopen this issue, and fix it.

Because it is a really special case and it isn't documented.

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

3 participants