gotta say this feels like a step in the wrong direction.

• Why did we leave in union types when the non-mixed type was something like \Illuminate\...? There are 11 instances of this in the current PR. If we're going to say custom classes have value when combined with mixed, how can we say things like array do not?

I had another point, but as I dug into it, it looks like there've been some commits/reversions in the past couple of weeks regarding typing, so I'm not sure if my point holds anymore.

All in all, what I'd like to see is us move more towards explicit typing, even if that means somewhat long union types. I also don't mind the array|mixed types, even though I accept the changes you've made are technically correct. The way I read array|mixed is "there's a pretty high chance you should be giving me an array, but technically I can accept anything".

What I'd really love to see is actual strict parameter and return typing, but that's a discussion for another day...

Thanks for your feedback and for the time you took to review it @browner12

• Why did we leave in union types when the non-mixed type was something like \Illuminate\...? There are 11 instances of this in the current PR. If we're going to say custom classes have value when combined with mixed, how can we say things like array do not?

I think it’s because array is one of PHP’s built-in data types, which is already covered by mixed. But this doesn’t apply to things like Laravel Collections.

• What I'd really love to see is actual strict parameter and return typing, but that's a discussion for another day...

I completely agree, and I’m also strict about this when I develop something. However, PRs in open source are a different matter, and in this case we also have the mixed type, which can’t be easily removed.

unless I'm mistaken, mixed also includes custom classes like \Collection.

https://www.php.net/manual/en/language.types.mixed.php

It’s an interesting point to examine. However, I think a collection is an instance of the \Collection class, which can contain a set of mixed items.

@browner12

I would assume, Collection is only included in mixed when it's used without a generic, narrowing it down.

@amirhshokri could please help me to understand why this code is not reachable?

if I'll remove throw_if and will use simple if statement then always will be fine

I tried some cases:

but why !$openedShift and $openedShift === null is not valid?

I think this issue is related to PHPStorm. Based on the current pattern, if we have the following code:

throw new Exception;

// another logic

PHPStorm considers another logic as an unreachable statement. The same thing happens with throw_if, which incorrectly matches this pattern since both start with throw.
I also noticed this issue in several places in the Laravel source code where throw_if is used.
By the way, I think it’s better to raise these kinds of questions in JetBrains forums and keep the discussion here focused on this specific PR.

@siarheipashkevich