From: Gergő Tisza <tgr.huwiki@gmail.com>
Date: Thu, 29 Oct 2015 02:06:37 +0000 (-0700)
Subject: Disallow associative arguments in ObjectFactory
X-Git-Tag: 1.31.0-rc.0~8898
X-Git-Url: http://git.cyclocoop.org/%24action?a=commitdiff_plain;h=10cf632683d41bbfcb6a930f68d4f10b231b87dc;p=lhc%2Fweb%2Fwiklou.git

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 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 );
 	}
 }