magento / magento-coding-standard

Magento Coding Standard
Open Software License 3.0
349 stars 153 forks source link

RestrictedCodeSniff suggests Laminas\Filter\FilterInterface, which is not required by Magento 2.4.5-p1 anymore #428

Open norgeindian opened 2 years ago

norgeindian commented 2 years ago

Preconditions

  1. Magento 2.4.5-p1
  2. Magento Coding Standard, 26

Steps to reproduce

  1. Create a file, where you include \Zend_Filter_Interface in the constructor
  2. Run vendor/bin/phpcs --standard=dev/tests/static/framework/Magento on that file.

Expected result

  1. RestrictedCodeSniff should suggest a class, which exists in a clean Magento instance

Actual result

  1. The following error comes up: ERROR | Class 'Zend_Filter_Interface' is restricted in XXX. Suggested replacement: Laminas\Filter\FilterInterface
m2-assistant[bot] commented 2 years ago

Hi @norgeindian. Thank you for your report. To speed up processing of this issue, make sure that you provided sufficient information.

Add a comment to assign the issue: @magento I am working on this


ihor-sviziev commented 1 year ago

Hi @norgeindian, Could you please provide us an example of such behavior, and what would you expect?

norgeindian commented 1 year ago

@ihor-sviziev , my class looks like this:

use Magento\Framework\View\Element\Template;
use Magento\Framework\View\Element\Template\Context;
use Zend_Filter_Interface;

class Top extends Template
{
    protected Zend_Filter_Interface $templateProcessor;

    public function __construct(
        Context $context,
        Zend_Filter_Interface $templateProcessor,
        array $data = []
    ) {
        $this->templateProcessor = $templateProcessor;
        $this->helper            = $helper;
        parent::__construct($context, $data);
    }

Like I said, as soon as I run vendor/bin/phpcs --standard=dev/tests/static/framework/Magento I get the following error message:

ERROR | Class 'Zend_Filter_Interface' is restricted in XXX. Suggested replacement: Laminas\Filter\FilterInterface

But since Magento 2.4.5-p1, Laminas is not included anymore by the Magento Core. So I would expect to get a suggestion about a model, which is actually included and can be used.

ihor-sviziev commented 1 year ago

@norgeindian the laminas filter is actually included in Magento 2.4.6 and 2.4.6-p1, and your custom extension can also add to requreiments the package you need https://github.com/magento/magento2/blob/2.4.6-p1/composer.json#L56 https://github.com/magento/magento2/blob/2.4.6/composer.json#L56

norgeindian commented 1 year ago

@ihor-sviziev , thanks for pointing that out, but it's not included in any 2.4.5 version (see https://github.com/magento/magento2/blob/2.4.5-p4/composer.json). This is why this hint is misleading for these versions. Do you see any good solution for that in general? Maybe the coding standard should then be required in a fixed version in the root composer.json of magento/project-community-edition?

norgeindian commented 1 year ago

@ihor-sviziev , what's your opinion here?

ihor-sviziev commented 1 year ago

@norgeindian in my opinion, if your extension depends on some laminas library, it should declare such dependence in require section. Otherwise you'll see such errors. I don't think this should be somehow accepted to coding standard, not through dependency (which coding standard don't have) not through additional sniff.