Range operator tests (#1501)

* Improved handling of named ranges, although there are still some issues (names ranges using a union type with an overlap don't handle the overlap twice, which as the MS Excel approach to set overlaps as opposed to the mathematical approach which only applies overlap values once)

* Fix tests that misused space and comma as simple separators in cell ranges

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'];

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
+        $cells = [];
-        $cellBlocks = self::getCellBlocksFromRangeString($pRange);
+        foreach ($ranges as $range) {
-        foreach ($cellBlocks as $cellBlock) {
+            $cells[] = self::getReferencesForCellBlock($range);
-            $returnValue = array_merge($returnValue, self::getReferencesForCellBlock($cellBlock));
         }
 
+        $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) {
+        foreach ($cellList as $coord) {
-            $column = '';
+            [$column, $row] = sscanf($coord, '%[A-Z]%d');
-            $row = 0;
-
-            sscanf($coord, '%[A-Z]%d', $column, $row);
             $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];
     }
 
     /**

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();
+        $sumRresult = $workSheet->getCell('E1')->getCalculatedValue();
-        self::assertSame($expectedResult, $actualRresult);
+        self::assertSame($expectedResult, $sumRresult);
     }
 
     public function providerNamedRangeEvaluation()
     {
         return[
-            [
+            ['A1:B3', 'A1:C2', '=SUM(GROUP1,GROUP2)', 48],
-                'A1:B3',
+            ['A1:B3', 'A1:C2', '=COUNT(GROUP1,GROUP2)', 12],
-                'A1:C2',
+            ['A1:B3', 'A1:C2', '=SUM(GROUP1 GROUP2)', 12],
-                '=SUM(GROUP1,GROUP2)',
+            ['A1:B3', 'A1:C2', '=COUNT(GROUP1 GROUP2)', 4],
-                48,
+            ['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:B3',
+            ['A1:B2', 'B2:C3', '=COUNT(GROUP1 GROUP2)', 1],
-                'A1:C2',
+        ];
-                '=SUM(GROUP1 GROUP2)',
+    }
-                12,
+
-            ],
+    /**
-            [
+     * @dataProvider providerCompositeNamedRangeEvaluation
-                'A1:B2',
+     *
-                'B2:C3',
+     * @param string $composite
-                '=SUM(GROUP1,GROUP2)',
+     * @param int $expectedSum
-                40,
+     * @param int $expectedCount
-            ],
+     */
-            [
+    public function testCompositeNamedRangeEvaluation($composite, $expectedSum, $expectedCount): void
-                'A1:B2',
+    {
-                'B2:C3',
+        $workSheet = $this->spreadSheet->getActiveSheet();
-                '=SUM(GROUP1 GROUP2)',
+        $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,
             ],
         ];

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);
     }
 

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',
     ],
     [
         [