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

Fixed mixed implicit nullability (closes #74) #75

Closed
wants to merge 3 commits into from

Conversation

dakur
Copy link

@dakur dakur commented Jan 19, 2021

@dakur
Copy link
Author

dakur commented Jan 19, 2021

Not sure if more places are affected, I've fixed only this one.

@dg
Copy link
Member

dg commented Jan 19, 2021

It will probably also apply to properties and return values, right?

Will it work with uppercase 'MIXED'?

@dakur
Copy link
Author

dakur commented Jan 19, 2021

Frankly said, I have no idea. 🤷‍♂️ I just wanted to sketch where the problem lies and what looks like solution. But I don't know the code base much (or at all), so I rather pass it on people more familiar with it.

@dg
Copy link
Member

dg commented Jan 19, 2021

fixed

@dg dg closed this Jan 19, 2021
@dakur
Copy link
Author

dakur commented Jan 19, 2021

@dg It is not fixed in 3.5.2. Probably something got wrong when rebasing? Factory#fromMethodReflection() still relies on allowsNull()

@dakur
Copy link
Author

dakur commented Jan 19, 2021

Aha, there should be probably ! strcmp() in here:

if ($nullable && strcasecmp($type, 'mixed')) {

@dakur dakur deleted the patch-1 branch January 19, 2021 14:09
@dakur
Copy link
Author

dakur commented Jan 19, 2021

@dg Ok, it works, sorry, I've screwed up my expected test file during previous testing.

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