IF Empty Arguments

Fix #3875. Even better, fix #2146, which has been open for 2.5 years. Empty arguments are improperly placed on the stack; in particular, they are added without `onlyIf` and `onlyIfNot` attributes.This results in problems described in 3875. IF has a somewhat unexpected design. In Excel, `IF(false, valueIfTrue)` evaluates as `false`, but `IF(false, valueIfTrue,)` evaluates as 0. This means that IF empty arguments should be handled in the same manner as MIN/MAX/MINA/MAXA, but you need to be careful to distinguish empty from omitted. Also note that IF requires 2 operands - `IF(true)` is an error, but `IF(true,)` evaluates to 0.
PHPOffice · Jan 27, 2024 · e1bcab6 · e1bcab6
1 parent 570c86f
commit e1bcab6
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ## Unreleased - TBD
 
+### Added
+
+- Nothing
+
+### Changed
+
+- Nothing
+
+### Deprecated
+
+- Nothing
+
+### Removed
+
+- Nothing
+
+### Fixed
+
+- IF Empty Arguments. [Issue #3875](https://github.com/PHPOffice/PhpSpreadsheet/issues/3875) [Issue #2146](https://github.com/PHPOffice/PhpSpreadsheet/issues/2146) [PR #3879](https://github.com/PHPOffice/PhpSpreadsheet/pull/3879)
+
+## 2.0.0 - 2024-01-04
+
 ### BREAKING CHANGE
 
 - Typing was strengthened by leveraging native typing. This should not change any behavior. However, if you implement

diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php
@@ -1307,7 +1307,7 @@ public static function getExcelConstants(string $key): bool|null
         'IF' => [
             'category' => Category::CATEGORY_LOGICAL,
             'functionCall' => [Logical\Conditional::class, 'statementIf'],
-            'argumentCount' => '1-3',
+            'argumentCount' => '2-3',
         ],
         'IFERROR' => [
             'category' => Category::CATEGORY_LOGICAL,
@@ -4189,7 +4189,7 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
                 //    If we've a comma when we're expecting an operand, then what we actually have is a null operand;
                 //        so push a null onto the stack
                 if (($expectingOperand) || (!$expectingOperator)) {
-                    $output[] = ['type' => 'Empty Argument', 'value' => self::$excelConstants['NULL'], 'reference' => 'NULL'];
+                    $output[] = $stack->getStackItem('Empty Argument', null, 'NULL');
                 }
                 // make sure there was a function
                 $d = $stack->last(2);
@@ -4436,7 +4436,7 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
                 ++$index;
             } elseif ($opCharacter === ')') { // miscellaneous error checking
                 if ($expectingOperand) {
-                    $output[] = ['type' => 'Empty Argument', 'value' => self::$excelConstants['NULL'], 'reference' => 'NULL'];
+                    $output[] = $stack->getStackItem('Empty Argument', null, 'NULL');
                     $expectingOperand = false;
                     $expectingOperator = true;
                 } else {
@@ -4996,11 +4996,12 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
                                 }
                             }
                         } else {
-                            $emptyArguments[] = ($arg['type'] === 'Empty Argument');
-                            if ($arg['type'] === 'Empty Argument' && in_array($functionName, ['MIN', 'MINA', 'MAX', 'MAXA'], true)) {
+                            if ($arg['type'] === 'Empty Argument' && in_array($functionName, ['MIN', 'MINA', 'MAX', 'MAXA', 'IF'], true)) {
+                                $emptyArguments[] = false;
                                 $args[] = $arg['value'] = 0;
                                 $this->debugLog->writeDebugLog('Empty Argument reevaluated as 0');
                             } else {
+                                $emptyArguments[] = $arg['type'] === 'Empty Argument';
                                 $args[] = self::unwrapResult($arg['value']);
                             }
                             if ($functionName !== 'MKMATRIX') {

diff --git a/tests/PhpSpreadsheetTests/Calculation/MissingArgumentsTest.php b/tests/PhpSpreadsheetTests/Calculation/MissingArgumentsTest.php
@@ -4,6 +4,7 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Calculation;
 
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
@@ -17,7 +18,14 @@ public function testMissingArguments(mixed $expected, string $formula): void
         $spreadsheet = new Spreadsheet();
         $sheet = $spreadsheet->getActiveSheet();
         $sheet->getCell('A1')->setValue($formula);
-        self::assertSame($expected, $sheet->getCell('A1')->getCalculatedValue());
+        $sheet->getCell('B1')->setValue(1);
+
+        try {
+            self::assertSame($expected, $sheet->getCell('A1')->getCalculatedValue());
+        } catch (CalcExp $e) {
+            self::assertSame('exception', $expected);
+        }
+
         $spreadsheet->disconnectWorksheets();
     }
 
@@ -35,6 +43,20 @@ public static function providerMissingArguments(): array
             'product ignores null argument' => [6.0, '=product(3,2,)'],
             'embedded function' => [5, '=sum(3,2,min(3,2,))'],
             'unaffected embedded function' => [8, '=sum(3,2,max(3,2,))'],
+            'if true missing at end' => [0, '=if(b1=1,min(3,2,),product(3,2,))'],
+            'if false missing at end' => [6.0, '=if(b1=2,min(3,2,),product(3,2,))'],
+            'if true missing in middle' => [0, '=if(b1=1,min(3,,2),product(3,,2))'],
+            'if false missing in middle' => [6.0, '=if(b1=2,min(3,,2),product(3,,2))'],
+            'if true missing at beginning' => [0, '=if(b1=1,min(,3,2),product(,3,2))'],
+            'if false missing at beginning' => [6.0, '=if(b1=2,min(,3,2),product(,3,2))'],
+            'if true nothing missing' => [2, '=if(b1=1,min(3,2),product(3,2))'],
+            'if false nothing missing' => [6.0, '=if(b1=2,min(3,2),product(3,2))'],
+            'if true empty arg' => [0, '=if(b1=1,)'],
+            'if true omitted args' => ['exception', '=if(b1=1)'],
+            'if true missing arg' => [0, '=if(b1=1,,6)'],
+            'if false missing arg' => [0, '=if(b1=2,6,)'],
+            'if false omitted arg' => [false, '=if(b1=2,6)'],
+            'multiple ifs and omissions' => [0, '=IF(0<9,,IF(0=0,,1))'],
         ];
     }
 }