hhvm icon indicating copy to clipboard operation
hhvm copied to clipboard

Unsound behaviour allowed with return static

Open muglug opened this issue 4 years ago • 4 comments

<<__ConsistentConstruct>>
abstract class C1<T> {
    final public function __construct(private T $value) {}

    public static function from(mixed $value): this {
        return new static($value);
    }

    public function getValue() : T {
        return $this->value;
    }
}

final class C2 extends C1<string> {}

<<__EntryPoint>>
function main() : string {
    return C2::from(10)->getValue();
}

Expected behavior

Some sort of typechecker error maybe?

Actual behavior

No error (but running this produces a TypeError)

Environment

hhvm 4.100

Would be very interested to know how this might be handled. Related issue here: https://github.com/vimeo/psalm/issues/5383

muglug avatar Mar 14 '21 01:03 muglug

I think the only way to prevent this is to prohibit new static for all generic classes

muglug avatar Mar 14 '21 03:03 muglug

Thanks - FB T87049790

fredemmott avatar Mar 18 '21 23:03 fredemmott

On T87049790, @dlreeves wrote:

Hmm we have known about this unsoundness at least since when we introduced reified generics, which is why we banned using new static in this way if T is a reified generic. Haven’t tested this yet, but I feel this particular unsoundness would go away if we used a where constraint in some way. Something like where this as C<mixed>. Then at the call site we will initiate the this type to be C2 which would fail the constraint C2 <: C and so we wouldn’t allow the method to be invoked.

If that does work then the trick now is figuring out how to force this constraint when declaring the method. Don’t have a great idea for that currently.

jthemphill avatar Apr 01 '21 20:04 jthemphill

FYI I solved this in Psalm by introducing a @psalm-consistent-templates annotation that works similarly to @psalm-consistent-constructor (which itself duplicates the behaviour of <<__ConsistentConstruct>>).

Using it forces all child classes to have the exact same template params with the same constraints and requires them to map directly to the parent params. new static is then prohibited in any function whose signature indicates it returns something involving static where that annotation is not also present on the class.

<<__ConsistentConstruct>>
<<__ConsistentGeneric>>
abstract class C1<T> {
    final public function __construct(private T $value) {}

    public static function from(mixed $value): this {
        return new static($value); // would be allowed
    }

    public function getValue() : T {
        return $this->value;
    }
}

final class C2 extends C1<string> {} // fail

final class C3<T3, TOther> extends C1<T3> {} // fail

final class C4<T4> extends C1<mixed> {} // fail

final class C5<T5 as object> extends C1<T5> {} // fail

final class C6<T6> extends C1<T6> {} // pass

muglug avatar Apr 10 '21 17:04 muglug