Better support for `static<T>`

[gharlan]

https://psalm.dev/r/b9d50b9872

It would be nice if psalm could fail here, because static<bool> does not match C2 extends C1<string>.
(But I don't know how difficult that would be to implement.)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/b9d50b9872

<?php

/** @template T */
abstract class C1 {
	/** @param T $value */
    final public function __construct(private $value) {}
    
    /**
     * @template TFrom
     * @param TFrom $value
     * @return static<TFrom>
     */
    public static function from($value): static {
    	return new static($value);
    }
}

/** @extends C1<string> */
class C2 extends C1 {}

C2::from(false);

Psalm output (using commit b549989):

No issues!

[muglug]

No idea how to solve this offhand.

[weirdan]

If static method had access to containing class type parameters we could probably require $value to have TFrom&T type or flag new static($value) call.

[muglug]

Yeah, thinking about it more we probably need to prohibit new static inside any templated class. Should be a simple fix.

[gharlan]

Yeah, thinking about it more we probably need to prohibit new static inside any templated class.

Hmm, I think using new static is a common pattern, e.g. in myclabs/php-enum.
I know that new static is problematic because of child constructor params, but for this we already have UnsafeInstantiation. I'm not sure if new static should really be prohibited in all templated classes.

If static method had access to containing class type parameters

This would be cool, then I would write it like this: https://psalm.dev/r/f42ebc781f
It would also help here: myclabs/php-enum#143

But I think I understand why template variables are coupled to instances only.
Theoretically, it could maybe be possible to enforce the base constraints of the template variables in static methods? I'm not sure.
But when calling the static method on a subclass, the overwritten constraint for the template variable should be used.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/f42ebc781f

<?php

/** @template T */
abstract class C1 {
	/** @param T $value */
     public function __construct(private $value) {}
    
    /**
     * @param T $value
     * @return static<T>
     */
    public static function from($value): static {
    	return new static($value);
    }
}

/** @extends C1<string> */
class C2 extends C1 {}

C2::from(false);

Psalm output (using commit f1a8407):

ERROR: PossiblyFalseArgument - 20:10 - Argument 1 of C2::from cannot be false, possibly false value provided

ERROR: UndefinedDocblockClass - 9:15 - Docblock-defined class or interface T does not exist

INFO: MixedInferredReturnType - 10:16 - Could not verify return type 'C1<T>' for C1::from

[muglug]

Hmm, I think using new static is a common pattern

Yeah, people currently do it, but it's still unsound (as you demonstrate)

There's a fundamental difference between C1&static and C2 whenever C2 extends with a parameter either explicitly or implicitly, and that's the underlying problem.

The alternative would be to force C2 to implement its own version of from: https://psalm.dev/r/f4989147d8, but this would be more noisy (creating an issue for every class extending the base one).

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/f4989147d8

<?php

/** @template T */
abstract class C1 {
	/** @param T $value */
    final public function __construct(private $value) {}
    
    /**
     * @template TFrom
     * @param TFrom $value
     * @return static<TFrom>
     */
    public static function from($value): static {
    	return new static($value);
    }
}

/** @extends C1<string> */
final class C2 extends C1 {
    /**
     * @param mixed $value
     * @return C2
     */
    public static function from($value): self {
        return new self((string) $value);
    }
}

C2::from(false); // now safe

Psalm output (using commit f1a8407):

No issues!

[muglug]

I don't think it's too burdensome to tell the maintainer of myclabs/php-enum that the code is theoretically unsound, even if it's practically safe.