Real Errors Identified in Calculation by Scrutinizer (#2774)

* Real Errors Identified in Calculation by Scrutinizer

Before Scrutinizer broke, I took a look at the remaining 43 errors which it categorized as 'major'. Most of these were false positives, but, in the case of Calculation and Reader/Xlsx/Chart, I was able to determine that its analysis of some of the problems was correct. There is little point addressing the false positives until it starts working again, but we should fix the real errors.

This PR addresses the real errors in Calculation.
- A test for `$pCellParent` should have been a test  for `$pCellWorksheet`.
- A test for `$operand1Data['reference']` should have been a test for `$operand1Data['value']`.
- A test for `$operand2Data['reference']` should have been a test for `$operand2Data['value']`.

* Fix Attempt to Erroneously Call trim on Array

Fix #2780. Warning message is being issued when getting calculated value of cell with value `=INDIRECT(ADDRESS(ROW(),COLUMN()-2))/$C$4`. This appears to be the case for all recent (and probably not so recent) releases; it is not a result of changes to the code base. Fix added to this PR because the erring section of code was proximate to code already changed in the PR. Test added.

* Minor Code Changes

Apply some suggestions from @MarkBaker

phpstan-baseline.neon

@@ -20,16 +20,6 @@ parameters:
 			count: 6
 			path: src/PhpSpreadsheet/Calculation/Calculation.php
 
-		-
-			message: "#^Cannot call method getColumn\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
-			count: 2
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
-		-
-			message: "#^Cannot call method getCoordinate\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
-			count: 2
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
 		-
 			message: "#^Cannot call method getHighestColumn\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#"
 			count: 1
@@ -40,21 +30,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Calculation/Calculation.php
 
-		-
-			message: "#^Cannot call method getRow\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
-			count: 2
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
-		-
-			message: "#^Cannot call method getTitle\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
-		-
-			message: "#^Cannot call method has\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\|null\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
 		-
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Calculation\\:\\:_translateFormulaToEnglish\\(\\) has no return type specified\\.$#"
 			count: 1
@@ -140,11 +115,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Calculation/Calculation.php
 
-		-
-			message: "#^Parameter \\#1 \\$parent of method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\:\\:attach\\(\\) expects PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells, PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\|null given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Calculation/Calculation.php
-
 		-
 			message: "#^Parameter \\#1 \\$str of function preg_quote expects string, int\\|string given\\.$#"
 			count: 1

src/PhpSpreadsheet/Calculation/Calculation.php

@@ -3223,10 +3223,11 @@ class Calculation
                 //    So instead we skip replacing in any quoted strings by only replacing in every other array element
                 //       after we've exploded the formula
                 $temp = explode(self::FORMULA_STRING_QUOTE, $formula);
-                $i = false;
+                $notWithinQuotes = false;
                 foreach ($temp as &$value) {
                     //    Only adjust in alternating array entries
-                    if ($i = !$i) {
+                    $notWithinQuotes = !$notWithinQuotes;
+                    if ($notWithinQuotes === true) {
                         $value = self::translateFormulaBlock($from, $to, $value, $inFunctionBracesLevel, $inMatrixBracesLevel, $fromSeparator, $toSeparator);
                     }
                 }
@@ -3898,10 +3899,11 @@ class Calculation
                 $temp = explode(self::FORMULA_STRING_QUOTE, $formula);
                 //    Open and Closed counts used for trapping mismatched braces in the formula
                 $openCount = $closeCount = 0;
-                $i = false;
+                $notWithinQuotes = false;
                 foreach ($temp as &$value) {
                     //    Only count/replace in alternating array entries
-                    if ($i = !$i) {
+                    $notWithinQuotes = !$notWithinQuotes;
+                    if ($notWithinQuotes === true) {
                         $openCount += substr_count($value, self::FORMULA_OPEN_MATRIX_BRACE);
                         $closeCount += substr_count($value, self::FORMULA_CLOSE_MATRIX_BRACE);
                         $value = str_replace($matrixReplaceFrom, $matrixReplaceTo, $value);
@@ -4593,7 +4595,7 @@ class Calculation
                         if (strpos($operand1Data['reference'], '!') !== false) {
                             [$sheet1, $operand1Data['reference']] = Worksheet::extractSheetTitle($operand1Data['reference'], true);
                         } else {
-                            $sheet1 = ($pCellParent !== null) ? $pCellWorksheet->getTitle() : '';
+                            $sheet1 = ($pCellWorksheet !== null) ? $pCellWorksheet->getTitle() : '';
                         }
 
                         [$sheet2, $operand2Data['reference']] = Worksheet::extractSheetTitle($operand2Data['reference'], true);
@@ -4602,21 +4604,23 @@ class Calculation
                         }
 
                         if (trim($sheet1, "'") === trim($sheet2, "'")) {
-                            if ($operand1Data['reference'] === null) {
+                            if ($operand1Data['reference'] === null && $cell !== null) {
-                                if ((trim($operand1Data['value']) != '') && (is_numeric($operand1Data['value']))) {
+                                if (is_array($operand1Data['value'])) {
+                                    $operand1Data['reference'] = $cell->getCoordinate();
+                                } elseif ((trim($operand1Data['value']) != '') && (is_numeric($operand1Data['value']))) {
                                     $operand1Data['reference'] = $cell->getColumn() . $operand1Data['value'];
-                                // @phpstan-ignore-next-line
+                                } elseif (trim($operand1Data['value']) == '') {
-                                } elseif (trim($operand1Data['reference']) == '') {
                                     $operand1Data['reference'] = $cell->getCoordinate();
                                 } else {
                                     $operand1Data['reference'] = $operand1Data['value'] . $cell->getRow();
                                 }
                             }
-                            if ($operand2Data['reference'] === null) {
+                            if ($operand2Data['reference'] === null && $cell !== null) {
-                                if ((trim($operand2Data['value']) != '') && (is_numeric($operand2Data['value']))) {
+                                if (is_array($operand2Data['value'])) {
+                                    $operand2Data['reference'] = $cell->getCoordinate();
+                                } elseif ((trim($operand2Data['value']) != '') && (is_numeric($operand2Data['value']))) {
                                     $operand2Data['reference'] = $cell->getColumn() . $operand2Data['value'];
-                                // @phpstan-ignore-next-line
+                                } elseif (trim($operand2Data['value']) == '') {
-                                } elseif (trim($operand2Data['reference']) == '') {
                                     $operand2Data['reference'] = $cell->getCoordinate();
                                 } else {
                                     $operand2Data['reference'] = $operand2Data['value'] . $cell->getRow();
@@ -4829,7 +4833,7 @@ class Calculation
                             $this->debugLog->writeDebugLog('Evaluation Result for cell %s in worksheet %s is %s', $cellRef, $matches[2], $this->showTypeDetails($cellValue));
                         } else {
                             $this->debugLog->writeDebugLog('Evaluating Cell %s in current worksheet', $cellRef);
-                            if ($pCellParent->has($cellRef)) {
+                            if ($pCellParent !== null && $pCellParent->has($cellRef)) {
                                 $cellValue = $this->extractCellRange($cellRef, $pCellWorksheet, false);
                                 $cell->attach($pCellParent);
                             } else {
@@ -5281,7 +5285,7 @@ class Calculation
             }
 
             // Named range?
-            $namedRange = DefinedName::resolveName($range, $worksheet);
+            $namedRange = DefinedName::resolveName($range, /** @scrutinizer ignore-type */ $worksheet);
             if ($namedRange === null) {
                 return Information\ExcelError::REF();
             }

tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2778Test.php

@@ -0,0 +1,26 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+use PHPUnit\Framework\TestCase;
+
+class Issue2778Test extends TestCase
+{
+    /**
+     * @var string
+     */
+    private static $testbook = 'tests/data/Reader/XLSX/issue.2778.xlsx';
+
+    public function testIssue2778(): void
+    {
+        $filename = self::$testbook;
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame(6, $sheet->getCell('D1')->getCalculatedValue());
+        self::assertSame(63, $sheet->getCell('F1')->getCalculatedValue());
+        self::assertSame(10, $sheet->getCell('C10')->getCalculatedValue());
+        $spreadsheet->disconnectWorksheets();
+    }
+}