Add magento/magento-coding-standard as a dependency to magento/magento2

[lenaorobei]

Description

This unified coding standard will contain all Magento related sniffs, so there is no need to keep core sniffs under Magento 2 dev/tests/static folded.

Acceptance Criteria

1. Sniffs are removed from Magento 2 dev/tests/static/framework/Magento folder.
2. magento/magento-coding-standard depencency is added to the require-dev section of Magento 2 composer.json file.
3. No duplicate findings detected by other static tools (eg: Magento\TestFramework\Utility\XssOutputValidatorTest covers the same as XssTemplateSniff, FinalImplementation Mess Detector rule and FinalImplementation sniff).
4. Static build is green.

Additional information

Needs to be done after the first Magento Coding Standard release.

[schmengler]

Magento\TestFramework\Utility\XssOutputValidator covers the same as XssTemplateSniff

are those static tests also going to be moved into this repository? More importantly, are they used for marketplace extensions?

[lenaorobei]

@schmengler PHP CodeSniffer should became preferred tool for static checks. If its functionality is not enough (dynamic check, etc), we should consider using other approaches like Mess Detector or PHPUnit. For example, I rewrite FinalImplementation Mess Detector rule with FinalImplementation sniff because for this case PHP CodeSniffer is good enough.

More importantly, are they used for marketplace extensions?

Not yet, but in future we want to make all static tests (not only sniffs) consistent for Marketplace and core development

[schmengler]

Sounds good. That means, Magento\TestFramework\Utility\XssOutputValidatorTest will be removed in favor of XssTemplateSniff, right?

[lenaorobei]

@schmengler yes, that's right.

[orlangur]

@lenaorobei,

For example, I rewrite FinalImplementation Mess Detector rule with FinalImplementation sniff because for this case PHP CodeSniffer is good enough.

Well, you missed the Design part. That was the whole point. It has nothing to do with Php.

[orlangur]

https://github.com/magento/magento2/blob/2.3-develop/composer.json#L88 this needs to be fixed a little bit: dev version of Magento must rely on dev version of coding standard so that we can introduce new rules, fix code and immediately enforce it.

Concrete version must be added during publication: https://github.com/magento/magento2/blob/2.3.1/composer.json#L99

Also, it would be really good to not introduce mess and use the same versioning as for other Magento components.

[lenaorobei]

@orlangur thanks for your feedback, however I don't see any reasons to

use same versioning as for other Magento components

because Codings Standard is not a Magento component (module). New rules may be added to develop (master) branch and will cause static build failure. By following SemVer we will explicitly let our application know about braking changes.

Here is my proposal for Coding Standard versioning strategy. If you have any concerns regarding this do not hesitate to reflect them there magento/architecture#136

[lenaorobei]

Done https://github.com/magento/magento2/blob/2.3-develop/composer.json#L88

[orlangur]

@lenaorobei,

because Codings Standard is not a Magento component (module)

Which is pretty wrong.

New rules may be added to develop (master) branch and will cause static build failure. By following SemVer we will explicitly let our application know about braking changes.

Yep, this is what I was talking about and such use cases needed to be covered PRIOR to changes applied to main repo.

Here is my proposal for Coding Standard versioning strategy

Thanks, I'll share my thoughts that we don't need to reinvent the wheel and introduce ONE MORE versioning strategy. Will strive to visiting corresponding arch council meeting as well.