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

Adding support for variadic arguments' method in generated proxy c… #15177

Conversation

bartoszkubicki
Copy link

@bartoszkubicki bartoszkubicki commented May 12, 2018

Proxy for class with variadic argument method

Pull request fixes proxy class generation, so class with method with variadic arguments can be also used in proxy class.

Description

In Magento/Framework/ObjectManager/Code/Generator/Proxy (lines 158, 159) I added check if parameter is variadic and if so its name is suffixed by '...', so then in method _getMethodBody() proxy method body is generated properly.

Fixed Issues (if relevant)

Extends fixes from 27065cf

Manual testing scenarios

  1. Create class, containing method, which as last argument takes variadic argument.
  2. Declare in di.xml proxy for this class.
  3. Let Magento generate it.
  4. Test if using class is possible and there are no errors. Moreover check manually content of generated proxy in /generated/code.

I was trying to extend unit test for modified class (Magento\Framework\ObjectManager\Test\Unit\Code\Generator), so it also would generate and test class with method with variadic argument, but with no success. Test broke on (mocked) method - generateResultFileName(). Actually, I only tried to edit fixtures files for test, not test itself:

Magento\Framework\ObjectManager\Code\Generator\Sample, adding method like:

/**
     * @param int $int
     * @param Sample[] ...$samples
     * @return int
     */
    public function takeVariadicObjectArguments(int $int, Sample ...$samples) : int
    {
        return (int) count($samples);
    }

And Magento\Framework\ObjectManager\Code\Generator\Sample_Proxy.txt, adding generated method like this:

/**
     * {@inheritdoc}
     */
    public function takeVariadicObjectArguments(int $int, \Magento\Framework\ObjectManager\Code\Generator\Sample ... $samples) : int
    {
        return $this->_getSubject()->takeVariadicObjectArguments($int, ... $samples);
    }

Maybe someone else would be able to extend unit test for this class. If we don't change anything in case of unit tests, test is green.

Original PR

#16080

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented May 12, 2018

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@bartoszkubicki bartoszkubicki changed the title Adding support for variadic arguments fro method in generated proxy c… Adding support for variadic arguments' method in generated proxy c… May 12, 2018
@magento-engcom-team magento-engcom-team added this to the May 2018 milestone May 12, 2018
@magento-engcom-team
Copy link
Contributor

@bartek9007 thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@okorshenko okorshenko removed this from the May 2018 milestone May 31, 2018
@magento-engcom-team magento-engcom-team merged commit 92a6cce into magento:2.3-develop Jun 2, 2018
@magento-engcom-team
Copy link
Contributor

Hi @bartek9007. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.3.0 release.

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

Successfully merging this pull request may close these issues.

6 participants