magento / marketplace-eqp

Magento 1.x Coding Standard
http://docs.magento.com/marketplace/user_guide/Resources/pdf/Extension_Quality_Program_Overview.pdf
MIT License
224 stars 68 forks source link

False positive: proxies declared in di.xml files #110

Closed guvra closed 5 years ago

guvra commented 5 years ago

The MEQP2 ruleset doesn't detect proxies that are declared in di.xml files. This results in false positives when proxies are declared using this convention.

Affected rule: Rule: MEQP2.Classes.MutableObjects.MutableObjects

Sample di.xml file:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <type name="Foo\Bar\Baz\ProxyTest">
        <arguments>
            <argument name="checkoutSession" xsi:type="object">Magento\Checkout\Model\Session\Proxy</argument>
        </arguments>
    </type>
</config>

Sample code:

namespace Foo\Bar\Baz;

use Magento\Checkout\Model\Session as CheckoutSession;

class ProxyTest
{
    private $checkoutSession;

    public function __construct(CheckoutSession $checkoutSession)
    {
        $this->checkoutSession = $checkoutSession;
    }
}

Expected result: No warning.

Actual result:

FILE: app/code/Foo/Bar/Baz/ProxyTest.php
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 10 | WARNING | Session object MUST NOT be requested in constructor. It can only be passed as a method argument. If you are working with sessions, consider using Proxies.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
lenaorobei commented 5 years ago

@guvra thanks for reporting. You're absolutely right about this rule. Now we are working on consolidated coding standard magento/magento-coding-standard and it doesn't contain this rule.

We are working on investigation more appropriate tool for such kind of checks.

lenaorobei commented 5 years ago

Closing this issue because this repo now contains sniffs for Magento 1.x code only. Please refer to magento/magento-coding-standard for Magento 2.x coding standard.

This rule was removed in magento/magento-coding-standard in order to eliminate false-positive findings.