VLOOKUP Handling of Strings and Number

Fix #1402, another in our "better late than never" series. Stalebot closed it in May 2020, and I have reopened it. @ljcag submitted the issue, and PR #1403 to resolve it, also marked stale. That change is more complicated than this one. Since there were no tests in that PR, and since this PR solves the original problem (test added), I will stick with this version, but will continue to study 1403 before merging.

This is also another in the "Excel doesn't believe in complete documentation" series. (See issue #3802 for a similar example.) Nothing that I have seen in the documentation suggests that a number will not match a numeric string, but that seems clearly to be the case. Despite the lack of complete documentation, PhpSpreadsheet implemented VLOOKUP with that in mind. Unfortunately, it did so by using `is_numeric` on its comparands, and so treats numeric strings as if they were numbers. This PR replaces those tests with `is_int() || is_float()`. The earlier PR used `is_string()` instead as a proxy for "not numeric" but that required other changes.

Author: oleibman

src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php

@@ -88,8 +88,8 @@ private static function vLookupSearch(mixed $lookupValue, array $lookupArray, $c
 
         $rowNumber = null;
         foreach ($lookupArray as $rowKey => $rowData) {
-            $bothNumeric = is_numeric($lookupValue) && is_numeric($rowData[$column]);
-            $bothNotNumeric = !is_numeric($lookupValue) && !is_numeric($rowData[$column]);
+            $bothNumeric = self::numeric($lookupValue) && self::numeric($rowData[$column]);
+            $bothNotNumeric = !self::numeric($lookupValue) && !self::numeric($rowData[$column]);
             $cellDataLower = StringHelper::strToLower((string) $rowData[$column]);
 
             // break if we have passed possible keys
@@ -114,4 +114,9 @@ private static function vLookupSearch(mixed $lookupValue, array $lookupArray, $c
 
         return $rowNumber;
     }
+
+    private static function numeric(mixed $value): bool
+    {
+        return is_int($value) || is_float($value);
+    }
 }

tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php

@@ -5,6 +5,7 @@
 namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\LookupRef;
 
 use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
+use PhpOffice\PhpSpreadsheet\Cell\DataType;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
@@ -78,4 +79,24 @@ public static function providerVLookupArray(): array
             ],
         ];
     }
+
+    public function testIssue1402(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $worksheet = $spreadsheet->getActiveSheet();
+
+        $worksheet->setCellValueExplicit('A1', 1, DataType::TYPE_STRING);
+        $worksheet->setCellValue('B1', 'Text Nr 1');
+        $worksheet->setCellValue('A2', 2);
+        $worksheet->setCellValue('B2', 'Numeric result');
+        $worksheet->setCellValueExplicit('A3', 2, DataType::TYPE_STRING);
+        $worksheet->setCellValue('B3', 'Text Nr 2');
+        $worksheet->setCellValueExplicit('A4', 2, DataType::TYPE_STRING);
+        $worksheet->setCellValue('B4', '=VLOOKUP(A4,$A$1:$B$3,2,0)');
+        self::assertSame('Text Nr 2', $worksheet->getCell('B4')->getCalculatedValue());
+        $worksheet->setCellValue('A5', 2);
+        $worksheet->setCellValue('B5', '=VLOOKUP(A5,$A$1:$B$3,2,0)');
+        self::assertSame('Numeric result', $worksheet->getCell('B5')->getCalculatedValue());
+        $spreadsheet->disconnectWorksheets();
+    }
 }