sonata-project / SonataBlockBundle

Symfony SonataBlockBundle
https://docs.sonata-project.org/projects/SonataBlockBundle
MIT License
413 stars 142 forks source link

Change typehint to BlockInterface #1093

Closed jordisala1991 closed 2 years ago

jordisala1991 commented 2 years ago

Subject

I am targeting this branch, because this is BC.

I don't know why, but psalm complains if I don't change this line. Here are 2 examples:

ERROR: InvalidArgument - src/Entity/Transformer.php:182:30 - Argument 1 of addChild expects static, Sonata\PageBundle\Model\PageBlockInterface provided (see https://psalm.dev/004)
            $block->addChild($this->loadBlock($child, $page));
ERROR: InvalidArgument - src/Entity/BlockInteractor.php:145:61 - Argument 1 of addChild expects static, Sonata\PageBundle\Model\PageBlockInterface provided (see https://psalm.dev/004)
            $blocks[$block->getParent()->getId()]->addChild($block);

With this changes, Psalm is happy (I think we will need to add checks with 5.x branch for PageBundle.

Changelog

### Changed
- Change typehint of `addChild` to `BlockInterface` instead of `self`.
VincentLanglet commented 2 years ago

We have

public function addChildren(self $children): void;

and I assume we will have the same for addChild() in 5.x.

I think we should try the 5.x branch on PageBundle to see if the issue will be here. If not, it's a psalm issue with @method.

jordisala1991 commented 2 years ago

It works with 5.x. It is probably a problem with psalm @method annotation. Even thought it should be reported to psalm, isn't it better to fix it here and not get blocked?

VincentLanglet commented 2 years ago

It works with 5.x. It is probably a problem with psalm @method annotation. Even thought it should be reported to psalm, isn't it better to fix it here and not get blocked?

Sure, I just wanted to be sure we won't have the issue on 5.x