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.

Sign up for GitHub

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

MATCH Problems with Int/Float Compare and Wildcards #3142

Merged
merged 3 commits into from
Nov 4, 2022
Merged

MATCH Problems with Int/Float Compare and Wildcards #3142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fa67d1
MATCH Problems with Int/Float Compare and Wildcards
oleibman Oct 26, 2022
1c1b0a1
More Changes - LibreOffice
oleibman Oct 27, 2022
9fb0587
Merge branch 'master' into issue3141
oleibman Nov 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 0 additions & 64 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,70 +312,6 @@ parameters:
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Address\\:\\:sheetName\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/Address.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchFirstValue\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchFirstValue\\(\\) has parameter \\$lookupArray with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchFirstValue\\(\\) has parameter \\$lookupValue with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchLargestValue\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchLargestValue\\(\\) has parameter \\$keySet with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchLargestValue\\(\\) has parameter \\$lookupArray with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchLargestValue\\(\\) has parameter \\$lookupValue with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchSmallestValue\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchSmallestValue\\(\\) has parameter \\$lookupArray with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:matchSmallestValue\\(\\) has parameter \\$lookupValue with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:prepareLookupArray\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:prepareLookupArray\\(\\) has parameter \\$lookupArray with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:prepareLookupArray\\(\\) has parameter \\$matchType with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:validateLookupArray\\(\\) has parameter \\$lookupArray with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:validateLookupValue\\(\\) has parameter \\$lookupValue with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\ExcelMatch\\:\\:validateMatchType\\(\\) has parameter \\$matchType with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Lookup\\:\\:verifyResultVector\\(\\) has no return type specified\\.$#"
count: 1
Expand Down
85 changes: 67 additions & 18 deletions src/PhpSpreadsheet/Calculation/LookupRef/ExcelMatch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,32 +87,61 @@ public static function MATCH($lookupValue, $lookupArray, $matchType = self::MATC
return ExcelError::NA();
}

private static function matchFirstValue($lookupArray, $lookupValue)
/**
* @param mixed $lookupValue
*
* @return mixed
*/
private static function matchFirstValue(array $lookupArray, $lookupValue)
{
$wildcardLookup = ((bool) preg_match('/([\?\*])/', $lookupValue));
$wildcard = WildcardMatch::wildcard($lookupValue);
if (is_string($lookupValue)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need an is not numeric check here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, although this is another confusing aspect of MATCH. If you look at the test to which I added the label 'string compared to number type -1', numeric 6 does not seem to match string '6', and that test returned #N/A both before and after this change, as does Excel. I will add more tests.

$valueIsString = true;
$wildcard = WildcardMatch::wildcard($lookupValue);
} else {
$valueIsString = false;
$wildcard = '';
}

$valueIsNumeric = is_int($lookupValue) || is_float($lookupValue);
foreach ($lookupArray as $i => $lookupArrayValue) {
$typeMatch = ((gettype($lookupValue) === gettype($lookupArrayValue)) ||
(is_numeric($lookupValue) && is_numeric($lookupArrayValue)));

if (
$typeMatch && is_string($lookupValue) &&
$wildcardLookup && WildcardMatch::compare($lookupArrayValue, $wildcard)
$valueIsString
&& is_string($lookupArrayValue)
) {
// wildcard match
return $i;
} elseif ($lookupArrayValue === $lookupValue) {
// exact match
return $i;
if (WildcardMatch::compare($lookupArrayValue, $wildcard)) {
return $i; // wildcard match
}
} else {
if ($lookupArrayValue === $lookupValue) {
return $i; // exact match
}
if (
$valueIsNumeric
&& (is_float($lookupArrayValue) || is_int($lookupArrayValue))
&& $lookupArrayValue == $lookupValue
) {
return $i; // exact match
}
}
}

return null;
}

private static function matchLargestValue($lookupArray, $lookupValue, $keySet)
/**
* @param mixed $lookupValue
*
* @return mixed
*/
private static function matchLargestValue(array $lookupArray, $lookupValue, array $keySet)
{
if (is_string($lookupValue)) {
foreach ($lookupArray as $i => $lookupArrayValue) {
if ($lookupArrayValue === $lookupValue) {
return $keySet[$i];
}
}
}
foreach ($lookupArray as $i => $lookupArrayValue) {
$typeMatch = ((gettype($lookupValue) === gettype($lookupArrayValue)) ||
(is_numeric($lookupValue) && is_numeric($lookupArrayValue)));
Expand All @@ -125,21 +154,32 @@ private static function matchLargestValue($lookupArray, $lookupValue, $keySet)
return null;
}

private static function matchSmallestValue($lookupArray, $lookupValue)
/**
* @param mixed $lookupValue
*
* @return mixed
*/
private static function matchSmallestValue(array $lookupArray, $lookupValue)
{
$valueKey = null;

$valueIsNumeric = is_int($lookupValue) || is_float($lookupValue);
// The basic algorithm is:
// Iterate and keep the highest match until the next element is smaller than the searched value.
// Return immediately if perfect match is found
foreach ($lookupArray as $i => $lookupArrayValue) {
$typeMatch = gettype($lookupValue) === gettype($lookupArrayValue);
$bothNumeric = $valueIsNumeric && (is_int($lookupArrayValue) || is_float($lookupArrayValue));

if ($lookupArrayValue === $lookupValue) {
// Another "special" case. If a perfect match is found,
// the algorithm gives up immediately
return $i;
} elseif ($typeMatch && $lookupArrayValue >= $lookupValue) {
}
if ($bothNumeric && $lookupValue == $lookupArrayValue) {
return $i; // exact match, as above
}
if (($typeMatch || $bothNumeric) && $lookupArrayValue >= $lookupValue) {
$valueKey = $i;
} elseif ($typeMatch && $lookupArrayValue < $lookupValue) {
//Excel algorithm gives up immediately if the first element is smaller than the searched value
Expand All @@ -150,6 +190,9 @@ private static function matchSmallestValue($lookupArray, $lookupValue)
return $valueKey;
}

/**
* @param mixed $lookupValue
*/
private static function validateLookupValue($lookupValue): void
{
// Lookup_value type has to be number, text, or logical values
Expand All @@ -158,6 +201,9 @@ private static function validateLookupValue($lookupValue): void
}
}

/**
* @param mixed $matchType
*/
private static function validateMatchType($matchType): void
{
// Match_type is 0, 1 or -1
Expand All @@ -169,7 +215,7 @@ private static function validateMatchType($matchType): void
}
}

private static function validateLookupArray($lookupArray): void
private static function validateLookupArray(array $lookupArray): void
{
// Lookup_array should not be empty
$lookupArraySize = count($lookupArray);
Expand All @@ -178,7 +224,10 @@ private static function validateLookupArray($lookupArray): void
}
}

private static function prepareLookupArray($lookupArray, $matchType)
/**
* @param mixed $matchType
*/
private static function prepareLookupArray(array $lookupArray, $matchType): array
{
// Lookup_array should contain only number, text, or logical values, or empty (null) cells
foreach ($lookupArray as $i => $value) {
Expand Down
104 changes: 97 additions & 7 deletions tests/data/Calculation/LookupRef/MATCH.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
<?php

return [
'unsorted numeric array still finds match with type 1?' => [
2, // Expected
2, // Input
[2, 0, 4, 3],
1,
],
'unsorted array still finds match with type 1?' => [
6,
'Amplitude',
['Aardvark', 'Apple', 'A~*e', 'A*e', 'A[solve', 'Amplitude', 'Adverse', 'Apartment'],
1,
],
// Third argument = 0
[
1, // Expected
Expand Down Expand Up @@ -33,12 +45,6 @@
[2, 3, 4, 3],
1,
],
[
2, // Expected
2, // Input
[2, 0, 4, 3],
1,
],
[
3, // Expected
2, // Input
Expand Down Expand Up @@ -201,7 +207,7 @@
[true, false, 'a', 'z', 2, 888],
-1,
],
[
'string compared to number type -1' => [
'#N/A', // Expected
6,
['6'],
Expand Down Expand Up @@ -310,6 +316,18 @@
['Aardvark', 'Apple', 'Armadillo', 'Acre', 'Absolve', 'Amplitude', 'Adverse', 'Apartment'],
0,
],
'wildcard match with tilde' => [
4,
'A~*e',
['Aardvark', 'Apple', 'A~*e', 'A*e', 'Absolve', 'Amplitude', 'Adverse', 'Apartment'],
0,
],
'string with preg_quote escaped character' => [
5,
'A[solve',
['Aardvark', 'Apple', 'A~*e', 'A*e', 'A[solve', 'Amplitude', 'Adverse', 'Apartment'],
0,
],
[
'#N/A',
'A*e',
Expand Down Expand Up @@ -358,4 +376,76 @@
['abc123fff', 'abc/123fff'],
0,
],
'float lookup int array type0' => [
1, // Expected
2.0, // Input
[2, 3, 4, 5],
0,
],
'int lookup float array type0' => [
2, // Expected
3, // Input
[2, 3.0, 4, 5],
0,
],
'int lookup float array type0 not equal' => [
'#N/A', // Expected
3, // Input
[2, 3.1, 4, 5],
0,
],
'float lookup int array type0 not equal' => [
'#N/A', // Expected
3.1, // Input
[2, 3, 4, 5],
0,
],
'float lookup int array type1 equal' => [
1, // Expected
2.0, // Input
[2, 3, 4, 5],
1,
],
'int lookup float array type1 equal' => [
2, // Expected
3, // Input
[2, 3.0, 4, 5],
1,
],
'float lookup int array type1 less' => [
1, // Expected
2.5, // Input
[2, 3, 4, 5],
1,
],
'int lookup float array type1 less' => [
2, // Expected
3, // Input
[2, 2.9, 4, 5],
1,
],
'float lookup int array type -1 equal' => [
4, // Expected
2.0, // Input
[5, 4, 3, 2],
-1,
],
'int lookup float array type -1 equal' => [
3, // Expected
3, // Input
[5, 4, 3.0, 2],
-1,
],
'float lookup int array type -1 greater' => [
2, // Expected
3.5, // Input
[5, 4, 3, 2],
-1,
],
'int lookup float array type -1 greater' => [
2, // Expected
3, // Input
[5, 4, 2.9, 2],
-1,
],
];