Disallow associative arguments in ObjectFactory

There is no strong use case for associative "decoration"
of constructor arguments (the documentation benefits
mentioned in I43aa085 are outweighed by the confusion
caused by not failing loudly when someone passes an
associative argument by accident, e.g. by omitting an
array nesting level), so disallow them but make sure
they fail nicely, not with an invalid offset error.

Change-Id: I09e4af85ded6a1497b0db0265d2ee6707f91f5e3

diff --git a/includes/libs/ObjectFactory.php b/includes/libs/ObjectFactory.php
index bd0da57..2ffc1d3 100644 (file)
--- a/includes/libs/ObjectFactory.php
+++ b/includes/libs/ObjectFactory.php
@@ -128,8 +128,10 @@ class ObjectFactory {
         * @return mixed Constructed instance
         */
        public static function constructClassInstance( $clazz, $args ) {
-               // args are sometimes specified in a 'name' => $value format for readability
-               $args = array_values( $args );
+               // $args should be a non-associative array; show nice error if that's not the case
+               if ( $args && array_keys( $args ) !== range( 0, count( $args ) - 1 ) ) {
+                       throw new InvalidArgumentException( __METHOD__ . ': $args cannot be an associative array' );
+               }
 
                // TODO: when PHP min version supported is >=5.6.0 replace this
                // with `return new $clazz( ... $args );`.

diff --git a/tests/phpunit/includes/libs/ObjectFactoryTest.php b/tests/phpunit/includes/libs/ObjectFactoryTest.php
index 6337210..f338633 100644 (file)
--- a/tests/phpunit/includes/libs/ObjectFactoryTest.php
+++ b/tests/phpunit/includes/libs/ObjectFactoryTest.php
@@ -109,12 +109,14 @@ class ObjectFactoryTest extends PHPUnit_Framework_TestCase {
                );
        }
 
+       /**
+        * @expectedException InvalidArgumentException
+        */
        public function testNamedArgs() {
                $args = array( 'foo' => 1, 'bar' => 2, 'baz' => 3 );
                $obj = ObjectFactory::constructClassInstance(
                        'ObjectFactoryTestFixture', $args
                );
-               $this->assertSame( array( 1, 2, 3 ), $obj->args );
        }
 }