Open lenaorobei opened 5 years ago
As per architecture discussion instantiation via new
keyword is ok for:
Discuss the best way to implement this rule. Looks like PHPCodeSniffer is not suitable here.
For unit tests, factory, facade, builder pattern must are new allowed.
I don’t often see exceptions created by factories. Not sure whether that is the current best practice, though.
Going to remove this rule for the first release of Magento Coding Standard. Will keep the issue open though.
Need to find out the best way to implement this rule. Looks like PHPCodeSniffer is not the best option to perform such kind of checks.
Twitter thread: https://twitter.com/LenaOrobei/status/1108402289222602752
@paliarush @buskamuza @kandy @melnikovi
Looks like, first we should update our guidelines with the rules around objects instantiations with reasoning. And the the cs rule should cover it by the automation.
Please see https://github.com/magento/devdocs/pull/3982/files Let's have a discussion what we want as a rule. It should have good balance of value/overhead.
Description
Magento2.Classes.ObjectInstantiation
Detects direct object instantiation vianew
keyword.The test run of
Magento2.Classes.ObjectInstantiation
rule againstMagento2 codebase found >2000 issues. Some of them look like false-positive. Examples:Problem
Is there any cases when object instantiation via
new
keyword is allowed in Magento?