Merge pull request #4896 from oleibman/chartindex

getChartIndex

src/PhpSpreadsheet/Worksheet/Worksheet.php

@@ -569,14 +569,14 @@ class Worksheet
     /**
      * Get a chart by its index position.
      *
-     * @param ?string $index Chart index position
+     * @param null|int|string $index Chart index position
      *
      * @return Chart|false
      */
-    public function getChartByIndex(?string $index)
+    public function getChartByIndex(null|int|string $index)
     {
         $chartCount = count($this->chartCollection);
-        if ($chartCount == 0) {
+        if ($chartCount === 0 || (is_string($index) && $index !== (string) (int) $index)) {
             return false;
         }
         if ($index === null) {

src/PhpSpreadsheet/Writer/Xlsx/Drawing.php

@@ -60,15 +60,8 @@ class Drawing extends WriterPart
         }
 
         if ($includeCharts) {
-            $chartCount = $worksheet->getChartCount();
-            // Loop through charts and write the chart position
-            if ($chartCount > 0) {
-                for ($c = 0; $c < $chartCount; ++$c) {
-                    $chart = $worksheet->getChartByIndex((string) $c);
-                    if ($chart !== false) {
-                        $this->writeChart($objWriter, $chart, $c + $i);
-                    }
-                }
+            foreach ($worksheet->getChartCollection() as $c => $chart) {
+                $this->writeChart($objWriter, $chart, $c + $i);
             }
         }
 

tests/PhpSpreadsheetTests/Chart/ChartsByNameTest.php

@@ -19,6 +19,7 @@ class ChartsByNameTest extends TestCase
         $spreadsheet = new Spreadsheet();
         $sheet = $spreadsheet->getActiveSheet();
         $sheet->setTitle('Only Sheet');
+        self::assertFalse($sheet->getChartByIndex(null));
         $sheet->fromArray(
             [
                 ['Some Title'],
@@ -80,6 +81,13 @@ class ChartsByNameTest extends TestCase
         $sheet->setSelectedCells('D1');
         self::assertSame($chart, $sheet->getChartByName('namedchart1'));
         self::assertSame($chart, $sheet->getChartByNameOrThrow('namedchart1'));
+        self::assertSame($chart, $sheet->getChartByIndex(0));
+        self::assertSame($chart, $sheet->getChartByIndex('0'));
+        self::assertSame($chart, $sheet->getChartByIndex(null));
+        self::assertFalse($sheet->getChartByIndex(' 0'));
+        self::assertFalse($sheet->getChartByIndex(1));
+        self::assertFalse($sheet->getChartByIndex('2'));
+        self::assertFalse($sheet->getChartByIndex('x'));
         self::assertFalse($sheet->getChartByName('namedchart2'));
 
         try {

tests/PhpSpreadsheetTests/Worksheet/CloneTest.php

@@ -7,16 +7,23 @@ namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
 use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 class CloneTest extends TestCase
 {
-    public function testUnattachedIndex(): void
+    #[DataProvider('providerCopyClone')]
+    public function testUnattachedIndex(string $type): void
     {
         $spreadsheet = new Spreadsheet();
         $sheet1 = $spreadsheet->getActiveSheet();
         $sheet1->getCell('A1')->setValue(10);
-        $sheet2 = clone $sheet1;
+        $sheet1->getCell('A2')->setValue(30);
+        if ($type === 'copy') {
+            $sheet2 = $sheet1->copy();
+        } else {
+            $sheet2 = clone $sheet1;
+        }
         $sheet2->getCell('A1')->setValue(20);
         self::assertSame(0, $spreadsheet->getIndex($sheet1));
         $idx = $spreadsheet->getIndex($sheet2, true);
@@ -27,9 +34,19 @@ class CloneTest extends TestCase
         self::assertSame(1, $idx);
         self::assertSame(10, $spreadsheet->getSheet(0)->getCell('A1')->getValue());
         self::assertSame(20, $spreadsheet->getSheet(1)->getCell('A1')->getValue());
+        self::assertSame(30, $spreadsheet->getSheet(0)->getCell('A2')->getValue());
+        self::assertSame(30, $spreadsheet->getSheet(1)->getCell('A2')->getValue());
         $spreadsheet->disconnectWorksheets();
     }
 
+    public static function providerCopyClone(): array
+    {
+        return [
+            ['copy'],
+            ['clone'],
+        ];
+    }
+
     public function testGetCloneIndex(): void
     {
         $this->expectException(SpreadsheetException::class);

tests/PhpSpreadsheetTests/Worksheet/ShrinkRangeToFitTest.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PHPUnit\Framework\TestCase;
+
+class ShrinkRangeToFitTest extends TestCase
+{
+    public static function testToArray(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->getCell('B2')->setValue(1);
+        $sheet->getCell('E10')->setValue(2);
+        $testArray = [
+            'C3:F12' => 'C3:E10',
+            'C3:D12' => 'C3:D10',
+            'C3:D9' => 'C3:D9',
+            'A1:B2 C3:D4' => 'A1:B2 C3:D4',
+            'A1:B11 C3:D4' => 'A1:B10 C3:D4',
+            'A1:B2 C3:F12' => 'A1:B2 C3:E10',
+            'A1:B11 C3:F12' => 'A1:B10 C3:E10',
+            // In both of the following, the range
+            //    isn't merely shrunk - it has moved.
+            // This doesn't seem right, although I am
+            //    hard-pressed to come up with an alternative,
+            //    and not willing to make a breaking change,
+            //    even to code which probably isn't used much.
+            'A11:B12' => 'A10:B10',
+            'G1:H4' => 'E1:E4',
+        ];
+        foreach ($testArray as $input => $expectedOutput) {
+            $output = $sheet->shrinkRangeToFit($input);
+            self::assertSame($expectedOutput, $output, $input);
+        }
+        $spreadsheet->disconnectWorksheets();
+    }
+}