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

[PropertyAccess] Add/Remove doesn't work with magic call #22990

Open
mitrae opened this Issue May 31, 2017 · 2 comments

Comments

Projects
None yet
4 participants
@mitrae

mitrae commented May 31, 2017

Q A
Bug report? yes
Feature request? no
BC Break report? yes/no
RFC? yes/no
Symfony version 2.8.x

There is no way to use Add/Remove methods together with __call. If magic call enabled then PropertyAccessor always uses setter.

suggestion:

688: $methods = $this->findAdderAndRemover($reflClass, $singulars);
to
688: $methods = $this->findAdderAndRemover($reflClass, $singulars, $this->magicCall);

and fix findAdderAndRemover accordingly:
https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/PropertyAccess/PropertyAccessor.php#L688

private function findAdderAndRemover(\ReflectionClass $reflClass, array $singulars, $isMagicCall = false)
    {
        foreach ($singulars as $singular) {
            $addMethod = 'add'.$singular;
            $removeMethod = 'remove'.$singular;

            $addMethodFound = $this->isMethodAccessible($reflClass, $addMethod, 1);
            $removeMethodFound = $this->isMethodAccessible($reflClass, $removeMethod, 1);

            if ($addMethodFound && $removeMethodFound)
                return array($addMethod, $removeMethod);
            }
        }
        
        if ($singulars && $isMagicCall  && $this->isMethodAccessible($reflClass, '__call', 2))  {
            return array('add'.$singulars[0],  'remove'.$singulars[0]);
        }
    }
@mitrae

This comment has been minimized.

Show comment
Hide comment
@mitrae

mitrae Jun 1, 2017

Hmm.. but with this fix there is no way to use setter. and it could break app for those who already count on setter. i don't know then.
May be let's change scope of findAdderAndRemover from private to protected? At least I can extend core accessor and override findAdderAndRemover in my class and use it like this:

$builder->setDataMapper(new PropertyPathMapper(
   new PropertyAccessorWithAddRemoveMagicCall() 
));

mitrae commented Jun 1, 2017

Hmm.. but with this fix there is no way to use setter. and it could break app for those who already count on setter. i don't know then.
May be let's change scope of findAdderAndRemover from private to protected? At least I can extend core accessor and override findAdderAndRemover in my class and use it like this:

$builder->setDataMapper(new PropertyPathMapper(
   new PropertyAccessorWithAddRemoveMagicCall() 
));
@s001dxp

This comment has been minimized.

Show comment
Hide comment
@s001dxp

s001dxp commented Jun 5, 2018

1+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment