Add phpstan typing annotations #250
Conversation
KmeCnin
force-pushed
the
phpstan
branch
7 times, most recently
from
213a9b7
to
b21f921
Compare
KmeCnin
force-pushed
the
phpstan
branch
2 times, most recently
from
2ca165b
to
98a23c8
Compare
|
Terrific work, thank you very much. Let me know when you thinking's ready for merge |
The index_of changes would need to be deprecated first. For now why not use a conditional type to express the current behavior? |
KmeCnin
force-pushed
the
phpstan
branch
3 times, most recently
from
228b6f1
to
ea147af
Compare
|
@lstrojny I guess this work is ready to be merge. I don't see further improvements at the moment. (I removed the other commit I was previously based on) |
src/Functional/Group.php
Outdated
| * @param iterable<K,V> $collection | ||
| * @param callable(V,K,iterable<K,V>):G $callback | ||
| * | ||
| * @return (G is numeric-string ? array<int,array<K,V>> : array<G,array<K,V>>) |
There was a problem hiding this comment.
Is there a way to also hint phpstan when G is surely a (non-numeric) string?
With current annotation I get false-positive in this case:
There was a problem hiding this comment.
I don't think this is possible right now, see phpstan/phpstan#6847
Also the numeric-string isn’t correct, because this:
["1.2" => "foo"];The key is a numeric string but the result will be a string array, not an integer array.
There was a problem hiding this comment.
You got a very good point and this is very sad since I'll have to remove this in some other functions also 😢
| * @template T | ||
| * | ||
| * @param callable():T $callback | ||
| * @param T $result | ||
| * | ||
| * @return callable():T |
There was a problem hiding this comment.
Would be great to indicate with types that the callback given is type-wise identical. But this depends on phpstan/phpstan#8964 and phpstan/phpstan#8214
src/Functional/Contains.php
Outdated
| * @template V | ||
| * @template V2 of V | ||
| * | ||
| * @param iterable<V> $collection | ||
| * @param V2 $value | ||
| * @param bool $strict | ||
| * | ||
| * @return bool | ||
| * |
There was a problem hiding this comment.
This needs to be a bit more complicated, to properly type the strictness.
We need a conditional type here where V2 is of V if strict is true but otherwise V2 is independent of V
There was a problem hiding this comment.
Good catch.
Here is what I ended with: https://phpstan.org/r/c7d1770b-678b-446f-a285-095557fb6b3e
| if ($callback === null) { | ||
| $callback = '\Functional\id'; | ||
| } | ||
|
|
||
| foreach ($collection as $index => $element) { | ||
| if (!$callback($element, $index, $collection)) { | ||
| if (!(null === $callback ? id($element) : $callback($element, $index, $collection))) { |
There was a problem hiding this comment.
Yes sorry I should have explain it but here is the error I get if I keep the previous code:
------ -----------------------------------------------------------------------------------
Line Every.php
------ -----------------------------------------------------------------------------------
38 Callable '\\Functional\\id'|(callable(V, K, iterable<K, V>): bool) invoked with 3
parameters, 1 required.
------ -----------------------------------------------------------------------------------
src/Functional/Group.php
Outdated
| * @param iterable<K,V> $collection | ||
| * @param callable(V,K,iterable<K,V>):G $callback | ||
| * | ||
| * @return (G is numeric-string ? array<int,array<K,V>> : array<G,array<K,V>>) |
There was a problem hiding this comment.
I don't think this is possible right now, see phpstan/phpstan#6847
Also the numeric-string isn’t correct, because this:
["1.2" => "foo"];The key is a numeric string but the result will be a string array, not an integer array.
|
@lstrojny I think the PR could be merged. Please let me know if we need more work on it |
Remaining functions not typed with PhpStan (seems not doable with current features):
composecurryreduce_rightsuppress_error