Psalm 5: TypeDoesNotContainType is thrown instead of DocblockTypeContradiction

[VincentLanglet]

The following example is throwing a TypeDoesNotContainType with Psalm 5 but not with 4
https://psalm.dev/r/9950912ce2

This seems related to the union type since
https://psalm.dev/r/66b46ed7a0
works fine.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/9950912ce2

<?php

declare(strict_types=1);

/*
 * This file is part of the Sonata Project package.
 *
 * (c) Thomas Rabaix <thomas.rabaix@sonata-project.org>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Sonata\AdminBundle\Form\DataTransformer;

use Sonata\AdminBundle\Filter\Model\FilterData;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\UnexpectedTypeException;

final class FilterDataTransformer
{
    /**
     * @param array|null $value
     */
    public function reverseTransform($value): null
    {
        if (null === $value) {
            return null;
        }

        if (!\is_array($value)) {
            throw new \Exception('array');
        }

        return null;
    }
}

Psalm output (using commit b6faa3e):

ERROR: TypeDoesNotContainType - 31:14 - Type array<array-key, mixed> for $value is always !array<array-key, mixed>

https://psalm.dev/r/66b46ed7a0

<?php

declare(strict_types=1);

/*
 * This file is part of the Sonata Project package.
 *
 * (c) Thomas Rabaix <thomas.rabaix@sonata-project.org>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Sonata\AdminBundle\Form\DataTransformer;

use Sonata\AdminBundle\Filter\Model\FilterData;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\UnexpectedTypeException;

final class FilterDataTransformer
{
    /**
     * @param array $value
     */
    public function reverseTransform($value): null
    {
        if (null === $value) {
            return null;
        }

        if (!\is_array($value)) {
            throw new \Exception('array');
        }

        return null;
    }
}

Psalm output (using commit b6faa3e):

ERROR: DocblockTypeContradiction - 27:13 - array<array-key, mixed> does not contain null

ERROR: DocblockTypeContradiction - 31:14 - Docblock-defined type array<array-key, mixed> for $value is always array<array-key, mixed>

[pilif]

given the type of the $value parameter (array or null), after the null check, I expert $value to always be array and thus the check to be redundant, so I don't feel like this is a bug

[VincentLanglet]

given the type of the $value parameter (array or null), after the null check, I expert $value to always be array and thus the check to be redundant, so I don't feel like this is a bug

I agree it's redundant but i expect it to be redundant with the PHPDoc, and not a "native type".
That's why I expect a different type of error.

This should be:

    public function reverseTransform(array|null $value): null
    {
        if (null === $value) {
            return null;
        }

        if (!\is_array($value)) { // TypeDoesNotContainType

/**
     * @param array|null $value
     */
    public function reverseTransform($value): null
    {
        if (null === $value) {
            return null;
        }

        if (!\is_array($value)) { // DocblockTypeContradiction

That's why I gave the second example (without the union).
The is_array check is redundant, but is reported as a DocblockTypeContradiction ; the behavior I want.

This is something important for open-source library.
When type cannot be added for some reason, we cannot consider the PHPdoc as enough and need to add extra-safety check with specific message. So I generally use treatPhpdocTypeAsCertain: false on phpstan and ignore DocblockTypeContradiction on psalm.

[orklah]

WIP: https://github.com/vimeo/psalm/compare/master...orklah:psalm:from_docblock?expand=1

[VincentLanglet]

WIP: https://github.com/vimeo/psalm/compare/master...orklah:psalm:from_docblock?expand=1

I added a test about this change.

PR is #8999

[VincentLanglet]

https://psalm.dev/r/76264cec79 is working as expected, so I don't think it's an issue with the MutableUnion but more when parsing the Type.

With some dump, I get that the type is an Union of
Tarray (from_docblock=false) and TNull (from_docblock=true).

But the TArray should be from_docblock=true, too.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/76264cec79

<?php

declare(strict_types=1);

/*
 * This file is part of the Sonata Project package.
 *
 * (c) Thomas Rabaix <thomas.rabaix@sonata-project.org>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Sonata\AdminBundle\Form\DataTransformer;

use Sonata\AdminBundle\Filter\Model\FilterData;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\UnexpectedTypeException;

final class FilterDataTransformer
{
    /**
     * @param array|null $value
     */
    public function reverseTransform($value): null
    {
        if (\is_array($value)) {
            return null;
        }

        if ($value !== null) {
            throw new \Exception('array');
        }

        return null;
    }
}

Psalm output (using commit 58ae748):

ERROR: DocblockTypeContradiction - 31:13 - Docblock-defined type null for $value is always null