Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImplementedReturnTypeMismatch on template trait abstract method #6937

Closed
ptomulik opened this issue Nov 17, 2021 · 6 comments · Fixed by #6963
Closed

ImplementedReturnTypeMismatch on template trait abstract method #6937

ptomulik opened this issue Nov 17, 2021 · 6 comments · Fixed by #6963

Comments

@ptomulik
Copy link
Contributor

ptomulik commented Nov 17, 2021

Two template traits used by a class, one trait declares an abstract method, other implements it. When used, the ImplementedReturnTypeMismatch error is reported.

https://psalm.dev/r/07c75f40bc

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/07c75f40bc
<?php
/**
 * @psalm-template T
 */
trait AggregateTrait {
    /**
     * @psalm-readonly
     * @var T
     */
    private $value;
    /**
     * @psalm-return T
     */
    public function getValue()
    {
        return $this->value;
    }
}

/**
 * @psalm-template T
 */
trait AnotherTrait {
    /**
     * @psalm-return T
     */
    abstract public function getValue();
}

/**
 * @psalm-template T
 */ 
class Test {
    /**
     * @template-use AggregateTrait<T>
     */
    use AggregateTrait; 
    /**
     * @template-use AnotherTrait<T>
     */
    use AnotherTrait;
    
    /**
     * @psalm-param T $value
     */
    public function __construct($value) {
        $this->value = $value;
    }
}
Psalm output (using commit aabd96c):

ERROR: ImplementedReturnTypeMismatch - 12:22 - The inherited return type 'T:Test as mixed' for AnotherTrait::getValue is different to the implemented return type for AggregateTrait::getvalue 'T:AggregateTrait as mixed'

@weirdan
Copy link
Collaborator

weirdan commented Nov 17, 2021

Simplified and condensed a bit: https://psalm.dev/r/d6d08bda2f

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/d6d08bda2f
<?php
/** @psalm-template T */
trait AggregateTrait {
    /** @var T */
    private $value;
    
    /** @psalm-return T */
    public function getValue() {
        return $this->value;
    }
}

/** @psalm-template T */
trait AnotherTrait {
    /** @psalm-return T */
    abstract public function getValue();
}

class Test {
    /** @template-use AggregateTrait<int> */
    use AggregateTrait; 
    
    /** @template-use AnotherTrait<int> */
    use AnotherTrait;
    
    public function __construct() {
        $this->value = 123;
    }
}
Psalm output (using commit aabd96c):

ERROR: LessSpecificImplementedReturnType - 7:23 - The inherited return type 'int' for AnotherTrait::getValue is more specific than the implemented return type for AggregateTrait::getvalue 'T:AggregateTrait as mixed'

@ptomulik
Copy link
Contributor Author

I'd like to try to fix this, where shall I start?

@weirdan
Copy link
Collaborator

weirdan commented Nov 18, 2021

I usually start with running Psalm on the failing snippet with --debug-emitted-issues --threads=1 --no-cache flags. It produces backtrace pinpointing the place where the error is emitted. Then work back from there.

PSALM_ALLOW_XDEBUG=1 psalm --no-cache --threads=1 allows step debugging with Xdebug.

ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 18, 2021
@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 19, 2021

It looks like MethodComparator::create gets called twice for the above example. It first compares the two traits, then it attempts to compare Test class with AnotherTrait and the test fails. I've added some diagnostic messages and got:

comparing: 'AggregateTrait' with 'AnotherTrait': success                        
comparing: 'Test' with 'AnotherTrait': failure                                  

I observed that the comparison is successfull as long as the implementer class(like) is a trait. This may also be infered from the code of MethodComparator::compareMethodDocblockReturnType(). If it's not a trait, then (Test class in our case), then the code that transforms template parameters is not executed, and the information about "int" type provided as parameter to @template-use AnotherTrait is not used.

The question is now, where to dig further to find an solution?

ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 22, 2021
ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 22, 2021
@orklah orklah linked a pull request Nov 22, 2021 that will close this issue
ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 22, 2021
ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 22, 2021
ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 22, 2021
ptomulik added a commit to ptomulik/psalm that referenced this issue Nov 22, 2021
orklah added a commit that referenced this issue Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants