magento-ecg / coding-standard Goto Github PK

I've tried using this tool but when I try to run it I get an exception thrown in /usr/share/php/PHP/CodeSniffer.php on line 786

I feel I must be just running the command correctly? Would really appreciate any advice.

~:$ php -v
PHP 5.5.9-1ubuntu4.6 (cli) (built: Feb 13 2015 19:17:11) 
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies
    with Zend OPcache v7.0.3, Copyright (c) 1999-2014, by Zend Technologies
    with Xdebug v2.2.3, Copyright (c) 2002-2013, by Derick Rethans
~:$ phpcs --version
PHP_CodeSniffer version 1.5.0RC2 (beta) by Squiz Pty Ltd. (http://www.squiz.com.au)
~:$ git clone https://github.com/magento-ecg/coding-standard.git
Cloning into 'coding-standard'...
remote: Counting objects: 305, done.
remote: Total 305 (delta 0), reused 0 (delta 0), pack-reused 305
Receiving objects: 100% (305/305), 46.77 KiB | 0 bytes/s, done.
Resolving deltas: 100% (153/153), done.
Checking connectivity... done.       
~:$ cd coding-standard/
coding-standard: (master)$ phpcs --standard=/home/ubuntu/coding-standard/ruleset.xml /var/www/magento1-9
PHP Fatal error:  Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff "Generic.PHP.Syntax" does not exist' in /usr/share/php/PHP/CodeSniffer.php:786
Stack trace:
#0 /usr/share/php/PHP/CodeSniffer.php(564): PHP_CodeSniffer->_expandRulesetReference(Object(SimpleXMLElement), '/home/ubuntu/codi...', 0)
#1 /usr/share/php/PHP/CodeSniffer.php(419): PHP_CodeSniffer->processRuleset('/home/ubuntu/codi...')
#2 /usr/share/php/PHP/CodeSniffer/CLI.php(614): PHP_CodeSniffer->process(Array, Array, Array, false)
#3 /usr/bin/phpcs(37): PHP_CodeSniffer_CLI->process()
#4 {main}
  thrown in /usr/share/php/PHP/CodeSniffer.php on line 786

In https://github.com/magento-ecg/coding-standard/blob/master/Ecg/Sniffs/Security/ForbiddenFunctionSniff.php#L16 I find both call_user_func functions as forbidden.

I couldn't find a direct source for the reason, so that's why I'm posting it here.

Thanks in advance.

When concatenation is splitted to few lines there should be no alarm.
Example:
$a = '' . func($arg) . ''
. '' . func($arg) . ''
. '' . func($arg) . '';

PHP Fatal error:  Interface 'PHP_CodeSniffer_Sniff' not found in vendor/magento-ecg/coding-standard/EcgM2/Sniffs/Plugins/PluginSniff.php on line 7

Clean Magento 2.1.6 (except for the upped version of PHPCS to 3.0.0-RC4, and added "magento-ecg/coding-standard": "dev-master#5f8143d94677d79bca6f428ad71da63214a2efeb" as a dev dependency)

mageinferno/magento2-php docker image which runs following commands:

composer install --no-interaction --no-progress --optimize-autoloader --no-ansi
php -d memory_limit=128M vendor/bin/phpcs app/code  --extensions=php,phtml --standard=./vendor/magento-ecg/coding-standard/EcgM2/

Can you please explain why parse_url() is on the forbidden list?

Is there anywhere that actually explain why some of these things are errors or warnings? For example a common error I'm seeing amongst third party (and some of my own) modules is "The use of function curl_init() is forbidden". Why is this forbidden? I assume it's because you can't guarantee the curl extension is installed? What's the 'best practice' alternative that we should be doing? It's rather frustrating finding out your code isn't considered best practice but not being able to find out what the best practice is.

Whilst this isn't an 'issue' per se with this repo, I feel if such a thing does exist it should at least be referenced in the Readme.

According to Ecg_Sniffs_Security_ForbiddenFunctionSniff the function constant() is forbidden.
I'm unable to find a replacement within the core or lib, and also wonder why its forbidden.
What would be an alternative function or method to use?

Hello,

I get parse errors when running the coding-standards with PHP_CodeSniffer:
PHP Parse error: parse error in /Applications/MAMP/htdocs/coding-standard/Sniffs/Classes/Mysql4Sniff.php on line 13

Also on:
PHP Parse error: parse error in /Applications/MAMP/htdocs/coding-standard/Sniffs/Classes/ObjectInstantiationSniff.php on line 20
PHP Parse error: parse error in /Applications/MAMP/htdocs/coding-standard/Sniffs/Security/SuperglobalSniff.php on line 24
PHP Parse error: parse error in /Applications/MAMP/htdocs/coding-standard/Sniffs/Sql/RawQuerySniff.php on line 22

I didn't follow the updates closely but for some reason the standard stopped doing code style checks for me. My guess is the version 2.x (2.3.0 as of now) of the phpcs

Ecg.Security.ForbiddenFunction and Ecg.Performance sniffs still run ok. Any clues are appreciated

Performance Loop Sniff detects and reports when load/save/delete model methods are called in a loop. It's obvious why this is a bad practice, however it's quite common to run into this when dealing with Magento models.

Example: Magento Admin grids allow implementing "mass actions". In case of product grid, it allows changing product status, updating attributes and deleting products.
Deleting n products will require you to loop through array of ids, load and delete products.

I'm generally wondering if there's a recommended way how to deal this cases? If you consider catalog products, with all their related models (inventory, gallery) using LSD methods in a loop seems unavoidable in some cases.

Best regards,

I am trying to run:

vendor/bin/phpcs --standard=EcgM2 .

I am having the following error:

PHP Fatal error:  Uncaught PHP_CodeSniffer_Exception: 
Referenced sniff "Generic.PHP.Syntax" does not exist in 
vendor/squizlabs/php_codesniffer/CodeSniffer.php:847

Getting this error while trying to run phpcs with this standard.
Registering sniffs in the Ecg standard... PHP Fatal error: Access level to Ecg_Sniffs_Security_ForbiddenFunctionSniff::$forbiddenFunctions must be public (as in class Generic_Sniffs_PHP_ForbiddenFunctionsSniff) in vendor/magento-ecg/coding-standard/Ecg/Sniffs/Security/ForbiddenFunctionSniff.php on line 4

I changed that class property to public and it runs fine.

C:\xampp\htdocs\my_work\PHP_CodeSniffer\bin>php phpcbf C:\xampp\htdocs\my_work\csv_to_mysql\csv_to_mysql.php

PHP Fatal error: Uncaught exception 'PHP_CodeSniffer\Exceptions\RuntimeException' with message 'file_put_contents(C:\xampp\htdocs\my_work\PHP_CodeSniffer\C:\xa
mpp\htdocs\my_work\csv_to_mysql\csv_to_mysql.php): failed to open stream: Invalid argument in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reports\Cbf.php on lin
e 90' in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php:557
Stack trace:
#0 [internal function]: PHP_CodeSniffer\Runner->handleErrors(2, 'file_put_conten...', 'C:\xampp\htdocs...', 90, Array)
#1 C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reports\Cbf.php(90): file_put_contents('C:\xampp\htdocs...', '<?php\n// import...')
#2 C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reporter.php(262): PHP_CodeSniffer\Reports\Cbf->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile)
, false, 0)
#3 C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php(606): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile), Object(PHP_Co
deSniffer\Config))
#4 C:\xampp\htdocs\my_work\PHP_CodeS in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php on line 557

Fatal error: Uncaught exception 'PHP_CodeSniffer\Exceptions\RuntimeException' with message 'file_put_contents(C:\xampp\htdocs\my_work\PHP_CodeSniffer\C:\xampp\h
tdocs\my_work\csv_to_mysql\csv_to_mysql.php): failed to open stream: Invalid argument in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reports\Cbf.php on line 90'
in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php on line 557

PHP_CodeSniffer\Exceptions\RuntimeException: file_put_contents(C:\xampp\htdocs\my_work\PHP_CodeSniffer\C:\xampp\htdocs\my_work\csv_to_mysql\csv_to_mysql.php): f
ailed to open stream: Invalid argument in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reports\Cbf.php on line 90 in C:\xampp\htdocs\my_work\PHP_CodeSniffer\src
Runner.php on line 557

Call Stack:
0.0003 119568 1. {main}() C:\xampp\htdocs\my_work\PHP_CodeSniffer\bin\phpcbf:0
0.0047 296608 2. PHP_CodeSniffer\Runner->runPHPCBF() C:\xampp\htdocs\my_work\PHP_CodeSniffer\bin\phpcbf:18
0.4920 4710528 3. PHP_CodeSniffer\Runner->run() C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php:193
0.5218 5384552 4. PHP_CodeSniffer\Runner->processFile() C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php:394
0.5640 5653168 5. PHP_CodeSniffer\Reporter->cacheFileReport() C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Runner.php:606
0.5641 5672216 6. PHP_CodeSniffer\Reports\Cbf->generateFileReport() C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reporter.php:262
0.7576 5710456 7. file_put_contents() C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reports\Cbf.php:90
0.7582 5713520 8. PHP_CodeSniffer\Runner->handleErrors() C:\xampp\htdocs\my_work\PHP_CodeSniffer\src\Reports\Cbf.php:90

C:\xampp\htdocs\my_work\PHP_CodeSniffer\bin>

In case child class extends Magento\Framework\App\Config\Value class with afterSave() method a Warning message is shown:
Plugin afterSave function should have at least two parameters.

Note: there is no plugin declaration in di.xml file.

do {
    ...
} while (...);

leads to

----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | An error occurred during processing; checking has been
   |       | aborted. The error message was: Undefined array key
   |       | "parenthesis_opener" in
   |       | xxx/vendor/magento-ecg/coding-standard/Ecg/Sniffs/Performance/LoopSniff.php
   |       | on line 73 (Internal.Exception)
----------------------------------------------------------------------

Can LSD Model issues be as warning, not error?
It can be very bad to use them in big amount of loops, nevertheless it is not such a bad idea to use them in small collections

• LSD method save() detected in loop
• LSD method load() detected in loop

Change please
is_file
filemtime
pathinfo

to warning

Hi,
I have installed PHP_Codesniffer extension to find out the magento 2 coding standard issues.
I have run the below command.
"phpcs --standard=Ecg --extensions=php,xml,phtml --report-full=/path/testing.log /path/"

This extension lists out many errors in the log file from default magento 2 itself.

So, kindly let me know how to say that the default magento 2 coding standard has so many issues.

getting an error when using EcgM2 standard in phpcs in magento 2.4

Fatal error: Uncaught TypeError: vsprintf(): Argument #2 ($values) must be of type array, string given in /var/www/html/vendor/squizlabs/php_codesniffer/src/Files/File.php:1056
Stack trace:
#0 /var/www/html/vendor/squizlabs/php_codesniffer/src/Files/File.php(1056): vsprintf('Unescaped outpu...', '<?= json_encode...')
#1 /var/www/html/vendor/squizlabs/php_codesniffer/src/Files/File.php(672): PHP_CodeSniffer\Files\File->addMessage(true, 'Unescaped outpu...', 8, 29, 'Unescaped outpu...', '<?= json_encode...', 5, false)
#2 /var/www/html/vendor/magento-ecg/coding-standard/EcgM2/Sniffs/Templates/EscapedOutputSniff.php(84): PHP_CodeSniffer\Files\File->addError('Unescaped outpu...', 47, 'Unescaped outpu...', '<?= json_encode...')
#3 /var/www/html/vendor/squizlabs/php_codesniffer/src/Files/File.php(498): EcgM2\Sniffs\Templates\EscapedOutputSniff->process(Object(PHP_CodeSniffer\Files\LocalFile), 47)
#4 /var/www/html/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(92): PHP_CodeSniffer\Files\File->process()
#5 /var/www/html/vendor/squizlabs/php_codesniffer/src/Runner.php(628): PHP_CodeSniffer\Files\LocalFile->process()
#6 /var/www/html/vendor/squizlabs/php_codesniffer/src/Runner.php(434): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile))
#7 /var/www/html/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
#8 /var/www/html/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
#9 /var/www/html/vendor/bin/phpcs(117): include('/var/www/html/v...')
#10 {main}
thrown in /var/www/html/vendor/squizlabs/php_codesniffer/src/Files/File.php on line 1056

Hi,

the direct array dereference (e.g. $phpcs->getTokens()[$next]['content']) is only possible from PHP 5.4 up and should be replaced with temporary variables for lower PHP versions.
(Source: http://php.net/manual/en/language.types.array.php#example-88)

Best regards,
Chris

12 | ERROR | The use of function tempnam() is forbidden
20 | ERROR | The use of function pathinfo() is forbidden
35 | ERROR | The use of function unlink() is forbidden

111 | ERROR | The use of function filesize() is forbidden

Why ERROR level?..
Magento uses this function also.

i think this warning message is incorrect... this message show on my backend model for product attribute

<?php
namespace Training4\Warranty\Model\Attribute\Backend;

class AttributeWarranty extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend
{
    public function beforeSave($object)
    {
        $value = $object->getData($this->getAttribute()->getAttributeCode());
        if (is_numeric($value)) {
            $value .= " year(s)";
            $object->setData($this->getAttribute()->getAttributeCode(), $value);
        }
        return parent::beforeSave($object);
    }
}

What about that?
Why it is forbidden?
In shipping modules some external services want work in different encodings.

This doesn't appear to be executing on design templates? Am I missing something?

I have noticed that you list PHP 5.4 as a requirement, while Magento requires PHP 5.3. I am unsure how to reconcile this discrepancy. Is this okay to use with 5.3? Is Magento safe to use with 5.4?

Best wishes,
Max.

I'm using EcgM2 and I tought that it would be nice to add it as a dependency with composer and setup with a custom phpcs.xml so that every one in the team can use it more easily.

But if I add this line in phpcs.xml
<rule ref="./vendor/magento-ecg/coding-standard/EcgM2"/>

phpcs dies with this error:
PHP Fatal error: Uncaught PHP_CodeSniffer_Exception: Referenced sniff "Ecg.Performance.CollectionCount" does not exist

While if I try to add Ecg it works (but since it is a Magento 2 project I don't need it)

I tryed the standard Ecg on a Magento 1.9 custom Module. The stdout report works as expected and spots many Warning and Errors. Then I tryed to generate a diff file to eventually patch the code in the future, though mostly to browse what kind of modifications the standard would apply. But the resulting diff file is empty.

This is the command I used:

phpcs --report-diff=phpcs.diff --standard=Ecg .

I tryed the same command with different standard:

phpcs --report-diff=phpcs.diff --standard=PSR2 .

And it produce the desired diff file.

Is the release schedule of this repo tied to the M2 one? I'd rather this one be separate and have small, but more frequent tags/release, so we can use it via composer in a more elegant way, not using master.

Hi,

i wanted to use your project, to check my Magento module. However it doesn't work (i have tried both coding standards 1.2 version + php code sniffer 2.3.3 ... and latest versions from master - both have the same issue.

$ ../PHP_CodeSniffer-2.3.3/scripts/phpcs --standard=../coding-standard-1.2 ModuleName/ > standards2.txt

PHP Fatal error:  Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff "Ecg.Security.LanguageConstruct.DirectOutput" does not exist' in /workspace/PHP_CodeSniffer-2.3.3/CodeSniffer.php:1092

Stack trace:
#0 /workspace/PHP_CodeSniffer-2.3.3/CodeSniffer.php(733): PHP_CodeSniffer->_expandRulesetReference(Object(SimpleXMLElement), '/workspace/...', 0)
#1 /workspace/PHP_CodeSniffer-2.3.3/CodeSniffer.php(551): PHP_CodeSniffer->processRuleset('/workspace/...')
#2 /workspace/PHP_CodeSniffer-2.3.3/CodeSniffer/CLI.php(818): PHP_CodeSniffer->initStandard(Array, Array)
#3 /workspace/PHP_CodeSniffer-2.3.3/CodeSniffer/CLI.php(95): PHP_CodeSniffer_CLI->process()
#4 /workspace/PHP_CodeSniffer-2.3.3/scripts/phpcs(25): PHP_CodeSniffer_CLI->runphpcs()
#5 {main}
  thrown in /workspace/PHP_CodeSniffer-2.3.3/CodeSniffer.php on line 1092

Hello.

I was thinking that this standard shouldn't be standalone, but extending the Zend one - with <rule ref="Zend"/>.

What do you think?

Hello,

Magento 2 just published Technical Guidelines and it's pretty cool.

As per as:

2.7. All non-public properties and methods SHOULD be private.

This currently displays a warning with EcgM2, like M1 we warning about private methods and properties.

There are something more, should we improve it to prepare for a better 2.2 release?

P/s: To whom may concern, if you want to make this work. Please follow this pull request: #45

In #13e5071 the "NamespaceSniff" sniffer was introduced, with the comment "When catching an exception inside a namespace it is important that you escape to the global space."

I assume that this is meant to prevent accidental catch (Exception $e) statements where Exception is not imported.

However it is also triggered by code like this:

namespace N1;

class CustomException extends \Exception {}

try {
    throw CustomException;
} catch (CustomException) {
}

which is valid good code. Using the FQN instead even triggers a warning by PHP Inspections in PhpStorm.

If you do not want to remove this check (which I would prefer), I suggest to limit it to "Exception" and "Mage_*Exception" classes. But note that this would still give a false positive with N1\Exception.

But it should definitely be a warning, and not an error.

The tempnam function is forbidden and there seems to be no workaround.

I know this standard is supposed to be a simple tool to spot areas of the code that need a deeper review before shipping, however, there is a valid point in avoiding most of these functions whenever possible as they are not only potential security risks but also (in most cases) get in the way of making the code testable.

So as a general rule the workaround for these is "use whatever library Magento/Zend provide to abstract such functions". That's great and all, but for cases such as tempnam, there is no workaround provided by the platform.

So:

1. What is the proposed way to deal with these cases? I would like to avoid having multiple variations of MyCompany_MyModule_Helper_File::tempnam() across multiple modules if possible.

2. Is there actually a library to deal with temporary files in Magento? I haven't found any, and I found a lot of duplicated code across both Magento and ZendFramework dealing with tempnam in different scenarios with slight variations

Discuss & remove most functions from this sniff
First candidates:

• curl_