Skip to content

PHPStan returns false positives because of incoherent PHPDocs on functions/parameters that accept Phrase objects #36063

Open
1 of 2 issues completed
@guvra

Description

@guvra

Preconditions and environment

  • Magento version: 2.4.5
  • Using PHPStan with the following conditions:
    • Level >= 5
    • Adding declare(strict_types=1); at the top of php files

Steps to reproduce

  1. At the root of your Magento project, create a file named Test.php with the following contents:
<?php

declare(strict_types=1);

namespace Foo\Bar\Controller;

use Magento\Framework\App\Action\Action;
use Magento\Framework\App\Action\HttpGetActionInterface;
use Magento\Framework\View\Result\Page;

class Test extends Action implements HttpGetActionInterface
{
    public function execute()
    {
        $this->messageManager->addErrorMessage(__('foobar'));

        return $this->resultFactory->create(Page::class);
    }
}
  1. Run PHPStan:
vendor/bin/phpstan analyse --level 6 -c dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/phpstan.neon Test.php

Expected result

PHPStan shouldn't report any error.

Actual result

PHPStan reports the following error:

Parameter #1 $message of method Magento\Framework\Message\ManagerInterface::addErrorMessage() expects string, Magento\Framework\Phrase given.

Additional information

To fix this issue, the typehints of parameters and methods that accept Phrase objects should be changed from string to string|Phrase.

A few examples:

In Magento\Backend\Block\Widget\Container:

    /**
     * @var string
     */
    protected $_headerText = 'Container Widget Header';

Must be changed to:

    /**
     * @var string|Phrase
     */
    protected $_headerText = 'Container Widget Header';

In Magento\Backend\Block\Widget\Button\ButtonList:

    /**
     * Update specified button property
     *
     * @param string $buttonId
     * @param string|null $key
     * @param string $data
     * @return void
     */
    public function update($buttonId, $key, $data)
    {

Must be changed to:

    /**
     * Update specified button property
     *
     * @param string $buttonId
     * @param string|null $key
     * @param string|Phrase $data
     * @return void
     */
    public function update($buttonId, $key, $data)
    {

In Magento\Framework\Message\ManagerInterface:

    /**
     * Adds new error message
     *
     * @param string $message
     * @param string|null $group
     * @return ManagerInterface
     */
    public function addErrorMessage($message, $group = null);

Must be changed to:

    /**
     * Adds new error message
     *
     * @param string|Phrase $message
     * @param string|null $group
     * @return ManagerInterface
     */
    public function addErrorMessage($message, $group = null);

(same for the other addXxX methods of the same class).

In Magento\Framework\View\Page\Title:

    /**
     * Set page title
     *
     * @param string $title
     * @return $this
     */
    public function set($title)
    {

Must be changed to:

    /**
     * Set page title
     *
     * @param string|Phrase $title
     * @return $this
     */
    public function set($title)
    {

Other option: add Stringable interface to the phrase class and use string|Stringable.

Release note

No response

Triage and priority

  • Severity: S0 - Affects critical data or functionality and leaves users without workaround.
  • Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
  • Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
  • Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
  • Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

Sub-issues

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: FrameworkComponent: ObjectManagerIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedPriority: P3May be fixed according to the position in the backlog.Progress: ready for devReported on 2.4.5Indicates original Magento version for the Issue report.Reproduced on 2.4.xThe issue has been reproduced on latest 2.4-develop branchTriage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions