Open weierophinney opened 4 years ago
@adamlundrigan would everything be ok if you passed true
to $strict
?
Originally posted by @Ocramius at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151588334
@Ocramius
Simply changing FormSelect to use strict=true breaks an existing test which uses integer keys in the value options array.
I should have included a link to the source there for context. If I simply change this line to add true
for $strict
then I get a failure in this test case which uses integer keys for the value options because it's strictly searching for "string 0" in an array that contains "int 0".
Originally posted by @adamlundrigan at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151625729
@Ocramius inArray
is not covered by tests. https://coveralls.io/builds/3895736/source?filename=src%2FArrayUtils.php#L188
Originally posted by @Maks3w at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151627785
I'll do the logical question. Why not
<?php
$needle = '1.10';
$haystack = ['1.1'];
assert(ArrayUtils::inArray($needle, $haystack, **true**) === false);
Originally posted by @Maks3w at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151637816
Sorry I wrote wrong the example. I've edited
Originally posted by @Maks3w at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151638452
Marco asked same question above: https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151625729
It breaks an existing test in FormSelect. The value_options
array for Select can contain integer keys, which are converted to string when rendered by FormSelect. With a strict check alone that will fail since the haystack has "int 0" but the form data has "string 0". The only workaround that works for both cases is to pass "false" to ArrayUtils::inArray
(so it casts everything to strings) but pass "true" to in_array
to get strict matching. That's what the code in my PR does.
Originally posted by @adamlundrigan at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151639922
Sorry I'm not able to understand why ArrayUtils::inArray($needle, $haystack, **true**)
fail but ArrayUtils::inArray($needle, $haystack, false) { return in_array($needle, $haystack, **true**); }
works.
I've missing something in your answer.
Originally posted by @Maks3w at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151647054
ArrayUtils::inArray
will cast everything to string when it's $strict
argument is false:
/**
* Checks if a value exists in an array.
*
* Due to "foo" == 0 === TRUE with in_array when strict = false, an option
* has been added to prevent this. When $strict = 0/false, the most secure
* non-strict check is implemented. if $strict = -1, the default in_array
* non-strict behaviour is used.
*
* @param mixed $needle
* @param array $haystack
* @param int|bool $strict
* @return bool
*/
public static function inArray($needle, array $haystack, $strict = false)
{
if (!$strict) {
if (is_int($needle) || is_float($needle)) {
$needle = (string) $needle;
}
if (is_string($needle)) {
foreach ($haystack as &$h) {
if (is_int($h) || is_float($h)) {
$h = (string) $h;
}
}
}
}
return in_array($needle, $haystack, $strict);
}
We need that, or else FormSelect view helper will blow up if you set value_options with integer keys, like this:
$element = new SelectElement('foo');
$element->setValueOptions([
0 => 'label0',
1 => 'label1',
]);
// Pretend this value came from POST and we're re-displaying the form
$element->setValue("0");
That's a valid use case. FormSelect view helper will take that and output something like this:
<!-- snip -->
<option value="0" selected="selected">label0</option>
<option value="1" selected="selected">label1</option>
<!-- / snip -->
The types don't match so ArrayUtils::inArray($n, $h, true)
won't work there. ArrayUtils::inArray($n, $h, false)
fixes this case by casting everything to string before chucking it into in_array
.
However, PHP has another thing we need to work around:
var_dump('1.1' == '1.10');
// bool(true)
Which means that ArrayUtils::inArray($n, $h, false)
won't work for this case:
$element = new SelectElement('foo');
$element->setValueOptions([
'1.1' => 'label 1.1',
'1.10' => 'label 1.10',
'1.2' => 'label 1.2',
'1.20' => 'label 1.20',
]);
// Pretend this value came from POST and we're re-displaying the form
$element->setValue("1.1");
We'll end up with both "1.1" and "1.10" selected when we render to HTML because in PHP's bizarro world "1.1" == "1.10".
Basically, we need a check that's strict on value (so "1.1" != "1.10") but not strict type (so "int 0" == "string 0"), and that's what sending true
into in_array
's $strict
regardless of the value provided to ArrayUtils::inArray
's $strict
is doing. It's making sure in_array
does a strict check but casting everything to string first so types aren't an issue.
Originally posted by @adamlundrigan at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151657137
I understand the issue now.
Well, I see inArray
has three responsibilities:
My (abstract) proposal:
Deprecate inArray in favor of :
in_array(
ArrayUtils::needleTransform($needle),
ArrayUtils::haystackTransform($haystack),
$strict
);
Originally posted by @Maks3w at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151659773
I think it could be distilled down to a single generic "recursively cast array keys and non-array values to string" which would cover both the needle and haystack transforms. castKeysAndValuesToString
is long but leaves no ambiguity of purpose.
Regardless, it doesn't feel like the right solution. The intention of ArrayUtils::inArray
was to work around the wonky behaviour of non-strict in_array
, but it's not doing that job completely in it's current state.
Originally posted by @adamlundrigan at https://github.com/zendframework/zend-stdlib/issues/41#issuecomment-151686155
Related: https://github.com/zendframework/zend-form/issues/18
In
Zend\Form\View\Helper\FormSelect
ArrayUtils::inArray
is called with the strict parameterfalse
. This causes an issue with string matching ('1.1' and '1.10' treated equivalent):(3v4l: https://3v4l.org/HKM8Q)
Simply changing FormSelect to use strict=true breaks an existing test which uses integer keys in the value options array.
Since
ArrayUtils::inArray
uses string casting to work aroundin_array
's wonky non-strict behaviour, shouldn't the call toin_array
always have strict=true?I've tested this change here (all tests pass) and against
zend-form
(where it fixes the reported issue).What's the protocol for testing changes to packages which may have knock-on effects on other packages? Pull every repo that has a dependency on stdlib, apply the patch, and run the tests?
Originally posted by @adamlundrigan at https://github.com/zendframework/zend-stdlib/issues/41