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

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 95579a9..604a140 100644 (file)
--- 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 9040f39..171f2a6 100644 (file)
--- 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'
                );
        }