diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 5aa309c5..2d67a134 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -3625,7 +3625,6 @@ class Calculation $expectingOperand = false; $val = $match[1]; $length = strlen($val); - if (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/i', $val, $matches)) { $val = preg_replace('/\s/u', '', $val); if (isset(self::$phpSpreadsheetFunctions[strtoupper($matches[1])]) || isset(self::$controlFunctions[strtoupper($matches[1])])) { // it's a function @@ -3660,7 +3659,6 @@ class Calculation } elseif (preg_match('/^' . self::CALCULATION_REGEXP_CELLREF . '$/i', $val, $matches)) { // Watch for this case-change when modifying to allow cell references in different worksheets... // Should only be applied to the actual cell column, not the worksheet name - // If the last entry on the stack was a : operator, then we have a cell range reference $testPrevOp = $stack->last(1); if ($testPrevOp !== null && $testPrevOp['value'] == ':') { @@ -3717,6 +3715,8 @@ class Calculation } $localeConstant = false; + $stackItemType = 'Value'; + $stackItemReference = null; if ($opCharacter == self::FORMULA_STRING_QUOTE) { // UnEscape any quotes within the string $val = self::wrapResult(str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($val))); @@ -3727,12 +3727,17 @@ class Calculation $val = (int) $val; } } elseif (isset(self::$excelConstants[trim(strtoupper($val))])) { + $stackItemType = 'Constant'; $excelConstant = trim(strtoupper($val)); $val = self::$excelConstants[$excelConstant]; } elseif (($localeConstant = array_search(trim(strtoupper($val)), self::$localeBoolean)) !== false) { + $stackItemType = 'Constant'; $val = self::$excelConstants[$localeConstant]; + } elseif (preg_match('/^' . self::CALCULATION_REGEXP_NAMEDRANGE . '.*/Ui', $val, $match)) { + $stackItemType = 'Named Range'; + $stackItemReference = $val; } - $details = $stack->getStackItem('Value', $val, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + $details = $stack->getStackItem($stackItemType, $val, $stackItemReference, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); if ($localeConstant) { $details['localeValue'] = $localeConstant; } @@ -3842,7 +3847,6 @@ class Calculation $fakedForBranchPruning = []; // help us to know when pruning ['branchTestId' => true/false] $branchStore = []; - // Loop through each token in turn foreach ($tokens as $tokenData) { $token = $tokenData['value']; diff --git a/src/PhpSpreadsheet/Cell/Coordinate.php b/src/PhpSpreadsheet/Cell/Coordinate.php index 8c679913..2afeebe9 100644 --- a/src/PhpSpreadsheet/Cell/Coordinate.php +++ b/src/PhpSpreadsheet/Cell/Coordinate.php @@ -312,32 +312,59 @@ abstract class Coordinate /** * Extract all cell references in range, which may be comprised of multiple cell ranges. * - * @param string $pRange Range (e.g. A1 or A1:C10 or A1:E10 A20:E25) + * @param string $cellRange Range: e.g. 'A1' or 'A1:C10' or 'A1:E10,A20:E25' or 'A1:E5 C3:G7' or 'A1:C1,A3:C3 B1:C3' * * @return array Array containing single cell references */ - public static function extractAllCellReferencesInRange($pRange) + public static function extractAllCellReferencesInRange($cellRange): array { - $returnValue = []; + [$ranges, $operators] = self::getCellBlocksFromRangeString($cellRange); - // Explode spaces - $cellBlocks = self::getCellBlocksFromRangeString($pRange); - foreach ($cellBlocks as $cellBlock) { - $returnValue = array_merge($returnValue, self::getReferencesForCellBlock($cellBlock)); + $cells = []; + foreach ($ranges as $range) { + $cells[] = self::getReferencesForCellBlock($range); } + $cells = self::processRangeSetOperators($operators, $cells); + + if (empty($cells)) { + return []; + } + + $cellList = array_merge(...$cells); + $cellList = self::sortCellReferenceArray($cellList); + + return $cellList; + } + + private static function processRangeSetOperators(array $operators, array $cells): array + { + for ($offset = 0; $offset < count($operators); ++$offset) { + $operator = $operators[$offset]; + if ($operator !== ' ') { + continue; + } + + $cells[$offset] = array_intersect($cells[$offset], $cells[$offset + 1]); + unset($operators[$offset], $cells[$offset + 1]); + $operators = array_values($operators); + $cells = array_values($cells); + --$offset; + } + + return $cells; + } + + private static function sortCellReferenceArray(array $cellList): array + { // Sort the result by column and row $sortKeys = []; - foreach (array_unique($returnValue) as $coord) { - $column = ''; - $row = 0; - - sscanf($coord, '%[A-Z]%d', $column, $row); + foreach ($cellList as $coord) { + [$column, $row] = sscanf($coord, '%[A-Z]%d'); $sortKeys[sprintf('%3s%09d', $column, $row)] = $coord; } ksort($sortKeys); - // Return value return array_values($sortKeys); } @@ -482,15 +509,25 @@ abstract class Coordinate } /** - * Get the individual cell blocks from a range string, splitting by space and removing any $ characters. + * Get the individual cell blocks from a range string, removing any $ characters. + * then splitting by operators and returning an array with ranges and operators. * - * @param string $pRange + * @param string $rangeString * - * @return string[] + * @return array[] */ - private static function getCellBlocksFromRangeString($pRange) + private static function getCellBlocksFromRangeString($rangeString) { - return explode(' ', str_replace('$', '', strtoupper($pRange))); + $rangeString = str_replace('$', '', strtoupper($rangeString)); + + // split range sets on intersection (space) or union (,) operators + $tokens = preg_split('/([ ,])/', $rangeString, -1, PREG_SPLIT_DELIM_CAPTURE); + // separate the range sets and the operators into arrays + $split = array_chunk($tokens, 2); + $ranges = array_column($split, 0); + $operators = array_column($split, 1); + + return [$ranges, $operators]; } /** diff --git a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php index d1ad229b..ee566db2 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php @@ -45,11 +45,21 @@ class RangeTest extends TestCase { return[ ['=SUM(A1:B3,A1:C2)', 48], + ['=COUNT(A1:B3,A1:C2)', 12], ['=SUM(A1:B3 A1:C2)', 12], + ['=COUNT(A1:B3 A1:C2)', 4], ['=SUM(A1:A3,C1:C3)', 30], + ['=COUNT(A1:A3,C1:C3)', 6], ['=SUM(A1:A3 C1:C3)', Functions::null()], + ['=COUNT(A1:A3 C1:C3)', 0], ['=SUM(A1:B2,B2:C3)', 40], + ['=COUNT(A1:B2,B2:C3)', 8], ['=SUM(A1:B2 B2:C3)', 5], + ['=COUNT(A1:B2 B2:C3)', 1], + ['=SUM(A1:C1,A3:C3,B1:C3)', 63], + ['=COUNT(A1:C1,A3:C3,B1:C3)', 12], + ['=SUM(A1:C1,A3:C3 B1:C3)', 23], + ['=COUNT(A1:C1,A3:C3 B1:C3)', 5], ]; } @@ -69,35 +79,57 @@ class RangeTest extends TestCase $workSheet->setCellValue('E1', $formula); - $actualRresult = $workSheet->getCell('E1')->getCalculatedValue(); - self::assertSame($expectedResult, $actualRresult); + $sumRresult = $workSheet->getCell('E1')->getCalculatedValue(); + self::assertSame($expectedResult, $sumRresult); } public function providerNamedRangeEvaluation() { return[ - [ - 'A1:B3', - 'A1:C2', - '=SUM(GROUP1,GROUP2)', - 48, - ], - [ - 'A1:B3', - 'A1:C2', - '=SUM(GROUP1 GROUP2)', - 12, - ], - [ - 'A1:B2', - 'B2:C3', - '=SUM(GROUP1,GROUP2)', - 40, - ], - [ - 'A1:B2', - 'B2:C3', - '=SUM(GROUP1 GROUP2)', + ['A1:B3', 'A1:C2', '=SUM(GROUP1,GROUP2)', 48], + ['A1:B3', 'A1:C2', '=COUNT(GROUP1,GROUP2)', 12], + ['A1:B3', 'A1:C2', '=SUM(GROUP1 GROUP2)', 12], + ['A1:B3', 'A1:C2', '=COUNT(GROUP1 GROUP2)', 4], + ['A1:B2', 'B2:C3', '=SUM(GROUP1,GROUP2)', 40], + ['A1:B2', 'B2:C3', '=COUNT(GROUP1,GROUP2)', 8], + ['A1:B2', 'B2:C3', '=SUM(GROUP1 GROUP2)', 5], + ['A1:B2', 'B2:C3', '=COUNT(GROUP1 GROUP2)', 1], + ]; + } + + /** + * @dataProvider providerCompositeNamedRangeEvaluation + * + * @param string $composite + * @param int $expectedSum + * @param int $expectedCount + */ + public function testCompositeNamedRangeEvaluation($composite, $expectedSum, $expectedCount): void + { + $workSheet = $this->spreadSheet->getActiveSheet(); + $this->spreadSheet->addNamedRange(new NamedRange('COMPOSITE', $workSheet, $composite)); + + $workSheet->setCellValue('E1', '=SUM(COMPOSITE)'); + $workSheet->setCellValue('E2', '=COUNT(COMPOSITE)'); + + $actualSum = $workSheet->getCell('E1')->getCalculatedValue(); + self::assertSame($expectedSum, $actualSum); + $actualCount = $workSheet->getCell('E2')->getCalculatedValue(); + self::assertSame($expectedCount, $actualCount); + } + + public function providerCompositeNamedRangeEvaluation() + { + return[ + // Calculation engine doesn't yet handle union ranges with overlap + // 'Union with overlap' => [ + // 'A1:C1,A3:C3,B1:C3', + // 63, + // 12, + // ], + 'Intersection' => [ + 'A1:C1,A3:C3 B1:C3', + 23, 5, ], ]; diff --git a/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php b/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php index 37579e80..8e0e98a9 100644 --- a/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php +++ b/tests/PhpSpreadsheetTests/Cell/CoordinateTest.php @@ -83,10 +83,11 @@ class CoordinateTest extends TestCase * @dataProvider providerCoordinates * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testCoordinateFromString($expectedResult, ...$args): void + public function testCoordinateFromString($expectedResult, $rangeSet): void { - $result = Coordinate::coordinateFromString(...$args); + $result = Coordinate::coordinateFromString($rangeSet); self::assertEquals($expectedResult, $result); } @@ -143,11 +144,12 @@ class CoordinateTest extends TestCase /** * @dataProvider providerAbsoluteCoordinates * - * @param mixed $expectedResult + * @param string $expectedResult + * @param string $rangeSet */ - public function testAbsoluteCoordinateFromString($expectedResult, ...$args): void + public function testAbsoluteCoordinateFromString($expectedResult, $rangeSet): void { - $result = Coordinate::absoluteCoordinate(...$args); + $result = Coordinate::absoluteCoordinate($rangeSet); self::assertEquals($expectedResult, $result); } @@ -175,10 +177,11 @@ class CoordinateTest extends TestCase * @dataProvider providerAbsoluteReferences * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testAbsoluteReferenceFromString($expectedResult, ...$args): void + public function testAbsoluteReferenceFromString($expectedResult, $rangeSet): void { - $result = Coordinate::absoluteReference(...$args); + $result = Coordinate::absoluteReference($rangeSet); self::assertEquals($expectedResult, $result); } @@ -206,10 +209,11 @@ class CoordinateTest extends TestCase * @dataProvider providerSplitRange * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testSplitRange($expectedResult, ...$args): void + public function testSplitRange($expectedResult, $rangeSet): void { - $result = Coordinate::splitRange(...$args); + $result = Coordinate::splitRange($rangeSet); foreach ($result as $key => $split) { if (!is_array($expectedResult[$key])) { self::assertEquals($expectedResult[$key], $split[0]); @@ -252,10 +256,11 @@ class CoordinateTest extends TestCase * @dataProvider providerRangeBoundaries * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testRangeBoundaries($expectedResult, ...$args): void + public function testRangeBoundaries($expectedResult, $rangeSet): void { - $result = Coordinate::rangeBoundaries(...$args); + $result = Coordinate::rangeBoundaries($rangeSet); self::assertEquals($expectedResult, $result); } @@ -268,10 +273,11 @@ class CoordinateTest extends TestCase * @dataProvider providerRangeDimension * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testRangeDimension($expectedResult, ...$args): void + public function testRangeDimension($expectedResult, $rangeSet): void { - $result = Coordinate::rangeDimension(...$args); + $result = Coordinate::rangeDimension($rangeSet); self::assertEquals($expectedResult, $result); } @@ -284,10 +290,11 @@ class CoordinateTest extends TestCase * @dataProvider providerGetRangeBoundaries * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testGetRangeBoundaries($expectedResult, ...$args): void + public function testGetRangeBoundaries($expectedResult, $rangeSet): void { - $result = Coordinate::getRangeBoundaries(...$args); + $result = Coordinate::getRangeBoundaries($rangeSet); self::assertEquals($expectedResult, $result); } @@ -299,11 +306,12 @@ class CoordinateTest extends TestCase /** * @dataProvider providerExtractAllCellReferencesInRange * - * @param mixed $expectedResult + * @param array $expectedResult + * @param string $rangeSet */ - public function testExtractAllCellReferencesInRange($expectedResult, ...$args): void + public function testExtractAllCellReferencesInRange($expectedResult, $rangeSet): void { - $result = Coordinate::extractAllCellReferencesInRange(...$args); + $result = Coordinate::extractAllCellReferencesInRange($rangeSet); self::assertEquals($expectedResult, $result); } @@ -350,10 +358,11 @@ class CoordinateTest extends TestCase * @dataProvider providerCoordinateIsRange * * @param mixed $expectedResult + * @param string $rangeSet */ - public function testCoordinateIsRange($expectedResult, ...$args): void + public function testCoordinateIsRange($expectedResult, $rangeSet): void { - $result = Coordinate::coordinateIsRange(...$args); + $result = Coordinate::coordinateIsRange($rangeSet); self::assertEquals($expectedResult, $result); } diff --git a/tests/data/CellExtractAllCellReferencesInRange.php b/tests/data/CellExtractAllCellReferencesInRange.php index cf093289..b005b1fe 100644 --- a/tests/data/CellExtractAllCellReferencesInRange.php +++ b/tests/data/CellExtractAllCellReferencesInRange.php @@ -22,12 +22,6 @@ return [ ], [ [ - 'B4', - 'B5', - 'B6', - 'D4', - 'D5', - 'D6', ], 'B4:B6 D4:D6', ], @@ -66,20 +60,10 @@ return [ ], [ [ - 'B4', - 'B5', - 'B6', - 'C4', 'C5', 'C6', - 'C7', - 'D4', 'D5', 'D6', - 'D7', - 'E5', - 'E6', - 'E7', ], 'B4:D6 C5:E7', ], @@ -105,7 +89,7 @@ return [ 'F5', 'F6', ], - 'B2:D4 C5:D5 E3:E5 D6:E6 F4:F6', + 'B2:D4,C5:D5,E3:E5,D6:E6,F4:F6', ], [ [ @@ -129,16 +113,13 @@ return [ 'F5', 'F6', ], - 'B2:D4 C3:E5 D4:F6', + 'B2:D4,C3:E5,D4:F6', ], [ [ - 'B4', 'B5', - 'B6', - 'B8', ], - 'B4:B6 B8', + 'B4:B6 B5', ], [ [