From 10cf632683d41bbfcb6a930f68d4f10b231b87dc Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Wed, 28 Oct 2015 19:06:37 -0700 Subject: [PATCH] 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 --- includes/libs/ObjectFactory.php | 6 ++++-- tests/phpunit/includes/libs/ObjectFactoryTest.php | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/includes/libs/ObjectFactory.php b/includes/libs/ObjectFactory.php index bd0da57fdc..2ffc1d3be8 100644 --- 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 633721097b..f3386339d0 100644 --- 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 ); } } -- 2.20.1