From 0a5757cda86b29462bddfbbc1c143811097bfef1 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Sat, 15 Sep 2018 22:31:18 +0100
Subject: [PATCH] resourceloader: Make
 ResourceLoader::makeLoaderRegisterScript() internal

* There is only a single non-test caller to this method in the entire
  codebase, and no callers elsewhere (Wikimedia Git, Codesearch).

  It's only used with an array, so remove the other unused code paths,
  and mark it internal (to my knowledge, nothing ever used it outside
  RL in the past, either).

* Add test coverage for the module indexing logic.

Change-Id: I9e0f95416d5b2fdd87323288231ee6d8c85d88e7
---
 includes/resourceloader/ResourceLoader.php    | 83 ++++++++-----------
 .../resourceloader/ResourceLoaderTest.php     | 42 ++++++++--
 2 files changed, 71 insertions(+), 54 deletions(-)

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 95579a99fb..604a140de5 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1382,69 +1382,56 @@ MESSAGE;
 
 	/**
 	 * Returns JS code which calls mw.loader.register with the given
-	 * parameters. Has three calling conventions:
+	 * parameter.
 	 *
-	 *   - ResourceLoader::makeLoaderRegisterScript( $name, $version,
-	 *        $dependencies, $group, $source, $skip
-	 *     ):
-	 *        Register a single module.
+	 * @par Example
+	 * @code
 	 *
-	 *   - ResourceLoader::makeLoaderRegisterScript( [ $name1, $name2 ] ):
-	 *        Register modules with the given names.
-	 *
-	 *   - ResourceLoader::makeLoaderRegisterScript( [
+	 *     ResourceLoader::makeLoaderRegisterScript( [
 	 *        [ $name1, $version1, $dependencies1, $group1, $source1, $skip1 ],
 	 *        [ $name2, $version2, $dependencies1, $group2, $source2, $skip2 ],
 	 *        ...
 	 *     ] ):
-	 *        Registers modules with the given names and parameters.
+	 * @endcode
 	 *
-	 * @param string $name Module name
-	 * @param string|null $version Module version hash
-	 * @param array|null $dependencies List of module names on which this module depends
-	 * @param string|null $group Group which the module is in
-	 * @param string|null $source Source of the module, or 'local' if not foreign
-	 * @param string|null $skip Script body of the skip function
+	 * @internal
+	 * @since 1.32
+	 * @param array $modules Array of module registration arrays, each containing
+	 *  - string: module name
+	 *  - string: module version
+	 *  - array|null: List of dependencies (optional)
+	 *  - string|null: Module group (optional)
+	 *  - string|null: Name of foreign module source, or 'local' (optional)
+	 *  - string|null: Script body of a skip function (optional)
 	 * @return string JavaScript code
 	 */
-	public static function makeLoaderRegisterScript( $name, $version = null,
-		$dependencies = null, $group = null, $source = null, $skip = null
-	) {
-		if ( is_array( $name ) ) {
+	public static function makeLoaderRegisterScript( array $modules ) {
+		// Optimisation: Transform dependency names into indexes when possible
+		// to produce smaller output. They are expanded by mw.loader.register on
+		// the other end using resolveIndexedDependencies().
+		$index = [];
+		foreach ( $modules as $i => &$module ) {
 			// Build module name index
-			$index = [];
-			foreach ( $name as $i => &$module ) {
-				$index[$module[0]] = $i;
-			}
-
-			// Transform dependency names into indexes when possible, they will be resolved by
-			// mw.loader.register on the other end
-			foreach ( $name as &$module ) {
-				if ( isset( $module[2] ) ) {
-					foreach ( $module[2] as &$dependency ) {
-						if ( isset( $index[$dependency] ) ) {
-							$dependency = $index[$dependency];
-						}
+			$index[$module[0]] = $i;
+		}
+		foreach ( $modules as &$module ) {
+			if ( isset( $module[2] ) ) {
+				foreach ( $module[2] as &$dependency ) {
+					if ( isset( $index[$dependency] ) ) {
+						// Replace module name in dependency list with index
+						$dependency = $index[$dependency];
 					}
 				}
 			}
+		}
 
-			array_walk( $name, [ 'self', 'trimArray' ] );
+		array_walk( $modules, [ 'self', 'trimArray' ] );
 
-			return Xml::encodeJsCall(
-				'mw.loader.register',
-				[ $name ],
-				self::inDebugMode()
-			);
-		} else {
-			$registration = [ $name, $version, $dependencies, $group, $source, $skip ];
-			self::trimArray( $registration );
-			return Xml::encodeJsCall(
-				'mw.loader.register',
-				$registration,
-				self::inDebugMode()
-			);
-		}
+		return Xml::encodeJsCall(
+			'mw.loader.register',
+			[ $modules ],
+			self::inDebugMode()
+		);
 	}
 
 	/**
diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index 9040f3974b..171f2a69ff 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -499,12 +499,42 @@ mw.example();
 		);
 
 		$this->assertEquals(
-			'mw.loader.register( "test.name", "1234567" );',
-			ResourceLoader::makeLoaderRegisterScript(
-				'test.name',
-				'1234567'
-			),
-			'Variadic parameters'
+			'mw.loader.register( [
+    [
+        "test.foo",
+        "100"
+    ],
+    [
+        "test.bar",
+        "200",
+        [
+            "test.unknown"
+        ]
+    ],
+    [
+        "test.baz",
+        "300",
+        [
+            3,
+            0
+        ]
+    ],
+    [
+        "test.quux",
+        "400",
+        [],
+        null,
+        null,
+        "return true;"
+    ]
+] );',
+			ResourceLoader::makeLoaderRegisterScript( [
+				[ 'test.foo', '100' , [], null, null ],
+				[ 'test.bar', '200', [ 'test.unknown' ], null ],
+				[ 'test.baz', '300', [ 'test.quux', 'test.foo' ], null ],
+				[ 'test.quux', '400', [], null, null, 'return true;' ],
+			] ),
+			'Compact dependency indexes'
 		);
 	}
 
-- 
2.20.1