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

ArrayType - string offset might exist as integer offset #2928

Merged
merged 4 commits into from
Feb 21, 2024
Merged

ArrayType - string offset might exist as integer offset #2928

merged 4 commits into from
Feb 21, 2024

Conversation

michalbundyra
Copy link
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
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
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
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
Member

Basically do the same thing I did here 0b78c55

@michalbundyra
Copy link
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
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
Contributor Author

I only hope you squash PRs ;-)

@ondrejmirtes ondrejmirtes merged commit b4dbfa4 into phpstan:1.10.x Feb 21, 2024
439 checks passed
@ondrejmirtes
Copy link
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.

None yet

2 participants