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

symfony/property-access 6.4.6 breaks magic methods #54739

Closed
boonkerz opened this issue Apr 26, 2024 · 14 comments
Closed

symfony/property-access 6.4.6 breaks magic methods #54739

boonkerz opened this issue Apr 26, 2024 · 14 comments

Comments

@boonkerz
Copy link

boonkerz commented Apr 26, 2024

Symfony version(s) affected

6.4.6

Description

6.4.6 break magic methods.

How to reproduce

class test
{

    private array $tests = [];


    public function __get(string $name): mixed
    {
        return $this->tests[$name]?? null;
    }

    public function __set(string $name, mixed $value): void
    {
        $this->tests[$name] = $value;
    }
}


$test = new test();
$propertyAccessor = new PropertyAccessor();
var_dump($propertyAccessor->getValue($test, 'Wouter'));

6.4.4 return null
6.4.6 return Can't get a way to read the property "Wouter" in class "test"

Possible Solution

No response

Additional Context

No response

@boonkerz boonkerz added the Bug label Apr 26, 2024
@boonkerz boonkerz changed the title symfony/property-access symfony/property-access 6.4.6 breaks magic methods Apr 26, 2024
@xabbuh
Copy link
Member

xabbuh commented Apr 26, 2024

Looks like your class is missing the implementation of the __isset() method:

public function __isset(string $name): bool
{
    return null !== ($this->tests[$name] ?? null);
}

@boonkerz
Copy link
Author

class test
{

    private array $tests = [];

    public function __get(string $name): mixed
    {
        return $this->tests[$name]?? null;
    }

    public function __set(string $name, mixed $value): void
    {
        $this->tests[$name] = $value;
    }

    public function __isset(string $name): bool
    {
        return null !== ($this->tests[$name]?? null);
    }
}


$test = new test();
$propertyAccessor = new PropertyAccessor();
var_dump($propertyAccessor->getValue($test, 'Wouter'));

this does not help to fix the behaviour.

returns the same error.

@boonkerz
Copy link
Author

Here is the Code

https://phpsandbox.io/n/propertyaccess-fipnm

@bastien-effetb
Copy link

Same problem here

@n0rbyt3
Copy link
Contributor

n0rbyt3 commented Apr 26, 2024

There is an updated check for non-initialized properties which causes the issue:

if (!isset($object->$name) && !\array_key_exists($name, (array) $object)) {
try {
$r = new \ReflectionProperty($class, $name);
if ($r->isPublic() && !$r->hasType()) {
throw new UninitializedPropertyException(sprintf('The property "%s::$%s" is not initialized.', $class, $name));
}
} catch (\ReflectionException $e) {
if (!$ignoreInvalidProperty) {
throw new NoSuchPropertyException(sprintf('Can\'t get a way to read the property "%s" in class "%s".', $property, $class));
}
}
}

Your magic __get() method can handle uninitialized properties by returning null. Adding this __isset() method resolves the issue:

public function __isset(string $name): bool
{
  return true;
}

@boonkerz
Copy link
Author

yea that works but this is not obvious.
the docs does not say anything about this new behaviour
https://symfony.com/doc/current/components/property_access.html#magic-get-method
and its differ from the previous version.

@n0rbyt3
Copy link
Contributor

n0rbyt3 commented Apr 26, 2024

Indeed, it's just a workaround.

Friendly ping @nicolas-grekas who changed the implementation in commit 9610a7c. An __isset() implementation has been added to the TestClassMagicGet fixture which is a BC break.

I don't see any reason why this additional check exists. IMO, it's part of the PropertyTypeExtractorInterface to detect if the property is accessible. The ReflectionExtractor inside the PropertyInfo component does all the reflection logic so there should be no need to reflect the property again - even if another PropertyTypeExtractorInterface implementation is used. Otherwise PropertyReadInfo should be extended to return more details for the PropertyAccessor.

@k0d3r1s
Copy link
Contributor

k0d3r1s commented Apr 30, 2024

i will add this here:
property-access 6.4.6 and 6.4.7 breaks doctrine migrations diff, going back to 6.4.4 fixes the issue

@derrabus
Copy link
Member

@k0d3r1s Please open a new issue with a detailed reproducer.

@aesislabs
Copy link

Same problem here, it break my applications

@nicolas-grekas
Copy link
Member

That's because you're missing an implementation for __isset. While property access was tolerant to this, this breaks the behavior of objects with the php engine.

@netsuo
Copy link

netsuo commented May 1, 2024

I have no problem with that, but maybe it needs some doc update

@nicolas-grekas
Copy link
Member

Maybe a note in the UPGRADE-6.4.md file? Up for a PR?

@boonkerz
Copy link
Author

boonkerz commented May 6, 2024

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

10 participants