Advice wanted about best practice

[LauHypershop]

Hi,

An issue I run into occasionally is that it's pretty common in Magento for a concrete class to implement a lot of methods not covered by its interface.

For example, we're usually pretty confident that a Product class is going to have a getData() method, and the \Magento\Catalog\Model\Product uses it fairly extensively. But the \Magento\Catalog\Api\Data\ProductInterface doesn't list that method. Any time you fetch a product from a ProductRepositoryInterface, PHPStan is going to complain that this method doesn't exist.

What's a good way of handling this? I can put an ignoreErrors in my phpstan.neon like this:

- '#Call to an undefined method Magento\\Catalog\\Api\\Data\\ProductInterface::getData\(\).#'

However, this is going to lead to a fairly bloated file. It's also awkward to scale across multiple projects, to keep that list consistent as you discover more examples. Is there a better way to go about it?

[shochdoerfer]

I feel you :)

Magento's handling of inheritance and interfaces makes static code analysis a bit more difficult than it should be. I would for sure avoid ignoring those errors in the PHPStan config. It's better to give PHPStan more information about the types you are dealing with. This is what you can do instead:

1. Use PHPStan Union types to expand the ProductInterface by adding a comment like this:

/** @var \Magento\Framework\DataObject&\Magento\Catalog\Api\Data\ProductInterface $object */

2. Use comments to switch between types, which is a bit annoying as your code gets bloated with unnecessary comments, but at least it works:

foreach ($products as $product)
{
    /** @var \Magento\Catalog\Api\Data\ProductInterface $product */
    $product->getSku();

    /** @var \Magento\Framework\DataObject $product */
    $product->getData('test');
}

3. Extend the ProductInterface and add the methods that you need to call but then you would also need the code comments to "trick" PHPStan into thinking it deals with "your" custom ProductInterface and not the one defined by Magento core.

Does that make sense to you?

[shochdoerfer]

@LauHypershop did my answer help you? Does it make sense to maintain an FAQ section to answer questions like yours?

[LauHypershop]

Well it's nice to know I'm not the only one struggling with this.

I was thinking of trying to write an extension to PHPStan where you could add notes to your PHPStan config to let it know that one interface/class actually also implies/extends another one. For example, that you can configure that if a method can't be found in the ProductInterface, that PHPStan should also look for the method in the DataObject class.

This isn't strictly orthodox OO of course, but then neither is Magento perfect. And any sane implementation of a ProductInterface is going to end up extended that DataObject. So this would reduce the amount of noise in your scans considerably.

Would you be open to including that in this extension, if I can get it working?

[shochdoerfer]

I'd be open to that idea if you agree to help maintain it.

[LauHypershop]

That's reasonable. I'm hoping to dig into this in a couple of weeks after holidays.