Skip to content

Add key typehints to iterable functions#102

Open
annadamm-check24 wants to merge 3 commits into
nikic:masterfrom
annadamm-check24:add-key-typehint
Open

Add key typehints to iterable functions#102
annadamm-check24 wants to merge 3 commits into
nikic:masterfrom
annadamm-check24:add-key-typehint

Conversation

@annadamm-check24
Copy link
Copy Markdown

@annadamm-check24 annadamm-check24 commented May 11, 2026

Description

Currently, PHPStan cannot handle string array/iterable keys for map, filter, and other functions, as they are not properly type hinted

Solution

The same as is already done in fromPairs, tap, etc., adding a @template TKey

Existing Psalm errors

There were, and still are psalm-errors after this PR. I did not fix the existing ones, as that would be out of scope. As far as I can see, I did not introduce new psalm errors.

Probably, the CI should be extended for phpstan + psalm and these errors be fixed. And maybe there should be Tests, if both PHPStan and Psalm are correctly working on code derived from this library (could already happen in tests by accident?)

Comment thread src/iter.php
*
* @template T
* @template U
* @template IterableValue
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the other ones use TKey/TValue, why the difference here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we have 3 variables and it was difficult to understand the key/value of which it should be.
Do you have other suggestions? I have seen UKey, TValue, etc., but I personally find that hard to understand.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic should I change it, or do you think this is still okay?

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

Successfully merging this pull request may close these issues.

2 participants