From 3bd0f6f9854bf858a0061eb52d4610467c3848d9 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Sun, 4 Dec 2016 16:44:40 +0900 Subject: [PATCH] Fix autofilter cloning across PHP versions Avoid indirect access variable ambiguity across PHP 5.6 and PHP 7.0 --- .../Worksheet/AutoFilter/Column.php | 24 ++++++------- .../Worksheet/AutoFilter/ColumnTest.php | 36 +++++++++++-------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/src/PhpSpreadsheet/Worksheet/AutoFilter/Column.php b/src/PhpSpreadsheet/Worksheet/AutoFilter/Column.php index 563cffdc..3e45a3da 100644 --- a/src/PhpSpreadsheet/Worksheet/AutoFilter/Column.php +++ b/src/PhpSpreadsheet/Worksheet/AutoFilter/Column.php @@ -376,21 +376,19 @@ class Column { $vars = get_object_vars($this); foreach ($vars as $key => $value) { - if (is_object($value)) { - if ($key == 'parent') { - // Detach from autofilter parent - $this->$key = null; - } else { - $this->$key = clone $value; - } - } elseif ((is_array($value)) && ($key == 'ruleset')) { - // The columns array of \PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter objects - $this->$key = []; + if ($key === 'parent') { + // Detach from autofilter parent + $this->parent = null; + } elseif ($key === 'ruleset') { + // The columns array of \PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter objects + $this->ruleset = []; foreach ($value as $k => $v) { - $this->$key[$k] = clone $v; - // attach the new cloned Rule to this new cloned Autofilter Cloned object - $this->$key[$k]->setParent($this); + $cloned = clone $v; + $cloned->setParent($this); // attach the new cloned Rule to this new cloned Autofilter Cloned object + $this->ruleset[$k] = $cloned; } + } elseif (is_object($value)) { + $this->$key = clone $value; } else { $this->$key = $value; } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/ColumnTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/ColumnTest.php index 080f1bf8..ed9dfc9b 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/ColumnTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/ColumnTest.php @@ -7,9 +7,7 @@ use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter; class ColumnTest extends \PHPUnit_Framework_TestCase { private $testInitialColumn = 'H'; - private $testAutoFilterColumnObject; - private $mockAutoFilterObject; public function setUp() @@ -111,9 +109,10 @@ class ColumnTest extends \PHPUnit_Framework_TestCase public function testSetAttributes() { - $attributeSet = ['val' => 100, - 'maxVal' => 200, - ]; + $attributeSet = [ + 'val' => 100, + 'maxVal' => 200, + ]; // Setters return the instance to implement the fluent interface $result = $this->testAutoFilterColumnObject->setAttributes($attributeSet); @@ -122,9 +121,10 @@ class ColumnTest extends \PHPUnit_Framework_TestCase public function testGetAttributes() { - $attributeSet = ['val' => 100, - 'maxVal' => 200, - ]; + $attributeSet = [ + 'val' => 100, + 'maxVal' => 200, + ]; $this->testAutoFilterColumnObject->setAttributes($attributeSet); @@ -135,9 +135,10 @@ class ColumnTest extends \PHPUnit_Framework_TestCase public function testSetAttribute() { - $attributeSet = ['val' => 100, - 'maxVal' => 200, - ]; + $attributeSet = [ + 'val' => 100, + 'maxVal' => 200, + ]; foreach ($attributeSet as $attributeName => $attributeValue) { // Setters return the instance to implement the fluent interface @@ -148,9 +149,10 @@ class ColumnTest extends \PHPUnit_Framework_TestCase public function testGetAttribute() { - $attributeSet = ['val' => 100, - 'maxVal' => 200, - ]; + $attributeSet = [ + 'val' => 100, + 'maxVal' => 200, + ]; $this->testAutoFilterColumnObject->setAttributes($attributeSet); @@ -164,7 +166,13 @@ class ColumnTest extends \PHPUnit_Framework_TestCase public function testClone() { + $originalRule = $this->testAutoFilterColumnObject->createRule(); $result = clone $this->testAutoFilterColumnObject; $this->assertInstanceOf(AutoFilter\Column::class, $result); + $this->assertCount(1, $result->getRules()); + $this->assertContainsOnlyInstancesOf(AutoFilter\Column\Rule::class, $result->getRules()); + $clonedRule = $result->getRules()[0]; + $this->assertNotSame($originalRule, $clonedRule); + $this->assertSame($result, $clonedRule->getParent()); } }