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

Add regression test #3066

Merged
merged 1 commit into from
May 30, 2024
Merged

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented May 12, 2024

Closes phpstan/phpstan#3300

See also https://phpstan.org/r/f16bc296-b26d-4081-a308-7dd0416def71

Not sure if more precision for constant arrays is needed? For such an edge case the current result might be enough? My PR #2117 adds some complexity, including changes on Type::getKeysArray() and rebasing those changes on latest 1.11.x is not trivial after #2516 and I'm worried they would make things even more complex..

The reason why this is fixed is the early exit in

if (count($functionCall->getArgs()) !== 1) {
return null;
}
when more than one argument is given which did not exist yet when the bug was reported.

@ondrejmirtes
Copy link
Member

How does this compare to your older PR? #2117

Is this issue really fixed?

@herndlm
Copy link
Contributor Author

herndlm commented May 15, 2024

How does this compare to your older PR? #2117

Is this issue really fixed?

before, when the issue was reported ArrayKeysFunctionDynamicReturnTypeExtension was incorrectly specifying types also when array_keys was called with more than only one arg -> even if people wanted to filter for specific keys we were always returning all constant array keys, see

Then I did some refactoring which didn't change behaviour.

Then @staabm added in 89cfc49#diff-a408f91196cfe8e476bde977d3ceab1867fddee0b39a9bf1bb4e94ff7c1a2eb3 the check where the extension would only handle calls of array_keys with a single arg anymore.

And that basically fixed it, I'd say. Since then the extension didn't wrongfully use the constant array keys anymore, the type specification must be falling back to the default function definition, where the types are wider and that doesn't make problems.

Difference to my solution? I tried to add support to array_keys with more than 1 arg and properly use the array keys to have a more specific / narrower type. But that's a nice-to-have thing which adds most likely only unnecessary complexity at the moment, especially after #2516.

@ondrejmirtes ondrejmirtes merged commit b543e8f into phpstan:1.11.x May 30, 2024
227 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@herndlm herndlm deleted the regression-test-bug-3300 branch May 31, 2024 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants