From 7d71a7ca54ea44f541d1479913d3b9876fbf1673 Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Mon, 3 Jan 2022 17:32:52 -0800
Subject: [PATCH] New Error Reported with Phpstan 1.3 (#2481)

* New Error Reported with Phpstan 1.3

Dependabot opened a number of PRs. Most are successful, but this change is necessary to allow PR #2477 to complete successfully, and that is apparently a necessity for PR #2479.

Phpstan 1.3 objected to:
```php
trigger_error($errorMessage, E_USER_ERROR);
return false;
```
It claims the return statement is unreachable. This isn't precisely true - an error handler might allow the return to be reached. At any rate, I have slightly restructured the code so that Phpstan will not object either with 1.2 or 1.3, which should allow the blocked PRs to succeed. There had been no previous tests for what happens when there is a formula error when suppressFormulaErrors is true.

* Scrutinizer

Didn't like effect of changes which Phpstan liked. Hopefully this will work better.

* Improvement

Eliminate added function.
---
 phpstan-baseline.neon                         | 10 ----
 .../Calculation/Calculation.php               | 13 +++--
 .../Calculation/CalculationErrorTest.php      | 54 +++++++++++++++++++
 3 files changed, 64 insertions(+), 13 deletions(-)
 create mode 100644 tests/PhpSpreadsheetTests/Calculation/CalculationErrorTest.php

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
index eaea1d3b..db35c729 100644
--- a/phpstan-baseline.neon
+++ b/phpstan-baseline.neon
@@ -160,16 +160,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Calculation/Calculation.php
 
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:raiseFormulaError\\(\\) has no return type specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
-		-
-			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:raiseFormulaError\\(\\) has parameter \\$errorMessage with no type specified\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
 		-
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:validateBinaryOperand\\(\\) has no return type specified\\.$#"
 			count: 1
diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php
index b473b27a..25f76959 100644
--- a/src/PhpSpreadsheet/Calculation/Calculation.php
+++ b/src/PhpSpreadsheet/Calculation/Calculation.php
@@ -5229,15 +5229,22 @@ class Calculation
         return $result;
     }
 
-    // trigger an error, but nicely, if need be
-    protected function raiseFormulaError($errorMessage)
+    /**
+     * Trigger an error, but nicely, if need be.
+     *
+     * @return false
+     */
+    protected function raiseFormulaError(string $errorMessage)
     {
         $this->formulaError = $errorMessage;
         $this->cyclicReferenceStack->clear();
         if (!$this->suppressFormulaErrors) {
             throw new Exception($errorMessage);
         }
-        trigger_error($errorMessage, E_USER_ERROR);
+
+        if (strlen($errorMessage) > 0) {
+            trigger_error($errorMessage, E_USER_ERROR);
+        }
 
         return false;
     }
diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationErrorTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationErrorTest.php
new file mode 100644
index 00000000..23ee5ba8
--- /dev/null
+++ b/tests/PhpSpreadsheetTests/Calculation/CalculationErrorTest.php
@@ -0,0 +1,54 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Calculation;
+
+use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException;
+use PHPUnit\Framework\TestCase;
+use Throwable;
+
+class CalculationErrorTest extends TestCase
+{
+    public function testCalculationException(): void
+    {
+        $this->expectException(CalcException::class);
+        $this->expectExceptionMessage('Formula Error:');
+        $calculation = Calculation::getInstance();
+        $result = $calculation->_calculateFormulaValue('=SUM(');
+        self::assertFalse($result);
+    }
+
+    public function testCalculationError(): void
+    {
+        $calculation = Calculation::getInstance();
+        $calculation->suppressFormulaErrors = true;
+        $error = false;
+
+        try {
+            $calculation->_calculateFormulaValue('=SUM(');
+        } catch (Throwable $e) {
+            self::assertSame("Formula Error: Expecting ')'", $e->getMessage());
+            self::assertSame('PHPUnit\\Framework\\Error\\Error', get_class($e));
+            $error = true;
+        }
+        self::assertTrue($error);
+    }
+
+    /**
+     * @param mixed $args
+     */
+    public static function errhandler2(...$args): bool
+    {
+        return $args[0] === E_USER_ERROR;
+    }
+
+    public function testCalculationErrorTrulySuppressed(): void
+    {
+        $calculation = Calculation::getInstance();
+        $calculation->suppressFormulaErrors = true;
+        set_error_handler([self::class, 'errhandler2']);
+        $result = $calculation->_calculateFormulaValue('=SUM(');
+        restore_error_handler();
+        self::assertFalse($result);
+    }
+}