Psalm: expects 'Traversable<SomeType>', parent type `...implements InteratorAggregate` provided, but not for `iterable<int>`

Created on 25 Oct 2018  路  12Comments  路  Source: vimeo/psalm

https://getpsalm.org/r/67d4675e8b

Getting notices for \IteratorAggregate<int> and \Traversable<int>:

INFO: MixedTypeCoercion - 33:22 - $this->value expects 'IteratorAggregate<int>', parent type `SomeIterator` provided

INFO: MixedTypeCoercion - 46:22 - $this->value expects 'Traversable<int>', parent type `SomeIterator` provided

On the other hand, nothing is said against iterable<int>. It is either these two notices are misattributed, or there's a notice missing.

(These notices are fixed by declaring explicit type for generics.)

bug

All 12 comments

I don't think this is a bug - at the very least, it's beyond the scope of Psalm's current abilities to infer that a class iterator will yield a particular type.

But this should definitely not have any issues (when we explicitly give a generic return type for the iterator: https://getpsalm.org/r/ca305d5520

class SomeIterator implements IteratorAggregate
{
    /**
     * @return Generator<int>
     */
    public function getIterator()
    {
        yield 1;         
    }
}

class Example2
{
    /**
     * @var \IteratorAggregate<int>
     */
    private $value; 

    public function __construct()
    {
        $this->value = new SomeIterator(); 
    }
}

Ok, it's a bit of a hack, but what I'm doing is: when calling new X(), Psalm now checks to see if X implements IteratorAggregate and also checks that the getIterator method exists. It then grabs the iterator generic params and types the newly-created object with them. So if X::getIterator returns Iterator<int, string> the expression new X() will have the type X<int, string>.

But if X::__construct exists and sets some other generic params, the iterator generic params will not be added.

Here's the version of the above code that works: https://getpsalm.org/r/af8d7f4ecd

The only difference is a return type on the getIterator method.

I'm going to remove the above hack in favour of the following:

/**
 * @template-implements IteratorAggregate<int>
 */
class SomeIterator implements IteratorAggregate
{
    public function getIterator()
    {
        yield 1;         
    }
}

class Example2
{
    /**
     * @var \IteratorAggregate<int>
     */
    private $value; 

    public function __construct()
    {
        $this->value = new SomeIterator(); 
    }
}

So I tried this new approach, and seems like I can't get it to work. Am I too early?

On the other hand, a return type on the getIterator method works flawless: https://getpsalm.org/r/5e84b54aa1

Should I wait until this newer approach goes live, or should I just use that hack for time being?

@muglug Is there any difference between @template-extends and @template-implements annotations?

@weirdan no

@sanmai I was wrong, this isn't a bug - you need to add the return type to getIterator like so: https://getpsalm.org/r/3a1f0aa80b.

Psalm _should_ complain if you don't, though.

Psalm _will_ warn if that new return type is wrong: https://getpsalm.org/r/276eaa97e1

I've ticketed the bad behaviour here: https://github.com/vimeo/psalm/issues/1232

Sorry to go back and forth on this.

@sanmai this _is_ a bug, I've ticketed separately in https://github.com/vimeo/psalm/issues/1234

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vudaltsov picture vudaltsov  路  3Comments

staabm picture staabm  路  3Comments

ADmad picture ADmad  路  3Comments

roukmoute picture roukmoute  路  3Comments

caugner picture caugner  路  3Comments