Skip to content

Add ArrayChangeKeyCaseFunctionReturnTypeExtension#3549

Merged
ondrejmirtes merged 7 commits intophpstan:1.12.xfrom
VincentLanglet:arrayChangeKeyCase
Jan 16, 2025
Merged

Add ArrayChangeKeyCaseFunctionReturnTypeExtension#3549
ondrejmirtes merged 7 commits intophpstan:1.12.xfrom
VincentLanglet:arrayChangeKeyCase

Conversation

@VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Oct 7, 2024 •
edited
Loading

@VincentLanglet VincentLanglet force-pushed the arrayChangeKeyCase branch 2 times, most recently from 615c172 to 461b22d Compare October 7, 2024 19:48
VincentLanglet
VincentLanglet commented Oct 7, 2024
View reviewed changes
@VincentLanglet VincentLanglet mentioned this pull request Oct 7, 2024
Closed
@VincentLanglet VincentLanglet force-pushed the arrayChangeKeyCase branch 3 times, most recently from 8f5fde7 to 61bdd79 Compare October 7, 2024 21:35
@VincentLanglet VincentLanglet marked this pull request as ready for review October 7, 2024 21:40
@phpstan-bot
Copy link
Collaborator

phpstan-bot commented Oct 7, 2024

This pull request has been marked as ready for review.

@VincentLanglet VincentLanglet requested a review from staabm October 7, 2024 21:50
staabm
staabm reviewed Oct 8, 2024
View reviewed changes
@VincentLanglet VincentLanglet requested a review from staabm October 8, 2024 07:50
staabm
staabm approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@staabm staabm left a comment

Choose a reason for hiding this comment

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

lgtm - thanks

@VincentLanglet VincentLanglet mentioned this pull request Nov 4, 2024
Closed
@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Nov 24, 2024

I updated the PR to also support uppercase-string @ondrejmirtes

ondrejmirtes
ondrejmirtes requested changes Jan 15, 2025
View reviewed changes
src/Type/Php/ArrayChangeKeyCaseFunctionReturnTypeExtension.php Outdated
foreach ($constantArrays as $constantArray) {
$newConstantArrayBuilder = ConstantArrayTypeBuilder::createEmpty();
foreach ($constantArray->getKeyTypes() as $i => $keyType) {
$valueType = $constantArray->getOffsetValueType($keyType);
Copy link
Member

@ondrejmirtes ondrejmirtes Jan 15, 2025

Choose a reason for hiding this comment

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

Calling getOffsetValueType is too expensive. You can $valueTypes = $constantArray->getValueTypes() and then access $i to get it.

Copy link
Contributor Author

@VincentLanglet VincentLanglet Jan 15, 2025

Choose a reason for hiding this comment

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

Changed

@ondrejmirtes ondrejmirtes merged commit b9894fa into phpstan:1.12.x Jan 16, 2025
451 of 452 checks passed
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jan 16, 2025

Thank you.

@ondrejmirtes ondrejmirtes mentioned this pull request Jan 16, 2025
Closed
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jan 22, 2025

Would be nice to add a similar extension for mb_convert_case, would you look into it? Context: phpstan/phpstan#12452

Thanks!

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jan 22, 2025

Would be nice to add a similar extension for mb_convert_case, would you look into it? Context: phpstan/phpstan#12452

Thanks!

Do you have a reproducer ?

Because, for me, the mb_convert_case extension is handled by

phpstan-src/src/Type/Php/StrCaseFunctionsReturnTypeExtension.php

Line 45 in e2b5a60

'mb_convert_case' => 2,

And we can see in the history that it was handled for lowercase-string and upper-case string
https://github.com/phpstan/phpstan-src/commits/e2b5a60fa52460ba3addd519b07e2244809878ee/src/Type/Php/StrCaseFunctionsReturnTypeExtension.php

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jan 22, 2025

I'm sorry, I haven't checked personally and trusted the user. PHPStan already handles it well https://phpstan.org/r/fad685d0-5331-4af2-8bb9-d0d1a0f81d00.

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jan 22, 2025

Yes, it works fine https://phpstan.org/r/b269ff18-e0b8-42d4-9586-87edaee9fccf

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

Reviewers

@staabm staabm staabm approved these changes

@ondrejmirtes ondrejmirtes Awaiting requested review from ondrejmirtes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@VincentLanglet @phpstan-bot @ondrejmirtes @staabm