Return signatures of Autoloader's loaders should be void

codeigniter4 · Jul 30, 2023 · 31b79b1 · 31b79b1
1 parent 30c628a
commit 31b79b1
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 40 deletions.
diff --git a/phpstan-baseline.php b/phpstan-baseline.php
@@ -1,16 +1,6 @@
 <?php declare(strict_types = 1);
 
 $ignoreErrors = [];
-$ignoreErrors[] = [
-	'message' => '#^Parameter \\#1 \\$callback of function spl_autoload_register expects \\(callable\\(string\\)\\: void\\)\\|null, array\\{\\$this\\(CodeIgniter\\\\Autoloader\\\\Autoloader\\), \'loadClass\'\\} given\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/system/Autoloader/Autoloader.php',
-];
-$ignoreErrors[] = [
-	'message' => '#^Parameter \\#1 \\$callback of function spl_autoload_register expects \\(callable\\(string\\)\\: void\\)\\|null, array\\{\\$this\\(CodeIgniter\\\\Autoloader\\\\Autoloader\\), \'loadClassmap\'\\} given\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/system/Autoloader/Autoloader.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Method CodeIgniter\\\\BaseModel\\:\\:chunk\\(\\) has no return type specified\\.$#',
 	'count' => 1,

diff --git a/system/Autoloader/Autoloader.php b/system/Autoloader/Autoloader.php
@@ -240,33 +240,30 @@ public function removeNamespace(string $namespace)
     /**
      * Load a class using available class mapping.
      *
-     * @return false|string
+     * @internal For `spl_autoload_register` use.
      */
-    public function loadClassmap(string $class)
+    public function loadClassmap(string $class): void
     {
         $file = $this->classmap[$class] ?? '';
 
         if (is_string($file) && $file !== '') {
-            return $this->includeFile($file);
+            $this->includeFile($file);
         }
-
-        return false;
     }
 
     /**
      * Loads the class file for a given class name.
      *
-     * @param string $class The fully qualified class name.
+     * @internal For `spl_autoload_register` use.
      *
-     * @return false|string The mapped file on success, or boolean false
-     *                      on failure.
+     * @param string $class The fully qualified class name.
      */
-    public function loadClass(string $class)
+    public function loadClass(string $class): void
     {
         $class = trim($class, '\\');
         $class = str_ireplace('.php', '', $class);
 
-        return $this->loadInNamespace($class);
+        $this->loadInNamespace($class);
     }
 
     /**

diff --git a/tests/system/Autoloader/AutoloaderTest.php b/tests/system/Autoloader/AutoloaderTest.php
@@ -12,12 +12,15 @@
 namespace CodeIgniter\Autoloader;
 
 use App\Controllers\Home;
+use Closure;
 use CodeIgniter\Exceptions\ConfigException;
 use CodeIgniter\Test\CIUnitTestCase;
+use CodeIgniter\Test\ReflectionHelper;
 use Config\Autoload;
 use Config\Modules;
 use Config\Services;
 use InvalidArgumentException;
+use PHPUnit\Framework\MockObject\MockObject;
 use RuntimeException;
 use UnnamespacedClass;
 
@@ -28,7 +31,10 @@
  */
 final class AutoloaderTest extends CIUnitTestCase
 {
+    use ReflectionHelper;
+
     private Autoloader $loader;
+    private Closure $classLoader;
 
     protected function setUp(): void
     {
@@ -50,13 +56,15 @@ protected function setUp(): void
 
         $this->loader = new Autoloader();
         $this->loader->initialize($config, $modules)->register();
+
+        $this->classLoader = $this->getPrivateMethodInvoker($this->loader, 'loadInNamespace');
     }
 
     protected function tearDown(): void
     {
-        $this->loader->unregister();
-
         parent::tearDown();
+
+        $this->loader->unregister();
     }
 
     public function testLoadStoredClass(): void
@@ -96,9 +104,10 @@ public function testInitializeTwice(): void
 
     public function testServiceAutoLoaderFromShareInstances(): void
     {
-        $autoloader = Services::autoloader();
+        $classLoader = $this->getPrivateMethodInvoker(Services::autoloader(), 'loadInNamespace');
+
         // look for Home controller, as that should be in base repo
-        $actual   = $autoloader->loadClass(Home::class);
+        $actual   = $classLoader(Home::class);
         $expected = APPPATH . 'Controllers' . DIRECTORY_SEPARATOR . 'Home.php';
         $this->assertSame($expected, realpath($actual) ?: $actual);
     }
@@ -109,8 +118,10 @@ public function testServiceAutoLoader(): void
         $autoloader->initialize(new Autoload(), new Modules());
         $autoloader->register();
 
+        $classLoader = $this->getPrivateMethodInvoker($autoloader, 'loadInNamespace');
+
         // look for Home controller, as that should be in base repo
-        $actual   = $autoloader->loadClass(Home::class);
+        $actual   = $classLoader(Home::class);
         $expected = APPPATH . 'Controllers' . DIRECTORY_SEPARATOR . 'Home.php';
         $this->assertSame($expected, realpath($actual) ?: $actual);
 
@@ -119,41 +130,43 @@ public function testServiceAutoLoader(): void
 
     public function testExistingFile(): void
     {
-        $actual   = $this->loader->loadClass(Home::class);
+        $actual   = ($this->classLoader)(Home::class);
         $expected = APPPATH . 'Controllers' . DIRECTORY_SEPARATOR . 'Home.php';
         $this->assertSame($expected, $actual);
 
-        $actual   = $this->loader->loadClass('CodeIgniter\Helpers\array_helper');
+        $actual   = ($this->classLoader)('CodeIgniter\Helpers\array_helper');
         $expected = SYSTEMPATH . 'Helpers' . DIRECTORY_SEPARATOR . 'array_helper.php';
         $this->assertSame($expected, $actual);
     }
 
     public function testMatchesWithPrecedingSlash(): void
     {
-        $actual   = $this->loader->loadClass(Home::class);
+        $actual   = ($this->classLoader)(Home::class);
         $expected = APPPATH . 'Controllers' . DIRECTORY_SEPARATOR . 'Home.php';
         $this->assertSame($expected, $actual);
     }
 
     public function testMatchesWithFileExtension(): void
     {
-        $actual   = $this->loader->loadClass('\App\Controllers\Home.php');
-        $expected = APPPATH . 'Controllers' . DIRECTORY_SEPARATOR . 'Home.php';
-        $this->assertSame($expected, $actual);
+        /** @var Autoloader&MockObject $classLoader */
+        $classLoader = $this->getMockBuilder(Autoloader::class)->onlyMethods(['loadInNamespace'])->getMock();
+        $classLoader->expects($this->once())->method('loadInNamespace')->with(Home::class);
+
+        $classLoader->loadClass('\App\Controllers\Home.php');
     }
 
     public function testMissingFile(): void
     {
-        $this->assertFalse($this->loader->loadClass('\App\Missing\Classname'));
+        $this->assertFalse(($this->classLoader)('\App\Missing\Classname'));
     }
 
     public function testAddNamespaceWorks(): void
     {
-        $this->assertFalse($this->loader->loadClass('My\App\Class'));
+        $this->assertFalse(($this->classLoader)('My\App\Class'));
 
         $this->loader->addNamespace('My\App', __DIR__);
 
-        $actual   = $this->loader->loadClass('My\App\AutoloaderTest');
+        $actual   = ($this->classLoader)('My\App\AutoloaderTest');
         $expected = __FILE__;
 
         $this->assertSame($expected, $actual);
@@ -168,11 +181,11 @@ public function testAddNamespaceMultiplePathsWorks(): void
             ],
         ]);
 
-        $actual   = $this->loader->loadClass('My\App\App');
+        $actual   = ($this->classLoader)('My\App\App');
         $expected = APPPATH . 'Config' . DIRECTORY_SEPARATOR . 'App.php';
         $this->assertSame($expected, $actual);
 
-        $actual   = $this->loader->loadClass('My\App\AutoloaderTest');
+        $actual   = ($this->classLoader)('My\App\AutoloaderTest');
         $expected = __FILE__;
         $this->assertSame($expected, $actual);
     }
@@ -183,7 +196,7 @@ public function testAddNamespaceStringToArray(): void
 
         $this->assertSame(
             __FILE__,
-            $this->loader->loadClass('App\Controllers\AutoloaderTest')
+            ($this->classLoader)('App\Controllers\AutoloaderTest')
         );
     }
 
@@ -201,15 +214,15 @@ public function testGetNamespaceGivesArray(): void
     public function testRemoveNamespace(): void
     {
         $this->loader->addNamespace('My\App', __DIR__);
-        $this->assertSame(__FILE__, $this->loader->loadClass('My\App\AutoloaderTest'));
+        $this->assertSame(__FILE__, ($this->classLoader)('My\App\AutoloaderTest'));
 
         $this->loader->removeNamespace('My\App');
-        $this->assertFalse((bool) $this->loader->loadClass('My\App\AutoloaderTest'));
+        $this->assertFalse(($this->classLoader)('My\App\AutoloaderTest'));
     }
 
     public function testloadClassNonNamespaced(): void
     {
-        $this->assertFalse($this->loader->loadClass('Modules'));
+        $this->assertFalse(($this->classLoader)('Modules'));
     }
 
     public function testSanitizationContailsSpecialChars(): void

diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst
@@ -109,6 +109,12 @@ Added Parameters
 - **Routing:** The third parameter ``Routing $routing`` has been added to
   ``RouteCollection::__construct()``.
 
+Return Type Changes
+-------------------
+
+- **Autoloader:** The return signatures of the `loadClass` and `loadClassmap` methods are made `void`
+  to be compatible as callbacks in `spl_autoload_register` and `spl_autoload_unregister` functions.
+
 Enhancements
 ************
 
@@ -225,6 +231,8 @@ Changes
   ``Config\App::$forceGlobalSecureRequests = true`` sets the HTTP status code 307,
   which allows the HTTP request method to be preserved after the redirect.
   In previous versions, it was 302.
+- The methods `Autoloader::loadClass()` and `Autoloader::loadClassmap()` are now both
+  marked `@internal`.
 
 Deprecations
 ************