Enable strict comparison for arrays of scalar values

[trickeyone]

With the change in v1.5.0 that enabled strict in_array checks, if an array of objects is passed, it is strictly checked instead of doing a more passive check on the values. This enables the ability to pass an array objects (i.e. Discriminated entities) to be used in filtering an array without the objects being the exact same object in memory.

This PR is in response to #113

[trickeyone]

It's been almost 2 years since I submitted this PR, and there hasn't been any response to it. I know @Ocramius added the "BC Break" tag, but I don't get how this can be considered a BC Break, while the change to make the in_array call do a strict check was not. It literally broke a lot of functionality.

The change I introduced would only make it where the strict check is done on scalar values. I fail to see how that would be a BC Break.

[greg0ire]

I don't get how this can be considered a BC Break, while the change to make the in_array call do a strict check was not. It literally broke a lot of functionality.

I think the rationale might be: "Just because we make one mistake does not mean we should make 2". Your PR is 2 years old as you pointed out, and the change you refer to (please link to it BTW) was introduced in 1.5, which can only be older than that. It's possible that some people are relying on the comparison being strict now, so that's where the BC-break would be I guess.

[trickeyone]

@greg0ire I understand that, however, the "mistake" was introducing a major change in the behavior for the IN/NOT_IN operators to use the strict checking. I'm not saying that doing the strict type-checking is wrong, but making it do a strict type-check on objects just introduces a whole host of other potentially unanticipated issues. Hence the reason why my change doesn't remove the strict type-checking, it modifies it to only strict type-check scalar values (i.e. string, ints, floats, arrays, etc).

If you do a blame on the files in this PR, you'll see when the change was added. That was with the 1.5.0 release, but if you want a specific commit, it's 310f54c. Before this commit, the call was a non-strict in_array. When I made this PR 2 years ago, I didn't really need to "link to it" because it was relatively new change that was introduced, and easily findable. The BC breaking change only came to light when I upgraded a project from 1.4.x to 1.5.x and I started having tests failing, which I outlined in the initiating issue I flagged (#113) over 2.5 years ago. That was the catalyst for this PR.

Also, I don't believe my change could be considered a "mistake" given that it was added to the 2.0 milestone.

Regardless, I added a new PR to make this directed at 1.6.x since it looks like master is geared towards 2.0 and this PR is no longer in-line with it. Until this is resolved, I'm probably going to be using my fork since this is a major issue for the project I need it for.

[greg0ire]

@greg0ire I understand that, however, the "mistake" was introducing a major change in the behavior for the IN/NOT_IN operators to use the strict checking.

It was, wasn't it?

If you do a blame on the files in this PR, you'll see when the change was added. That was with the 1.5.0 release, but if you want a specific commit, it's 310f54c. Before this commit, the call was a non-strict in_array. When I made this PR 2 years ago, I didn't really need to "link to it" because it was relatively new change that was introduced, and easily findable.

Maintainers like me are lazy. For sure, we know our way around git, but you know, sometimes we browse Github, or just don't feel like doing the digging. It's a good idea to provide as many links as possible if you want things to move forward. And as you pointed out, the more time passes, the hardest it is to find.

Also, I don't believe my change could be considered a "mistake" given that it was added to the 2.0 milestone.

Your change is definitely not a mistake, but making a PR to 1.6.x with it could be, since there seems to be a BC break. I'm not sure what our policy would be here, and I think the BC break might be small enough for the PR to 1.6.x to be accepted.

[greg0ire]

I found the original PR, for reference: #97

[greg0ire]

🤔 I think this should be closed since you ended up contributing this on another branch: #249