Skip to content

ArrayType - string offset might exist as integer offset#2928

Merged
ondrejmirtes merged 4 commits intophpstan:1.10.xfrom
michalbundyra:fix/10610
Feb 21, 2024
Merged

ArrayType - string offset might exist as integer offset#2928
ondrejmirtes merged 4 commits intophpstan:1.10.xfrom
michalbundyra:fix/10610

Conversation

@michalbundyra
Copy link
Copy Markdown
Contributor

Fixes phpstan/phpstan#10610

I am not sure yet what changes to do in phpstan-baseline.neon (if any), and if we need any assertType in test case 🤔

Fixes phpstan/phpstan#10610

I am not sure yet what changes to do in `phpstan-baseline.neon` (if any),
and if we need any `assertType` in test case 🤔
@michalbundyra
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes pretty unsure how to fix it now. Not completely sure if my changes are good too. Feel free to push change or if you could suggest something I am happy to try :) Thanks

@ondrejmirtes
Copy link
Copy Markdown
Member

Looks like you fixed it. You just need to update the existing tests that are failing because it should no longe error.

You should assertType self::MAP[$k][$value] to make sure that getOffsetValueType behaves the same way hasOffsetValueType does.

@michalbundyra
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes

Thanks for your help here.

Looks like you fixed it. You just need to update the existing tests that are failing because it should no longe error.

It's done now - 72c18e8

You should assertType self::MAP[$k][$value] to make sure that getOffsetValueType behaves the same way hasOffsetValueType does.

Not sure how I can get that? And where it is needed as atm I cannot see any test failure 🤔

Thanks

@ondrejmirtes
Copy link
Copy Markdown
Member

Basically do the same thing I did here 0b78c55

@michalbundyra
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes

Basically do the same thing I did here 0b78c55

I do not really understand the changes in there - 0b78c55, as it is:

		$value = self::MAP[$value] ?? $value;

		assertType("'Test1'|'Test2'", self::MAP[$value]);

but as you see $value is already changed. So I do not really know what this assertType does there and why it has this value. For me it looks wrong.

I've pushed some changes. Hope this is what we need 🤞

@michalbundyra
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes it should be sorted now. Thanks 🙏

Also feel free to make the array shorter

I've tried. But for mysterious reason I cannot replicate the issue with shorter array 🤦‍♂️

@michalbundyra
Copy link
Copy Markdown
Contributor Author

I only hope you squash PRs ;-)

@ondrejmirtes ondrejmirtes merged commit b4dbfa4 into phpstan:1.10.x Feb 21, 2024
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

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