From fa5ab52b66ac4db36561acfe638cb535a4d5cab4 Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Tue, 20 Aug 2024 00:02:48 -0700
Subject: [PATCH] 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.
---
 .../Calculation/LookupRef/VLookup.php         |  9 ++++++--
 .../Functions/LookupRef/VLookupTest.php       | 21 +++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php b/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php
index 247074cfd..1c24b2a33 100644
--- a/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php
+++ b/src/PhpSpreadsheet/Calculation/LookupRef/VLookup.php
@@ -88,8 +88,8 @@ class VLookup extends LookupBase
 
         $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 @@ class VLookup extends LookupBase
 
         return $rowNumber;
     }
+
+    private static function numeric(mixed $value): bool
+    {
+        return is_int($value) || is_float($value);
+    }
 }
diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php
index b898dbd5f..ad320dfd0 100644
--- a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php
+++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 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 @@ class VLookupTest extends TestCase
             ],
         ];
     }
+
+    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();
+    }
 }