Page MenuHomePhabricator

Implement a PSR-4 autoloader in MediaWiki core
Closed, ResolvedPublic

Description

As part of the namespaceization project we should implement some kind of PSR-4 autoloader in core. We should survey existing packages, see if any are usable, or whether we should write our own.

Event Timeline

composer already has a PSR-4 autoloader in it, so if we could use that it would save us from needing yet-another-autoloader. Here's a quick POC that works:

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 8dc7d4094a..a82dbe0a1d 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -88,6 +88,12 @@ class AutoLoader {
        static function resetAutoloadLocalClassesLower() {
                self::$autoloadLocalClassesLower = null;
        }
+
+       public static function getPSR4Mapping() {
+               return [
+                       'MediaWiki\\Linker\\' => __DIR__ . '/linker'
+               ];
+       }
 }
 
 spl_autoload_register( [ 'AutoLoader', 'autoload' ] );
diff --git a/includes/WebStart.php b/includes/WebStart.php
index e281b6f267..694bae16b1 100644
--- a/includes/WebStart.php
+++ b/includes/WebStart.php
@@ -80,11 +80,14 @@ require_once "$IP/includes/DefaultSettings.php";
 # Load global functions
 require_once "$IP/includes/GlobalFunctions.php";
 
-# Load composer's autoloader if present
-if ( is_readable( "$IP/vendor/autoload.php" ) ) {
-       require_once "$IP/vendor/autoload.php";
+# Load composer's autoloader
+/** @var \Composer\Autoload\ClassLoader $composerAutoloader */
+$composerAutoloader = require "$IP/vendor/autoload.php";
+foreach ( Autoloader::getPSR4Mapping() as $ns => $path ) {
+       $composerAutoloader->addPsr4( $ns, [ $path ] );
 }

The conditional of is_readable( vendor/autoload.php ) is actually outdated since wfEntryPointCheck() in index.php checks for the existence of that file.

I surveyed a few libraries by searching "psr-4" and "autoload" in packagist, and didn't find anything unique or impressive - I think the most battle tested and popular autoloader is going to be composer's. I'll put together a more realistic patch than the POC above.

Change 373626 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] [WIP] Enable using PSR-4 autoloader for MediaWiki core and extensions

https://gerrit.wikimedia.org/r/373626

I think there are a few more steps that need to be taken before this can be merged:

I think there are a few more steps that need to be taken before this can be merged:

Although you can't really enable sniffs until the sniffs pass, and making those hypothetical sniffs pass is what T166010 is going to be doing.

Once we actually use a PSR-4 autoloader, then the "Some kind of check to make sure that all classes are autoloadable" is probably going to be testing those two bullets too (or vice versa, if the second sniff also checks the path against the namespace).

I think there are a few more steps that need to be taken before this can be merged:

Although you can't really enable sniffs until the sniffs pass, and making those hypothetical sniffs pass is what T166010 is going to be doing.

We can definitely enforce one class per file ahead of time, it'll just require a bit of manual work (and save some time from trying to automate that). For the second sniff I was just going to validate the base filename against the class name, not the entire directory structure. We can implement the directory structure part afterwards.

Once we actually use a PSR-4 autoloader, then the "Some kind of check to make sure that all classes are autoloadable" is probably going to be testing those two bullets too (or vice versa, if the second sniff also checks the path against the namespace).

Makes sense. :)

We can definitely enforce one class per file ahead of time, it'll just require a bit of manual work (and save some time from trying to automate that).

It means files will be split and then the new files will be renamed soon after, making following the history in git that much more involved.

For the second sniff I was just going to validate the base filename against the class name, not the entire directory structure. We can implement the directory structure part afterwards.

Again, that means two renamings of the file in short order.

--follow will probably handle the renames ok for a straight log if you habitually use it, but I find that repeated blaming needs extra adjustment to the command line whenever the filename changes.

We can definitely enforce one class per file ahead of time, it'll just require a bit of manual work (and save some time from trying to automate that).

It means files will be split and then the new files will be renamed soon after, making following the history in git that much more involved.

For the second sniff I was just going to validate the base filename against the class name, not the entire directory structure. We can implement the directory structure part afterwards.

Again, that means two renamings of the file in short order.

--follow will probably handle the renames ok for a straight log if you habitually use it, but I find that repeated blaming needs extra adjustment to the command line whenever the filename changes.

If we split/renamed the files to their final destination ahead of time, would that resolve your concerns?

Yes, splitting to the final destination would be minimal churn in git, versus splitting to one filename and then renaming to a different name shortly after.

Change 373626 merged by jenkins-bot:
[mediawiki/core@master] Enable using PSR-4 autoloader for MediaWiki core and extensions

https://gerrit.wikimedia.org/r/373626