Fix binding of T of static when called from a class scope

[robchett]

Partial fix for #11368

Issue appears to cement itself here -

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php

Lines 418 to 421 in e052de4

	!$statements_analyzer->isStatic()
	&& (!$method_id || $method_id->method_name !== '__construct')
	? $context->self
	: null,

as context->self = "User" but the method was called on an instance of UserId

After updating to

 !$statements_analyzer->isStatic()
    && (!$method_id || $method_id->method_name !== '__construct')
    ? ($method_id ? $method_id->fq_class_name : $context->self)
    : null,

https://psalm.dev/r/a32e05de27

ERROR: ArgumentTypeCoercion - 19:33 - Argument 1 of UserId1::equals expects User&static, but parent type UserId1 provided

becomes

ERROR: ArgumentTypeCoercion - 19:33 - Argument 1 of UserId1::equals expects UserId1&static, but parent type UserId1 provided

Now, using self instead of static works

    /**
     * @template T of self
     * @param T $uuid
     */
    final public function equals($uuid): void {}

I've seen the "A&static, but parent type A" style messaging elsewhere and unsure if this is now a legitimate issue or not.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/a32e05de27

<?php

abstract class Uuid
{
    /**
     * @template T of static
     * @param T $uuid
     */
    final public function equals($uuid): void {}
}

class UserId1 extends Uuid {}
class UserId2 extends Uuid {}

class User
{
    public function equals(self $user): void
    {
        (new UserId1())->equals(new UserId2());
    }
}

Psalm output (using commit 514e954):

ERROR: ArgumentTypeCoercion - 19:33 - Argument 1 of UserId1::equals expects User&static, but parent type UserId2 provided

[robchett]

Ignore the above snippet - I pasted the wrong link. https://psalm.dev/r/a32e05de27

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/a32e05de27

<?php

abstract class Uuid
{
    /**
     * @template T of static
     * @param T $uuid
     */
    final public function equals($uuid): void {}
}

class UserId1 extends Uuid {}
class UserId2 extends Uuid {}

class User
{
    public function equals(self $user): void
    {
        (new UserId1())->equals(new UserId2());
    }
}

Psalm output (using commit 514e954):

ERROR: ArgumentTypeCoercion - 19:33 - Argument 1 of UserId1::equals expects User&static, but parent type UserId2 provided

[Guuzen]

I've seen the "A&static, but parent type A" style messaging elsewhere and unsure if this is now a legitimate issue or not.

Could you please elaborate?

[robchett]

Not sure how I managed to add the same link twice https://psalm.dev/r/1028cf9de2

The major issue is resolved by this PR - I'm unure if this secondary warning is a bug or expected as I've seen it in other places but have always resolved by changing the type definitions (I can't find any other examples to hand).

With this PR the error is:

ERROR: [ArgumentTypeCoercion] Argument 1 of UserId2::equals expects UserId2&static, but parent type UserId2 provided

In my mind UserId2&static == UserId2, but I may be mistaken

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/1028cf9de2

<?php

abstract class Uuid
{
    /**
     * @template T of static
     * @param T $uuid
     */
    final public function equals($uuid): void {}
}

class UserId1 extends Uuid {}
class UserId2 extends Uuid {}

class User
{
    public function equals(self $user): void
    {
        (new UserId2())->equals(new UserId2());
    }
}

Psalm output (using commit 1288d7a):

ERROR: ArgumentTypeCoercion - 19:33 - Argument 1 of UserId2::equals expects User&static, but parent type UserId2 provided

[Guuzen]

Well, I am not sure either. But to me it feels that you are right, they are equal.

I can also add, that it feels strange to use self instead of static, because in PHP self is about the class where code is defined and static is about the class of the object from which code is called. Self would mean that it is possible to pass any subtypes of Uuid which is undesirable.

Actually, I am not sure that our use case is valid (although it had been working till the recent change).
We can relatively easily change it to class level generic instead of method level generic and it will work just fine (except that we need to specify the type for each class). It is also possible to specify the bound for only Uuid types, so it feels like there is no problem.
Something like this https://psalm.dev/r/54e8166be5

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/54e8166be5

<?php

/**
 * @template T of Uuid
 */
abstract class Uuid
{
    /**
     * @param T $uuid
     */
    final public function equals($uuid): void {}
}

/**
 * @extends Uuid<UserId1>
 */
class UserId1 extends Uuid {}

/**
 * @extends Uuid<UserId2>
 */
class UserId2 extends Uuid {}

/**
 * @extends Uuid<Unrelated>
 */
class SomethingElse extends Uuid {}

class Unrelated {}

class User
{
    public function equals(self $user): void
    {
        (new UserId2())->equals(new UserId2());
    }
    
    public function failEquals(self $user): void
    {
        (new UserId1())->equals(new UserId2());
    }
}

Psalm output (using commit 1288d7a):

ERROR: InvalidTemplateParam - 27:7 - Extended template param T expects type Uuid, type Unrelated given

ERROR: InvalidArgument - 40:33 - Argument 1 of UserId1::equals expects UserId1, but UserId2 provided