Working with Magento 2 there are currently 4 standards in use

PSR-2 MEQP2 ECG2 Tests included in codebase

Let's assess the merits and choose one as the base ruleset.

Travis fails with an error on the command openssl aes-256-cbc -K $encrypted_2b7e0505597a_key -iv $encrypted_2b7e0505597a_iv -in .travis/deploy_rsa_encrypted -out ~/.ssh/id_rsa -d which is defined in the travis.yml file of this repository. Before jumping ahead to fix this, I would like to know why this is needed :)

Curly braces should not be used in PHTML files.

Bad:

<?php
if ($block->getFooList()) {
    foreach ($block->getFooList() as $foo) {
?>
        <!-- Template stuff -->
<?php
    }
}
?>

Better (Proposal 1):

<?php
if ($block->getFooList()):
    foreach ($block->getFooList() as $foo):
?>
        <!-- Template stuff -->
<?php
    endforeach;
endif;
?>

Better (Proposal 2):

<?php if ($block->getFooList()) ?>
    <?php foreach ($block->getFooList() as $foo): ?>
        <!-- Template stuff -->
    <?php endforeach; ?>
<?php endif; ?>

(Source: https://twitter.com/jissereitsma/status/990301162090975232)

Description

Related issues

1. extdn/extdn-phpcs#<issue_number>: Issue title
2. ...

Sniff checklist

• [ ] Sniff is accompanied by unit test
• [ ] Sniff is documented using the documentation template (FooSniff.md next to FooSniff.php)

Rule

MEQP Version 4 is released under new repo which not merged in extdn-phpcs

Reason

A lot of Magento 2 specific rules are added with severity 10 which should be added

Implementation

Merge https://github.com/magento/magento-coding-standard/

Description

Add package type to composer.json to allow the package to be automatically registered with PHPCS.

Please refer to https://github.com/Dealerdirect/phpcodesniffer-composer-installer/wiki/Change-%60composer.json%60-%22type%22-to-%60phpcodesniffer-standard%60 for more information.

Related issues

None.

Sniff checklist

N/A

Description

The sniff MEQP2.Templates.ThisInTemplate gets included. It checks for deprecated usage of $this in PHTML templates.

The Marketplace Extension Quality Program (MEQP2) has been added as composer requirement.

Sniff checklist

• [X] Sniff is accompanied by unit test
• [X] Sniff is documented using the documentation template (FooSniff.md next to FooSniff.php)

Description

If you follow the README.md instructions to set up the sniffs you get an error:

[~/projects/glovre-hosting/public_html] composer config repositories:extdn-phpcs vcs [email protected]:extdn/extdn-phpcs.git

  [InvalidArgumentException]
  Setting repositories:extdn-phpcs does not exist or is not supported by this command

I've amended the command.

Sniff checklist

None relevant, very tiny PR

Modules should not call home - either using an internal PHP call (CURL or file_get_open or something similar) or using a static asset. Not sure if this could be done using PHPCS.

Description

Bugfixes for edge cases in DeprecatedParentsSniff:

• Due to sniffs that evaluate code, a class may already be autoloaded, do not include the file in that case
• Do not throw PHP Fatal error: Uncaught Error: Call to a member function getName() on boolean if a block does not have a parent class

Description

Documentation for rule was still missing. See https://github.com/extdn/extdn-phpcs/issues/47

Related issues

1. extdn/extdn-phpcs#47: Document "strict types" rule

Rule

The AbstractSource model contains a protected attribute $_options that is meant to cache the options. Unfortunately this is not enforced, instead each implementation of getAllOptions() is responsible to use that cache properly.

Reason

EAV source models often load possible values for attributes from external resources. Every access to those values, e.g. with $product->getAttributeText() calls getAllOptions() on the source model. The result of this method should only be calculated once.

Implementation

Any class that extends the abstract source model should return $this->_options in getAllOptions()

Rule

Never Extend from Magento\Framework\App\Helper\AbstractHelper

Reason

The AbstractHelper has many dependy that you never need for your helper implementation. Also it breacks your helper if you use the ScopeConfigInterface and upgrade 2.1 to 2.2.

Implementation

After a call between various Magento people, @schmengler and myself, it was suggested that having too many PHPCS repositories is a bad thing. In short, we would be happy if some of our rules are merged into the Magento core or the MEQP ruleset. Another goal could be to minimize the chance of us and them working on the same kind of rules.

Shall we create new labels "Move to Magento core" and "Move to MEQP" because of this?

Description

In a production-ready extension, there shouldn't be any functions like die(), print_r(), var_export() or var_dump(). This PR adds a new rule that checks upon this. This is a very simple straightforward PHPCS implementation.

One thing that I foresee as an issue already is that I sometimes log things away by using var_export($x, true). I'm not sure if that's a lack of quality or simply a nifty feature.

Related issues

None

Sniff checklist

• [X] Sniff is accompanied by unit test
• [X] Sniff is documented using the documentation template