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

Type hint array reduce closure #6725

Merged

Conversation

peterfox
Copy link
Contributor

@peterfox peterfox commented Feb 9, 2025

Changes

  • Adds AddClosureParamTypeForArrayReduceRector rule
  • Adds tests for AddClosureParamTypeForArrayReduceRector
  • Adds to the type dec config:
    • AddClosureParamTypeForArrayReduceRector
    • AddClosureParamTypeForArrayMapRector

Why

This rule is following in the path of #6377 that performed additional type hinting array function calls that use closures. This covers type hinting the items of the array and type hinting the carry.

$array = ['a', 'b', 'c'];
-array_reduce($array, function ($carry, $item) {
+array_reduce($array, function (string $carry, string $item) {
    return $carry . $item;
});

@TomasVotruba
Copy link
Member

Looks good 👍

Could you add it to type declaration set?

@peterfox
Copy link
Contributor Author

peterfox commented Feb 9, 2025

Thanks @TomasVotruba I've added it to the config but also added the array map rule as well as it wasn't added previously I believe.

@TomasVotruba TomasVotruba merged commit 3b14af2 into rectorphp:main Feb 9, 2025
44 checks passed
@TomasVotruba
Copy link
Member

Thank you 😊

@samsonasik
Copy link
Member

@peterfox it seems buggy on nullable object on 2nd param

 Argument #2 ($someParam) must be of type SomeInterface, null given

cause TypeError, I will check if that resolvable.

@samsonasik
Copy link
Member

I see, the issue is it read from docblock type as well, which unreliable, per @staabm review at #6377 (comment) , this rule is similar risky, this should not be part of type declaration set, and should be manually added when needed, @peterfox could you create new PR to undo register from type declaration set? Thank you.

@samsonasik
Copy link
Member

@staabm @peterfox I created new PR to unregister it, this needs to be registered manually due to docblock type usage

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.

3 participants