Inconsistent generic beheviour on optional generics

[veewee]

This code best describes it:

https://psalm.dev/r/f86eb62c76

/**
 * @template A
 */
class Config {
    /**
     * @param (callable(): A)|null $x
     */
    public function __construct($x = null)
    {}

    /**
     * @template NewA
     * @param (callable(): NewA)|null $x
     * @return Config<NewA>
     */
    public static function create($x = null)
    {
        return new self($x);
    }
}

// Named constructor Tests:
/** @return Config<never>*/
function namedEmptyConfig() {
    return Config::create();
}

/**
 * This somehow resolves to mixed instead of never - even though the value is also null here:
 * @return Config<never>
 */
function namedNullConfig() {
    return Config::create(null);
}


// Test config constructor:
/**
 * @return Config<never>
 * Why does this work differently than the named constructor?
 * I would expect "never" to be the result here, but instead it is mixed.
 */
function emptyConfig() {
    return new Config();
}

/**
 * @return Config<never>
 * Same issue as in named constructor test - I'dd expect never here.
 */
function nullConfig() {
    return new Config(null);
}

/** @return Config<string> */
function configured() {
    return new Config(fn () => 'hello');
}

I noticed this error when wrapping in yet another layer which applies conditional love to this config. Something along the lines of

@return ConfigA is empty ? Foo : Bar

It never resolves to Foo, since it is not empty. If I change it to: ConfigA is mixed it always resolves to Foo

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/f86eb62c76

<?php

/**
 * @template A
 */
class Config {
    /**
     * @param (callable(): A)|null $x
     */
    public function __construct($x = null)
    {}

    /**
     * @template NewA
     * @param (callable(): NewA)|null $x
     * @return Config<NewA>
     */
    public static function create($x = null)
    {
        return new self($x);
    }
}

// Named constructor Tests:
/** @return Config<never>*/
function namedEmptyConfig() {
    return Config::create();
}

/**
 * This somehow resolves to mixed instead of never - even though the value is also null here:
 * @return Config<never>
 */
function namedNullConfig() {
    return Config::create(null);
}


// Test config constructor:
/**
 * @return Config<never>
 * Why does this work differently than the named constructor?
 * I would expect "never" to be the result here, but instead it is mixed.
 */
function emptyConfig() {
    return new Config();
}

/**
 * @return Config<never>
 * Same issue as in named constructor test - I'dd expect never here.
 */
function nullConfig() {
    return new Config(null);
}

/** @return Config<string> */
function configured() {
    return new Config(fn () => 'hello');
}

Psalm output (using commit d957ff2):

INFO: MixedReturnTypeCoercion - 35:12 - The type 'Config<mixed>' is more general than the declared return type 'Config<never>' for namedNullConfig

INFO: MixedReturnTypeCoercion - 32:12 - The declared return type 'Config<never>' for namedNullConfig is more specific than the inferred return type 'Config<mixed>'

INFO: MixedReturnTypeCoercion - 46:12 - The type 'Config<mixed>' is more general than the declared return type 'Config<never>' for emptyConfig

INFO: MixedReturnTypeCoercion - 41:12 - The declared return type 'Config<never>' for emptyConfig is more specific than the inferred return type 'Config<mixed>'

INFO: MixedReturnTypeCoercion - 54:12 - The type 'Config<mixed>' is more general than the declared return type 'Config<never>' for nullConfig

INFO: MixedReturnTypeCoercion - 50:12 - The declared return type 'Config<never>' for nullConfig is more specific than the inferred return type 'Config<mixed>'

[AndrolGenhald]

There's definitely something weird going on here if it's sometimes never and sometimes mixed. If Psalm has no way to infer the template type, I would think it should always use the constraint type (or mixed in case there is no constraint).

[veewee]

(or mixed in case there is no constraint).

Mixed is not the correct type here. It should be never, since it will never contain a type. That's the only way you can correctly use conditional types on it:T is never ? Foo : Bar If it were mixed, you always get Foo with T is mixed ? Foo : Bar

I even noticed today that the named constructor doesn't consistently results in never

[AndrolGenhald]

Hmm, maybe you're right. I don't think this use case was accounted for when templates were implemented though, so I'm not terribly surprised that it doesn't work.

[veewee]

I can imagine :) For reference. This is what typescript makes of it:

https://www.typescriptlang.org/play?#code/C4TwDgpgBAwg9gOwGYEsDmAeAggPigXigG8AoKcqAJwgEMATRAGxCgA8oB+ALigAoBKAniwkSAXwDcogMaIAzsCgQAtmFABVBAGs4AdwQFikkrIQKlq0ADkIANwiUe8ZOgwI7DvISLHT5lWogWAggToiomDQhXkZSfoqmEQCu1HSGRKw8AkJQAOQAFhCMjHC5xiRISQjSwCiIVFXYOLyJ6JxhLpi4-CSkFFQQwCkGrWgcAHTsHFCjk9k8ubpwlIx0uVJioibyitRySYzAAKKWIJo6+oaUVbwBGtp6CPxxOwP7hyeBNvaUVzd3IG+Dme2zMuwg72Op2CLEI1wQt2hIRB8TeB2AzmSqT+CNGKQgdBBJCAA

They fall back to the unkown type by default. But using the never type, you get better results in conditional returns. Both the unkown and any type are less optimal as a default IMO.

Let me know if there is anything else I can do here to make your life easier. I have looked at the source-code for templates in psalm, but it's a bit all over the place for me ATM :)

[aszenz]

Faced a similar issue here:

https://psalm.dev/r/765fa2e560

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/765fa2e560

<?php

declare(strict_types=1);

/**
 * @template-covariant T
 */
class Ok
{
    /** @param T $v */
    public function __construct(
        public mixed $v
    ) {
    }
}

class Err
{
    public function __construct(
        public string $e
    ) {
    }
}

/**
 * @template-covariant V
 */
class Result
{
    /** @param Ok<V>|Err $val */
    private function __construct(
        public Ok|Err $val
    ) {
    }

    /**
     * @template OkType
     *
     * @param OkType $item
     *
     * @return self<OkType>
     */
    public static function ok($item): self
    {
        return new self(new Ok($item));
    }

    /**
     * @return self<never>
     */
    public static function err(string $error): self
    {
        return new self(new Err($error));
    }
}

/**
 * @template-covariant T
 */
class Decoder
{
    /**
     * @param \Closure(mixed): Result<T> $fn
     */
    public function __construct(
        public Closure $fn
    ) {
    }

    /**
     * @psalm-return self<bool>
     */
    public static function bool(): self
    {
        return new self(
            function (mixed $value): Result {
                if (!is_bool($value)) {
                    return Result::err('Not a bool value');
                }

                return Result::ok($value);
            }
        );
    }
}

Psalm output (using commit 028ac7f):

INFO: MixedReturnTypeCoercion - 53:16 - The type 'Result<mixed>' is more general than the declared return type 'Result<never>' for Result::err

INFO: MixedReturnTypeCoercion - 49:16 - The declared return type 'Result<never>' for Result::err is more specific than the inferred return type 'Result<mixed>'

[veewee]

In case someone want to play around with this, I made a faling test-suite on how I would see the inferred results here: https://gist.github.com/veewee/3624bd80a4c9d28c4048a8cf51eeb4d5

[veewee]

Having the possibility to set default types on templates could be the solution for these kind of issues:

E.g. PHPStan: https://github.com/phpstan/phpdoc-parser/pull/148