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

Undefined offset: 0 when using @var null|string instead of @var string|null #1301

Merged
merged 2 commits into from
Mar 22, 2021

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Mar 22, 2021

Q A
Bug fix? yes
New feature? no
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? see CI
License MIT

After upgrading from 3.12.0 to 3.12.1 I received Undefined offset: 0 errors. After some debugging I found the problem.

In the commits I explain what the problem is and how I fixed it.

ruudk added 2 commits March 22, 2021 15:19
When the property is typed with `@var string|null` everything works fine.

But when it's typed in opposite order it breaks `@var null|string`: Undefined offset: 0
When `filterNullFromTypes` receives the following `[ 0 => IdentifierTypeNode("null"), 1 => IdentifierTypeNode("string") ]` types, it will map the one that is called `"null"` with `null` like this: `[ 0 => null, 1 => IdentifierTypeNode("string") ]`.

It then passes the array to `array_filter` that removes all the `null` values, leaving the following array as a result: `[1 => IdentifierTypeNode("string") ]`.

The problem is that `array_filter` keeps the keys intact, while the code that uses this doesn't expect it.

By using `array_values` we get a clean array again: `[0 => IdentifierTypeNode("string") ]`.
@ruudk ruudk changed the title Undefined offset: 0 when using @var null|string instead of @var string|null Undefined offset: 0 when using @var null|string instead of @var string|null Mar 22, 2021
@goetas goetas added the bug label Mar 22, 2021
@goetas goetas merged commit b1e5bb6 into schmittjoh:master Mar 22, 2021
@ruudk ruudk deleted the fix-nullable branch March 23, 2021 06:30
@ruudk
Copy link
Contributor Author

ruudk commented Mar 23, 2021

@goetas Thanks for the merge. Can this be tagged as the latest one is broken. 🙏

@goetas
Copy link
Collaborator

goetas commented Mar 23, 2021

https://github.com/schmittjoh/serializer/releases/tag/3.12.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants