array_push(...) calls behaving as '$array[] = ...', $array[] = works faster than invoking functions in PHP

[lfluvisotto]

Description

array_push(...) calls behaving as '$array[] = ...', $array[] = works faster than invoking functions in PHP

See note in PHP documentation:

http://php.net/manual/en/function.array-push.php

Note: If you use array_push() to add one element to the array, it's better to use $array[] = because in that way there is no overhead of calling a function.

[magento-engcom-team]

Hi @lfluvisotto. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on Pull Request changes
• @magento-engcom-team give me new test instance - deploy NEW test instance based on Pull Request changes
• @magento-engcom-team give me {$VERSION} instance - deploy Vanilla Magento instance for Issue or Pull Request

For more details, please, review the Magento Contributor Assistant documentation

[lfluvisotto]

@orlangur and @osrecio

Let me know when this PR gets merged then I will create the forwardport (2.3-develop) and backport (2.1-develop)

[orlangur]

@lfluvisotto nice, didn't know that 👍 Really x10 performance boost? O.o

Don't think it is a measurable imrpvement but like the $array[] more 🤣

Please add array_push to a list of forbidden functions, there is such phpcs sniff I believe.

[lfluvisotto]

Hi @orlangur

I didn't get you meant

What am I supposed to do?

[orlangur]

@lfluvisotto you are supposed to make https://github.com/magento/magento2/blob/2.2-develop/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php#L202 catch such occurrences.

Here is the sniff I'm referring to: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Generic/Sniffs/PHP/ForbiddenFunctionsSniff.php

[ihor-sviziev]

@lfluvisotto you can find example where function was added to forbidden list:
magento-engcom/php-7.2-support#31

[orlangur]

@ihor-sviziev I believe this is incorrect usage of obsolete methods and they are not even running on Travis :/

[ishakhsuvarov]

@orlangur @ihor-sviziev I believe array_push is still valid to add more then one element to the array. I would not forbid it completely

[lfluvisotto]

So do we add array_push to the list of forbidden functions or not?

Like the PHP note says:

http://php.net/manual/en/function.array-push.php

I prefer don't use array_push.

[orlangur]

@ishakhsuvarov why? It's more wordy and slower anyway :)

Let's proceed with this PR and I'll propose corresponding sniff as separate PR, then we can discuss.

[magento-engcom-team]

Hi @orlangur, thank you for the review.
ENGCOM-2012 has been created to process this Pull Request

[lfluvisotto]

@orlangur do I create other PR to put the array_push function to the list of forbidden functions?

[orlangur]

@lfluvisotto not needed, thanks, I have a couple of other phpcs-related staff in my ToDo, will contribute it this month.

[magento-engcom-team]

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

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

[orlangur]

@lfluvisotto this is just a bot, better mention or even directly assign reviewers of original PR 😉