magento-coding-standard icon indicating copy to clipboard operation
magento-coding-standard copied to clipboard

Add description why php functions marked as discouraged and give alternatives

Open ihor-sviziev opened this issue 5 years ago • 6 comments

Preconditions

N/A

Steps to reproduce

  1. Make PR to Magento with changed file, that don't have like about ignoring discouraged pathinfo function or run magneto coding standard against some file.

Expected result

  1. You should get understandable message what's wrong with your code, message The use of function pathinfo() is discouraged isn't clean enough
  2. For case when some function marked as discouraged - there should be some information that shows reason of it, what is best practice
  3. That's actually relevant for all rules

Good example: https://github.com/extdn/extdn-phpcs/blob/master/Extdn/Sniffs/Classes/StrictTypesSniff.md

Actual result

  1. I'm getting result like this: Errors is quite clear, but not warnings
PHP Code Sniffer detected 2 violation(s): 

FILE: /var/www/html/app/code/Magento/Deploy/Service/DeployPackage.php
----------------------------------------------------------------------
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------
 109 | ERROR   | [x] Missing short description
 110 | ERROR   | [x] There must be exactly one blank line before tags
 224 | WARNING | [ ] The use of function pathinfo() is discouraged
 225 | WARNING | [ ] The use of function pathinfo() is discouraged
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------



Failed asserting that 2 matches expected 0.

Related question on stack overflow, answer not seems to me correct. https://magento.stackexchange.com/questions/192978/the-use-of-function-pathinfo-is-discouraged-magento-2

ihor-sviziev avatar May 24 '19 13:05 ihor-sviziev

Totally agree. I wanted to follow the rules but sometimes it's hard to find an alternative function.

Usually, my approach is searching for that function in the whole vendor/magento folder, see if the Core team use it else where in the code. If I can't find it, I got stuck.

I also know that some are just warning, it is there to help developers mind the usage of the function, for example base64_decode, there is no wrapper, but it is safe to use with trusted input. But the help with a suggestion will save developers time finding, say, in this case, the pathinfo can be found here https://github.com/magento/magento2/blob/2.3-develop/lib/internal/Magento/Framework/Filesystem/Io/File.php#L890

gixid192 avatar May 24 '19 13:05 gixid192

You can use Magento\Framework\Filesystem\Io\File which has a wrapper for that function.

Example: https://magento.stackexchange.com/questions/192978/the-use-of-function-pathinfo-is-discouraged-magento-2/284317#284317

diazwatson avatar Aug 03 '19 14:08 diazwatson

Hi @diazwatson,

You can use Magento\Framework\Filesystem\Io\File which has a wrapper for that function.

Example: https://magento.stackexchange.com/questions/192978/the-use-of-function-pathinfo-is-discouraged-magento-2/284317#284317

Looks really good! Thank you!

However it adds alternative only for one function. This issue was created in order to provide some alternative for all discouraged functions, it could be done even with documentation that describes motivation why each function was marked as discouraged and what should be used instead.

Are you interested in adding such documentation?

ihor-sviziev avatar Aug 03 '19 14:08 ihor-sviziev

Sure, np I will add those

diazwatson avatar Aug 03 '19 17:08 diazwatson

@ihor-sviziev I just reviewed the list of $forbiddenFunctions and is huge (187 functions are forbidden).

Also not all of them have an equivalent or wrapper in Magento. Should we add them to Magento2 repo?

Anyway I have added some more alternatives for discouraged functions

diazwatson avatar Aug 04 '19 00:08 diazwatson

Keep this for future recommendations enhancement.

lenaorobei avatar Aug 12 '19 14:08 lenaorobei