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

mixed template when implementing interface #4302

Open
marcosh opened this issue Oct 9, 2020 · 6 comments
Open

mixed template when implementing interface #4302

marcosh opened this issue Oct 9, 2020 · 6 comments

Comments

@marcosh
Copy link
Contributor

marcosh commented Oct 9, 2020

Consider the code in https://psalm.dev/r/f3afaafff3

I would expect it to break since $stringCollection is an instance of ArrayCollection<string> which implements Collection<string>. The latter says that the second parameter of add should be a string, while here I can actually pass an int without any reported issues

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/f3afaafff3
<?php

/**
 * @template T
 */
interface Collection
{
    /**
     * @param T $value
     */
    public function add(string $key, $value): self;
}

/**
 * @template T
 * @implements Collection<T>
 */
class ArrayCollection implements Collection
{
    /** @var array<string, mixed> */
    private array $data = [];

    /**
     * @param array<string, mixed> $data
     */
    public function __construct(array $data = [])
    {
        $this->data = $data;
    }

    /**
     * @param mixed $value
     */
    public function add(string $key, $value): Collection
    {
        $newData = array_merge($this->data, [$key => $value]);

        return new self($newData);
    }
}

/** @var ArrayCollection<string> $stringCollection */
$stringCollection = new ArrayCollection();
$stringCollection = $stringCollection->add('one', 1);
Psalm output (using commit 7195275):

INFO: UnusedVariable - 44:1 - Variable $stringCollection is never referenced

@weirdan
Copy link
Collaborator

weirdan commented Oct 9, 2020

Widening a parameter type (aka parameter contravariance) is totally valid under LSP.

interface I { public function add(string $value): void; }

does not mean that the parameter should be string only. What it means is that any implementors must accept string. They are free to accept any type that is wider than string as well, e.g string|int or mixed.

@weirdan weirdan closed this as completed Oct 9, 2020
@marcosh
Copy link
Contributor Author

marcosh commented Oct 10, 2020

right! Thanks for the reply and the clarification!

But what about the case when we have also a return type widening? for example https://psalm.dev/r/32966f69fa

Here I would expect $stringCollection->get to return a string since $stringCollection is an instance of Collection<string>, or am I missing something else?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/32966f69fa
<?php

/**
 * @template T
 */
interface Collection
{
    /**
     * @param T $value
     */
    public function add(string $key, $value): self;
    
    /**
     * @return T
     */
    public function get(string $key);
}

/**
 * @template T
 * @implements Collection<T>
 */
class ArrayCollection implements Collection
{
    /** @var array<string, mixed> */
    private array $data = [];

    /**
     * @param array<string, mixed> $data
     */
    public function __construct(array $data = [])
    {
        $this->data = $data;
    }

    /**
     * @param mixed $value
     */
    public function add(string $key, $value): Collection
    {
        $newData = array_merge($this->data, [$key => $value]);

        return new self($newData);
    }
    
    /**
     * @return mixed
     */
    public function get(string $key)
    {
        return $this->data[$key];
    }
}

/** @var ArrayCollection<string> $stringCollection */
$stringCollection = new ArrayCollection();
$stringCollection = $stringCollection->add('one', 1);

echo $stringCollection->get('one');
Psalm output (using commit 7195275):

No issues!

@weirdan
Copy link
Collaborator

weirdan commented Oct 10, 2020

This (https://psalm.dev/r/2bfe7193d0) should be flagged, as T:ArrayCollection as mixed (inherited from the interface) could be narrower than just mixed (declared by the implementor).

@weirdan weirdan reopened this Oct 10, 2020
@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/2bfe7193d0
<?php

/** @template T */
interface Collection
{  
    /** @return T */
    public function get(string $key);
}

/**
 * @template T
 * @implements Collection<T>
 */
abstract class ArrayCollection implements Collection
{  
    /** @return mixed */
    abstract public function get(string $key);
}
Psalm output (using commit 7195275):

No issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants