Merge pull request #2 from zorji/FixSetForArrayAccess

Fix set()/remove() for ArrayAccess
zorji · Mar 29, 2018 · 0ba7aa2 · 0ba7aa2
2 parents b1401c8 + b851eb6
commit 0ba7aa2
Show file tree

Hide file tree

Showing 8 changed files with 89 additions and 40 deletions.
diff --git a/.php_cs b/.php_cs
@@ -1,10 +1,5 @@
 <?php
-use Symfony\CS\Config\Config;
-use Symfony\CS\Finder\DefaultFinder;
-use Symfony\CS\Fixer\Contrib\HeaderCommentFixer;
-use Symfony\CS\FixerInterface;
-
-$finder = DefaultFinder::create()->in(['config', 'src', 'tests']);
+$finder = PhpCsFixer\Finder::create()->in(['config', 'src', 'tests']);
 $header = <<< EOF
 This file is part of Underscore.php
 
@@ -14,21 +9,20 @@ For the full copyright and license information, please view the LICENSE
 file that was distributed with this source code.
 EOF;
 
-HeaderCommentFixer::setHeader($header);
+$fixer = new \PhpCsFixer\Fixer\Comment\HeaderCommentFixer;
 
-return Config::create()
-             ->level(FixerInterface::SYMFONY_LEVEL)
-             ->fixers([
-                 'ereg_to_preg',
-                 'header_comment',
-                 'multiline_spaces_before_semicolon',
-                 'ordered_use',
-                 'php4_constructor',
-                 'phpdoc_order',
-                 'short_array_syntax',
-                 'short_echo_tag',
-                 'strict',
-                 'strict_param',
-             ])
-             ->setUsingCache(true)
-             ->finder($finder);
+return PhpCsFixer\Config::create()
+    ->setRules([
+        '@PSR2' => true,
+        // 'strict_param' => true,
+        'array_syntax' => ['syntax' => 'short'],
+        'header_comment' => [
+            'header' => $header
+        ],
+        'ordered_imports' => [],
+        // 'ereg_to_preg' => true,
+        'phpdoc_order' => true,
+        // 'no_php4_constructor' => true,
+    ])
+    ->setUsingCache(true)
+    ->setFinder($finder);
diff --git a/.travis.yml b/.travis.yml
@@ -4,8 +4,6 @@ sudo: false
 # Setup build matrix
 language: php
 php:
-  - 5.4
-  - 5.5
   - 5.6
   - 7.0
   - hhvm

diff --git a/composer.json b/composer.json
@@ -14,12 +14,12 @@
     }
   ],
   "require": {
-    "php": ">=5.4.0",
+    "php": ">=5.6.0",
     "doctrine/inflector": "^1.0",
     "patchwork/utf8": "^1.2"
   },
   "require-dev": {
-    "friendsofphp/php-cs-fixer": "^1.10",
+    "friendsofphp/php-cs-fixer": "^2.11",
     "phpunit/phpunit": "^4.8"
   },
   "autoload": {
@@ -35,5 +35,8 @@
   "scripts": {
     "test": ["php-cs-fixer fix --dry-run", "phpunit --verbose"],
     "lint": "php-cs-fixer fix"
+  },
+  "config": {
+    "sort-packages": true
   }
 }
diff --git a/src/Methods/CollectionMethods.php b/src/Methods/CollectionMethods.php
@@ -114,7 +114,7 @@ public static function setAndGet(&$collection, $key, $default = null)
     public static function remove($collection, $key)
     {
         // Recursive call
-        if (is_array($key)) {
+        if (static::isArrayLike($key)) {
             foreach ($key as $k) {
                 static::internalRemove($collection, $k);
             }
@@ -137,7 +137,7 @@ public static function pluck($collection, $property)
         }, (array) $collection);
 
         // Convert back to object if necessary
-        if (is_object($collection)) {
+        if (static::isNonArrayAccessObject($collection)) {
             $plucked = (object) $plucked;
         }
 
@@ -199,7 +199,7 @@ public static function filterBy($collection, $property, $value, $comparisonOp =
 
             return $ops[$comparisonOp]($item, $property, $value);
         }));
-        if (is_object($collection)) {
+        if (static::isNonArrayAccessObject($collection)) {
             $result = (object) $result;
         }
 
@@ -300,6 +300,16 @@ public static function group($collection, $grouper, $saveKeys = false)
         return $result;
     }
 
+    protected static function isNonArrayAccessObject($collection)
+    {
+        return is_object($collection) && !($collection instanceof \ArrayAccess);
+    }
+
+    protected static function isArrayLike($collection)
+    {
+        return is_array($collection) || $collection instanceof \ArrayAccess;
+    }
+
     ////////////////////////////////////////////////////////////////////
     ////////////////////////////// HELPERS /////////////////////////////
     ////////////////////////////////////////////////////////////////////
@@ -321,10 +331,10 @@ protected static function internalSet(&$collection, $key, $value)
             $key = array_shift($keys);
 
             // If we're dealing with an object
-            if (is_object($collection)) {
+            if (static::isNonArrayAccessObject($collection)) {
                 $collection->$key = static::get($collection, $key, []);
                 $collection = &$collection->$key;
-                // If we're dealing with an array
+            // If we're dealing with an array
             } else {
                 $collection[$key] = static::get($collection, $key, []);
                 $collection = &$collection[$key];
@@ -333,7 +343,7 @@ protected static function internalSet(&$collection, $key, $value)
 
         // Bind final tree on the collection
         $key = array_shift($keys);
-        if (is_array($collection)) {
+        if (static::isArrayLike($collection)) {
             $collection[$key] = $value;
         } else {
             $collection->$key = $value;
@@ -357,16 +367,16 @@ protected static function internalRemove(&$collection, $key)
             }
 
             // If we're dealing with an object
-            if (is_object($collection)) {
+            if (static::isNonArrayAccessObject($collection)) {
                 $collection = &$collection->$key;
-                // If we're dealing with an array
+            // If we're dealing with an array
             } else {
                 $collection = &$collection[$key];
             }
         }
 
         $key = array_shift($keys);
-        if (is_object($collection)) {
+        if (static::isNonArrayAccessObject($collection)) {
             unset($collection->$key);
         } else {
             unset($collection[$key]);

diff --git a/src/Types/Strings.php b/src/Types/Strings.php
@@ -16,7 +16,7 @@
 
 /**
  * Strings repository.
- 
+
  *
  *@mixin StringsMethods
  */

diff --git a/tests/MethodTest.php b/tests/MethodTest.php
@@ -67,8 +67,10 @@ public function testCanFindMethodsInClasses()
 
     public function testCanThrowExceptionAtUnknownMethods()
     {
-        $this->setExpectedException('BadMethodCallException',
-            'The method Underscore\Types\Arrays::fuck does not exist');
+        $this->setExpectedException(
+            'BadMethodCallException',
+            'The method Underscore\Types\Arrays::fuck does not exist'
+        );
 
         $test = Arrays::fuck($this);
     }

diff --git a/tests/Types/ArraysTest.php b/tests/Types/ArraysTest.php
@@ -14,6 +14,10 @@
 use Underscore\Underscore;
 use Underscore\UnderscoreTestCase;
 
+class MyArrayAccess extends \ArrayObject
+{
+}
+
 class ArraysTest extends UnderscoreTestCase
 {
     // Tests --------------------------------------------------------- /
@@ -61,6 +65,14 @@ public function testCanSetValues()
 
         $this->assertEquals('ter', $array['foo']['bar']['bis']);
         $this->assertArrayHasKey('bar', $array);
+
+        $array = Arrays::set($this->arrayObjectMulti, 'a.d', 3);
+        $this->assertEquals(2, Arrays::get($array, 'a.c'));
+        $this->assertEquals(3, Arrays::get($array, 'a.d'));
+
+        $array = Arrays::set($this->arrayObjectMulti2, 'a.d', 3);
+        $this->assertEquals(2, Arrays::get($array, 'a.c'));
+        $this->assertEquals(3, Arrays::get($array, 'a.d'));
     }
 
     public function testCanRemoveValues()
@@ -70,6 +82,14 @@ public function testCanRemoveValues()
         unset($matcher[0]['foo']);
 
         $this->assertEquals($matcher, $array);
+
+        $array = Arrays::remove($this->arrayObjectMulti, 'a.b');
+        $this->assertEquals(2, Arrays::get($array, 'a.c'));
+        $this->assertEquals(null, Arrays::get($array, 'a.b'));
+
+        $array = Arrays::remove($this->arrayObjectMulti2, 'a.b');
+        $this->assertEquals(2, Arrays::get($array, 'a.c'));
+        $this->assertEquals(null, Arrays::get($array, 'a.b'));
     }
 
     public function testCanRemoveMultipleValues()
@@ -103,7 +123,7 @@ public function testCanGetSumOfArray()
         $this->assertEquals(6, $array);
     }
 
-    public function testCanGetcountArray()
+    public function testCanGetCountArray()
     {
         $array = Arrays::size([1, 2, 3]);
 
@@ -637,6 +657,9 @@ public function testFilterBy()
         $this->assertCount(3, $f, 'Count should pick up groups which are explicitly set as null AND those which don\'t have the property at all');
         $this->assertContains('qux', Arrays::pluck($f, 'name'));
         $this->assertContains('flux', Arrays::pluck($f, 'name'));
+
+        $under = Arrays::filterBy($this->arrayObjectNumbers, 'value', 1, 'gt');
+        $this->assertEquals([(object) ['value' => 2], (object) ['value' => 3]], $under);
     }
 
     public function testFindBy()

diff --git a/tests/UnderscoreTestCase.php b/tests/UnderscoreTestCase.php
@@ -23,6 +23,9 @@ abstract class UnderscoreTestCase extends PHPUnit_Framework_TestCase
         ['bar' => 'foo', 'bis' => 'ter'],
     ];
     public $object;
+    public $arrayObjectNumbers;
+    public $arrayObjectMulti;
+    public $arrayObjectMulti2;
 
     /**
      * Restore data just in case.
@@ -35,5 +38,21 @@ public function setUp()
             (object) $this->arrayMulti[1],
             (object) $this->arrayMulti[2],
         ];
+
+        $this->arrayObjectNumbers = new \ArrayObject();
+        $this->arrayObjectNumbers[] = (object) ['value' => 1];
+        $this->arrayObjectNumbers[] = (object) ['value' => 2];
+        $this->arrayObjectNumbers[] = (object) ['value' => 3];
+
+        $this->arrayObjectMulti = new \ArrayObject();
+        $this->arrayObjectMulti['a'] = new \ArrayObject();
+        $this->arrayObjectMulti['a']['b'] = 1;
+        $this->arrayObjectMulti['a']['c'] = 2;
+
+        $this->arrayObjectMulti2 = new \ArrayObject();
+        $this->arrayObjectMulti2['a'] = [
+            'b' => 1,
+            'c' => 2,
+        ];
     }
 }