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 crash on a union type including null #1106

Merged
merged 1 commit into from Dec 11, 2020

Conversation

LeSuisse
Copy link
Contributor

@LeSuisse LeSuisse commented Dec 6, 2020

This change prevents the generation of type hints with 2 "null".

For example, "string|array|null" should generate the code "string|array|null" instead of "string|array|null|null" to avoid a PHP fatal error "Duplicate type null is redundant".

Closes #1105.

@GrahamCampbell
Copy link
Contributor

GrahamCampbell commented Dec 6, 2020

Does that test pass without the code change? When I originally wrote this code string|null would not show up as a union type, and PHP would pretend it was ?string. I wonder if the bug you were experiencing is a bug with union types in core code, and not userland functions?

@LeSuisse
Copy link
Contributor Author

LeSuisse commented Dec 6, 2020

No it does not, see https://travis-ci.com/github/LeSuisse/mockery/jobs/452045803 which correspond to LeSuisse@d328ed6 (the test without the code change).
I detected the issue by mocking a core class but it seems userland is also affected.

@LeSuisse
Copy link
Contributor Author

LeSuisse commented Dec 6, 2020

The CI failures with DEPS=lowest on PHP 7.3 and PHP 7.4 seems to be unrelated to this change (I did not confirm it but it looks like a side effect of the release of Xdebug 3.0).

@GrahamCampbell
Copy link
Contributor

NB This PR should target the 1.3 branch. PHPUnit issues can be fixed by replacing ^5.7.10|^6.5|^7.5|^8.5|^9.3 with the same thing, only with each major version min changed to very latest in that series.

@LeSuisse LeSuisse changed the base branch from master to 1.3 December 7, 2020 09:50
@LeSuisse
Copy link
Contributor Author

LeSuisse commented Dec 7, 2020

The PR now targets the 1.3 branch.

Not exactly sure to understand how to fix the PHPUnit/Xdebug 3.0 issues on it since we need PHPUnit ^5 .7 to run the tests on PHP 5.6 and this version also covers PHP ^7.0.

@GrahamCampbell
Copy link
Contributor

Hmm, mockery should probably just drop it's prefer-lowest tests. They don't add any value, since Mockery has basically no dependencies.

@davedevelopment
Copy link
Collaborator

This looks good to me and I'm happy to drop the prefer-lowest tests, to be honest that all confuses me anyway.

@LeSuisse
Copy link
Contributor Author

LeSuisse commented Dec 8, 2020

I opened #1107 to drop the prefer-lowest tests. I will rebase this PR once the other is merged :) .

This change prevents the generation of type hints with 2 "null".
For example, "string|array|null" should generate the code
"string|array|null" instead of "string|array|null|null" to avoid
a PHP fatal error "Duplicate type null is redundant".

Closes mockery#1105.
@davedevelopment
Copy link
Collaborator

Sorry for the delay, just tagged 1.4.3 and 1.3.4

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.

Union type like "array|string|null" or "string|int|null" crashes Mockery
3 participants