[New Rule] Class instantiation via new keyword

[lenaorobei]

Description

Magento2.Classes.ObjectInstantiation Detects direct object instantiation via new keyword.

The test run of Magento2.Classes.ObjectInstantiation rule againstMagento2 codebase found >2000 issues. Some of them look like false-positive. Examples:

Direct Phrase object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \DOMXPath object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \DOMDocument object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \DateTime object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \Magento\Framework\File\Uploader object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \Zend_Validate_Alnum object instantiation is discouraged in Magento. Use dependency injection or factories instead. 
Direct \Zend_Validate_Alpha object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \Zend_Db_Expr object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct \NumberFormatter object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct DataObject object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct InputOption object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct InputArgument object instantiation is discouraged in Magento. Use dependency injection or factories instead.
Direct Table object instantiation is discouraged in Magento. Use dependency injection or factories instead.

Problem

Is there any cases when object instantiation via new keyword is allowed in Magento?

[lenaorobei]

As per architecture discussion instantiation via new keyword is ok for:

• factories;
• 3rd-party code (not the same namespace the checked file is).

Discuss the best way to implement this rule. Looks like PHPCodeSniffer is not suitable here.

[larsroettig]

For unit tests, factory, facade, builder pattern must are new allowed.

[maxbucknell]

I don’t often see exceptions created by factories. Not sure whether that is the current best practice, though.

[lenaorobei]

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

[buskamuza]

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.

[buskamuza]

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.