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

Templates & late static binding #8142

Open
daniel-gasparotto opened this issue Jun 22, 2022 · 7 comments
Open

Templates & late static binding #8142

daniel-gasparotto opened this issue Jun 22, 2022 · 7 comments

Comments

@daniel-gasparotto
Copy link

Please consider this example: https://psalm.dev/r/10d35322cc

Container<T> has a final ctor, which calls static::fromIterable().

Map extends Container.
Map::fromIterable() decides the TValue that goes into the Container.

The problem here is that Psalm fails to detect the type of Map<TKey, TValue>
and concludes it's a generic Map<mixed, mixed>. (see Trace)

@ line 37, I try to override the definition of Map's ctor via @method, but to no avail..

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/10d35322cc
<?php

$ARR = [
    'A' => 'B'
];
/** @psalm-trace $m */
$m = new Map($ARR);
$m->every(fn(KeyValuePair $entry) : bool => $ARR[$entry->key()] === $entry->value());

/**
 * @template TValue
 */
abstract class Container
{
    /** @var array<int, TValue> */
	protected array $entries;
    
    // TValue will depend on the implementation of fromIterable()
	final function __construct(iterable $entries = [])
	{
		$this->entries = static::fromIterable($entries);
	}
    
	/**
	 * @template TStaticKey
	 * @template TStaticValue
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, mixed> // we don't know how TStaticKey and TStaticValue are transformed, hence mixed
	 */
	abstract static protected function fromIterable(iterable $iterable) : array;
}

/**
 * @template TEntryKey
 * @template TEntryValue
 * @extends Container<KeyValuePair<TEntryKey, TEntryValue>>
 * @method __construct(iterable<TEntryKey, TEntryValue> $entries = [])
 */
class Map extends Container
{
	/**
	 * @template TStaticKey
	 * @template TStaticValue
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, KeyValuePair<TStaticKey, TStaticValue>>
	 */
	final static protected function fromIterable(iterable $iterable) : array
	{
		$entries = [];
		foreach($iterable as $k => $v) $entries[] = new KeyValuePair($k, $v);
		return $entries;
	}
   
    /** @param \Closure(KeyValuePair<TEntryKey, TEntryValue>) : bool $process */
    final public function every(\Closure $process) : bool
	{
		foreach($this->entries as $entry) if(!$process($entry)) return false;
		return true;
	}
}

/** @template TKey
  * @template TValue */
final class KeyValuePair
{
	/** @var TKey */ private $key;
	/** @var TValue */ private $value;
	/** @return TKey */ public function key() { return $this->key; }
	/** @return TValue */ public function &value() { return $this->value; }
	/** @param TKey $key
	  * @param TValue $value */
	public function __construct($key, $value) { $this->key = $key; $this->value = $value; }
}
Psalm output (using commit 29a2162):

INFO: Trace - 7:1 - $m: Map<mixed, mixed>

INFO: MixedArrayOffset - 8:45 - Cannot access value on variable $ARR using mixed offset

@AndrolGenhald
Copy link
Collaborator

I don't think there's any way around specifying the TStatic* templates on the class itself, but right now class templates can't be used in static methods (#5753 and related issues). This works fine if you're ok having fromIterable not be static until that's supported.

@psalm-github-bot
Copy link

I found these snippets:

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

$ARR = [
    'A' => 'B'
];
/** @psalm-trace $m */
$m = new Map($ARR);
$m->every(fn(KeyValuePair $entry) : bool => $ARR[$entry->key()] === $entry->value());

/**
 * @template TValue
 * @template TStaticKey
 * @template TStaticValue
 */
abstract class Container
{
    /** @var array<int, TValue> */
	protected array $entries;
    
    /**
     * @param iterable<TStaticKey, TStaticValue> $entries
     */
	final function __construct(iterable $entries = [])
	{
		$this->entries = $this->fromIterable($entries);
	}
    
	/**
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, TValue>
	 */
	abstract protected function fromIterable(iterable $iterable) : array;
}

/**
 * @template TEntryKey
 * @template TEntryValue
 * @extends Container<KeyValuePair<TEntryKey, TEntryValue>, TEntryKey, TEntryValue>
 */
class Map extends Container
{
	/**
	 * @param iterable<TEntryKey, TEntryValue> $iterable
	 * @return array<int, KeyValuePair<TEntryKey, TEntryValue>>
	 */
	final protected function fromIterable(iterable $iterable) : array
	{
		$entries = [];
		foreach($iterable as $k => $v) $entries[] = new KeyValuePair($k, $v);
		return $entries;
	}
   
    /** @param \Closure(KeyValuePair<TEntryKey, TEntryValue>) : bool $process */
    final public function every(\Closure $process) : bool
	{
		foreach($this->entries as $entry) if(!$process($entry)) return false;
		return true;
	}
}

/** @template TKey
  * @template TValue */
final class KeyValuePair
{
	/** @var TKey */ private $key;
	/** @var TValue */ private $value;
	/** @return TKey */ public function key() { return $this->key; }
	/** @return TValue */ public function &value() { return $this->value; }
	/** @param TKey $key
	  * @param TValue $value */
	public function __construct($key, $value) { $this->key = $key; $this->value = $value; }
}
Psalm output (using commit 29a2162):

INFO: Trace - 7:1 - $m: Map<'A', 'B'>

@daniel-gasparotto
Copy link
Author

daniel-gasparotto commented Jun 22, 2022

Thanks for your suggestion, @AndrolGenhald

btw, by adding an explicit ctor to Map, it works as expected.
https://psalm.dev/r/2419c49458

But for that, I had to remove the final on the Container ctor... which is what I was trying to avoid :(
that's why I was looking fo a @method approach

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/2419c49458
<?php

$ARR = [
    'A' => 'B'
];
/** @psalm-trace $m */
$m = new Map($ARR);
$m->every(fn(KeyValuePair $entry) : bool => $ARR[$entry->key()] === $entry->value());

/**
 * @template TValue
 * @psalm-consistent-constructor
 */
abstract class Container
{
    /** @var array<int, TValue> */
	protected array $entries;
    
    // TValue will depend on the implementation of fromIterable()
	/*final*/function __construct(iterable $entries = [])
	{
		$this->entries = static::fromIterable($entries);
	}
    
	/**
	 * @template TStaticKey
	 * @template TStaticValue
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, mixed> // we don't know how TStaticKey and TStaticValue are transformed, hence mixed
	 */
	abstract static protected function fromIterable(iterable $iterable) : array;
}

/**
 * @template TEntryKey
 * @template TEntryValue
 * @extends Container<KeyValuePair<TEntryKey, TEntryValue>>
 */
class Map extends Container
{
	/** @param iterable<TEntryKey, TEntryValue> $entries */
	final function __construct(iterable $entries = []) { parent::__construct($entries); }
    
	/**
	 * @template TStaticKey
	 * @template TStaticValue
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, KeyValuePair<TStaticKey, TStaticValue>>
	 */
	final static protected function fromIterable(iterable $iterable) : array
	{
		$entries = [];
		foreach($iterable as $k => $v) $entries[] = new KeyValuePair($k, $v);
		return $entries;
	}
   
    /** @param \Closure(KeyValuePair<TEntryKey, TEntryValue>) : bool $process */
    final public function every(\Closure $process) : bool
	{
		foreach($this->entries as $entry) if(!$process($entry)) return false;
		return true;
	}
}

/** @template TKey
  * @template TValue */
final class KeyValuePair
{
	/** @var TKey */ private $key;
	/** @var TValue */ private $value;
	/** @return TKey */ public function key() { return $this->key; }
	/** @return TValue */ public function &value() { return $this->value; }
	/** @param TKey $key
	  * @param TValue $value */
	public function __construct($key, $value) { $this->key = $key; $this->value = $value; }
}
Psalm output (using commit 29a2162):

INFO: Trace - 7:1 - $m: Map<'A', 'B'>

@AndrolGenhald
Copy link
Collaborator

btw, by adding an explicit ctor to Map, it works as expected.
https://psalm.dev/r/2419c49458

In that case you're overriding the constructor so that you can use the Map templates on $entries. If you don't want to have to override the constructor, those templates will need to be on Containers constructor, which is why you'd need the 2 extra templates on Container itself.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/2419c49458
<?php

$ARR = [
    'A' => 'B'
];
/** @psalm-trace $m */
$m = new Map($ARR);
$m->every(fn(KeyValuePair $entry) : bool => $ARR[$entry->key()] === $entry->value());

/**
 * @template TValue
 * @psalm-consistent-constructor
 */
abstract class Container
{
    /** @var array<int, TValue> */
	protected array $entries;
    
    // TValue will depend on the implementation of fromIterable()
	/*final*/function __construct(iterable $entries = [])
	{
		$this->entries = static::fromIterable($entries);
	}
    
	/**
	 * @template TStaticKey
	 * @template TStaticValue
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, mixed> // we don't know how TStaticKey and TStaticValue are transformed, hence mixed
	 */
	abstract static protected function fromIterable(iterable $iterable) : array;
}

/**
 * @template TEntryKey
 * @template TEntryValue
 * @extends Container<KeyValuePair<TEntryKey, TEntryValue>>
 */
class Map extends Container
{
	/** @param iterable<TEntryKey, TEntryValue> $entries */
	final function __construct(iterable $entries = []) { parent::__construct($entries); }
    
	/**
	 * @template TStaticKey
	 * @template TStaticValue
	 * @param iterable<TStaticKey, TStaticValue> $iterable
	 * @return array<int, KeyValuePair<TStaticKey, TStaticValue>>
	 */
	final static protected function fromIterable(iterable $iterable) : array
	{
		$entries = [];
		foreach($iterable as $k => $v) $entries[] = new KeyValuePair($k, $v);
		return $entries;
	}
   
    /** @param \Closure(KeyValuePair<TEntryKey, TEntryValue>) : bool $process */
    final public function every(\Closure $process) : bool
	{
		foreach($this->entries as $entry) if(!$process($entry)) return false;
		return true;
	}
}

/** @template TKey
  * @template TValue */
final class KeyValuePair
{
	/** @var TKey */ private $key;
	/** @var TValue */ private $value;
	/** @return TKey */ public function key() { return $this->key; }
	/** @return TValue */ public function &value() { return $this->value; }
	/** @param TKey $key
	  * @param TValue $value */
	public function __construct($key, $value) { $this->key = $key; $this->value = $value; }
}
Psalm output (using commit cbc597f):

INFO: Trace - 7:1 - $m: Map<'A', 'B'>

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

No branches or pull requests

2 participants