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

Fix ConstantArrayType creation for preg_split with PREG_SPLIT_OFFSET_CAPTURE #1486

Closed
wants to merge 2 commits into from

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Jun 30, 2022

Refs: #1484

Looks like it was forgotten to explicitly specifiy $nextAutoIndexes correctly in that edge case. And then it falls back to [0] which can break the ConstantArrayTypeBuilder (since the array keys 0 and 1 are explicitly specified already).

I quickly looked through most of the other ConstantArrayType creations but did not find any more problems.

Fyi the 3 errors, that looked fine to me, are

Emitted errors:
- Method Bug7554\Readline::_bindControlB() has parameter $line with no type specified
  in /home/martin/PhpstormProjects/phpstan-src/tests/PHPStan/Analyser/data/bug-7554.php on line 11

- Parameter #1 $value of function count expects array|Countable, array<int, array<int, int<0, max>|string>>|false given
  in /home/martin/PhpstormProjects/phpstan-src/tests/PHPStan/Analyser/data/bug-7554.php on line 25

- Cannot access offset int<1, max> on array<int, array{string, int<0, max>}>|false
  in /home/martin/PhpstormProjects/phpstan-src/tests/PHPStan/Analyser/data/bug-7554.php on line 26

@ondrejmirtes
Copy link
Member

Awesome, great! What I'd love additionally is a test for https://github.com/phpstan/phpstan-src/blob/1.8.x/tests/PHPStan/Type/Constant/ConstantArrayTypeBuilderTest.php that shows the scenario that failed with the internal error 😊

@herndlm
Copy link
Contributor Author

herndlm commented Jul 1, 2022

Added something, but I'm pretty sure you meant it differently :) let me know please what to adapt

@ondrejmirtes
Copy link
Member

Oh, I didn't check the patch, I thought the bug was in ConstantArrayTypeBuilder, not the preg_split extension...

@ondrejmirtes
Copy link
Member

I merged it without the last commit, thanks :)

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