resourceloader: Require $context parameter for FileModule::readStyleFiles()

Deprecated since MW 1.27.

Also update ResourcesTest to use TestingAccessWrapper instead of long-form
object reflection, and also apply it to its call for this method given
its meant to be private.

Change-Id: I9cc1af93730f632e4f8bf3a16d514a51ee73cb03

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index cacfa51..e545af8 100644 (file)
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -247,6 +247,8 @@ because of Phabricator reports.
 * The mw.user.stickyRandomId() method, deprecated in 1.32, was removed.
   Use mw.user.getPageviewToken() instead.
 * Removed deprecated class property WikiRevision::$importer.
+* ResourceLoaderFileModule::readStyleFiles() now requires its $context
+  parameter.
 
 === Deprecations in 1.33 ===
 * The configuration option $wgUseESI has been deprecated, and is expected

diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php
index d6573af..67de192 100644 (file)
--- a/includes/resourceloader/ResourceLoaderContext.php
+++ b/includes/resourceloader/ResourceLoaderContext.php
@@ -136,7 +136,6 @@ class ResourceLoaderContext implements MessageLocalizer {
         *
         * Use cases:
         * - Creating html5shiv script tag in OutputPage.
-        * - FileModule::readStyleFiles (deprecated, to be removed).
         * - Unit tests (deprecated, create empty instance directly or use RLTestCase).
         *
         * @return ResourceLoaderContext

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 0e53e5e..4444b13 100644 (file)
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -878,25 +878,16 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
        /**
         * Get the contents of a list of CSS files.
         *
-        * This is considered a private method. Exposed for internal use by WebInstallerOutput.
-        *
-        * @private
+        * @internal This is considered a private method. Exposed for internal use by WebInstallerOutput.
         * @param array $styles Map of media type to file paths to read, remap, and concatenate
         * @param bool $flip
-        * @param ResourceLoaderContext|null $context
+        * @param ResourceLoaderContext $context
         * @return array List of concatenated and remapped CSS data from $styles,
         *     keyed by media type
         * @throws MWException
-        * @since 1.27 Calling this method without a ResourceLoaderContext instance
-        *   is deprecated.
         */
-       public function readStyleFiles( array $styles, $flip, $context = null ) {
-               if ( $context === null ) {
-                       wfDeprecated( __METHOD__ . ' without a ResourceLoader context', '1.27' );
-                       $context = ResourceLoaderContext::newDummyContext();
-               }
-
-               if ( empty( $styles ) ) {
+       public function readStyleFiles( array $styles, $flip, $context ) {
+               if ( !$styles ) {
                        return [];
                }
                foreach ( $styles as $media => $files ) {

diff --git a/tests/phpunit/structure/ResourcesTest.php b/tests/phpunit/structure/ResourcesTest.php
index e08ffd7..776dee1 100644 (file)
--- a/tests/phpunit/structure/ResourcesTest.php
+++ b/tests/phpunit/structure/ResourcesTest.php
@@ -1,6 +1,7 @@
 <?php
 
 use MediaWiki\MediaWikiServices;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * Sanity checks for making sure registered resources are sane.
@@ -245,9 +246,6 @@ class ResourcesTest extends MediaWikiTestCase {
        /**
         * Get all resource files from modules that are an instance of
         * ResourceLoaderFileModule (or one of its subclasses).
-        *
-        * Since the raw data is stored in protected properties, we have to
-        * overrride this through ReflectionObject methods.
         */
        public static function provideResourceFiles() {
                $data = self::getAllModules();
@@ -275,14 +273,12 @@ class ResourcesTest extends MediaWikiTestCase {
                                continue;
                        }
 
-                       $reflectedModule = new ReflectionObject( $module );
+                       $moduleProxy = TestingAccessWrapper::newFromObject( $module );
 
                        $files = [];
 
                        foreach ( $filePathProps['lists'] as $propName ) {
-                               $property = $reflectedModule->getProperty( $propName );
-                               $property->setAccessible( true );
-                               $list = $property->getValue( $module );
+                               $list = $moduleProxy->$propName;
                                foreach ( $list as $key => $value ) {
                                        // 'scripts' are numeral arrays.
                                        // 'styles' can be numeral or associative.
@@ -297,9 +293,7 @@ class ResourcesTest extends MediaWikiTestCase {
                        }
 
                        foreach ( $filePathProps['nested-lists'] as $propName ) {
-                               $property = $reflectedModule->getProperty( $propName );
-                               $property->setAccessible( true );
-                               $lists = $property->getValue( $module );
+                               $lists = $moduleProxy->$propName;
                                foreach ( $lists as $list ) {
                                        foreach ( $list as $key => $value ) {
                                                // We need the same filter as for 'lists',
@@ -313,29 +307,23 @@ class ResourcesTest extends MediaWikiTestCase {
                                }
                        }
 
-                       // Get method for resolving the paths to full paths
-                       $method = $reflectedModule->getMethod( 'getLocalPath' );
-                       $method->setAccessible( true );
-
                        // Populate cases
                        foreach ( $files as $file ) {
                                $cases[] = [
-                                       $method->invoke( $module, $file ),
+                                       $moduleProxy->getLocalPath( $file ),
                                        $moduleName,
                                        ( $file instanceof ResourceLoaderFilePath ? $file->getPath() : $file ),
                                ];
                        }
 
                        // To populate missingLocalFileRefs. Not sure how sane this is inside this test...
-                       $module->readStyleFiles(
+                       $moduleProxy->readStyleFiles(
                                $module->getStyleFiles( $data['context'] ),
                                $module->getFlip( $data['context'] ),
                                $data['context']
                        );
 
-                       $property = $reflectedModule->getProperty( 'missingLocalFileRefs' );
-                       $property->setAccessible( true );
-                       $missingLocalFileRefs = $property->getValue( $module );
+                       $missingLocalFileRefs = $moduleProxy->missingLocalFileRefs;
 
                        foreach ( $missingLocalFileRefs as $file ) {
                                $cases[] = [