Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimizations for Autoloader #4604

Merged
merged 3 commits into from
Apr 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ parameters:
- '#Method CodeIgniter\\Validation\\ValidationInterface::run\(\) invoked with 3 parameters, 0-2 required#'
- '#Negated boolean expression is always (true|false)#'
- '#Parameter \#1 \$db of class CodeIgniter\\Database\\SQLite3\\Table constructor expects CodeIgniter\\Database\\SQLite3\\Connection, CodeIgniter\\Database\\BaseConnection given#'
- '#Return type \(bool\) of method CodeIgniter\\Log\\Logger::(emergency|alert|critical|error|warning|notice|info|debug|log)\(\) should be compatible with return type \(null\) of method Psr\\Log\\LoggerInterface::(emergency|alert|critical|error|warning|notice|info|debug|log)\(\)#'
- '#Return type \(bool\) of method CodeIgniter\\HTTP\\Files\\UploadedFile::move\(\) should be compatible with return type \(CodeIgniter\\Files\\File\) of method CodeIgniter\\Files\\File::move\(\)#'
- '#Return type \(bool\) of method CodeIgniter\\Test\\TestLogger::log\(\) should be compatible with return type \(null\) of method Psr\\Log\\LoggerInterface::log\(\)#'
parallel:
processTimeout: 300.0
scanDirectories:
Expand Down
84 changes: 39 additions & 45 deletions system/Autoloader/Autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\Autoloader;

use Composer\Autoload\ClassLoader;
use Config\Autoload;
use Config\Modules;
use InvalidArgumentException;
Expand All @@ -21,17 +22,17 @@
* An autoloader that uses both PSR4 autoloading, and traditional classmaps.
*
* Given a foo-bar package of classes in the file system at the following paths:
*
*```
* /path/to/packages/foo-bar/
* /src
* Baz.php # Foo\Bar\Baz
* Qux/
* Quux.php # Foo\Bar\Qux\Quux
*
*```
* you can add the path to the configuration array that is passed in the constructor.
* The Config array consists of 2 primary keys, both of which are associative arrays:
* 'psr4', and 'classmap'.
*
*```
* $Config = [
* 'psr4' => [
* 'Foo\Bar' => '/path/to/packages/foo-bar'
Expand All @@ -40,35 +41,34 @@
* 'MyClass' => '/path/to/class/file.php'
* ]
* ];
*
*```
* Example:
*
*```
* <?php
* // our configuration array
* $Config = [ ... ];
* $loader = new \CodeIgniter\Autoloader\Autoloader($Config);
*
* // register the autoloader
* $loader->register();
*```
*/
class Autoloader
{
/**
* Stores namespaces as key, and path as values.
*
* @var array
* @var array<string, array<string>>
*/
protected $prefixes = [];

/**
* Stores class name as key, and path as values.
*
* @var array
* @var array<string, string>
*/
protected $classmap = [];

//--------------------------------------------------------------------

/**
* Reads in the configuration array (described above) and stores
* the valid parts that we'll need.
Expand Down Expand Up @@ -106,8 +106,6 @@ public function initialize(Autoload $config, Modules $modules)
return $this;
}

//--------------------------------------------------------------------

/**
* Register the loader with the SPL autoloader stack.
*/
Expand All @@ -117,22 +115,9 @@ public function register()
spl_autoload_register([$this, 'loadClass'], true, true); // @phpstan-ignore-line

// Now prepend another loader for the files in our class map.

// @phpstan-ignore-next-line
spl_autoload_register(function ($class) {
if (empty($this->classmap[$class]))
{
return false;
}

include_once $this->classmap[$class];
}, true, // Throw exception
true // Prepend
);
spl_autoload_register([$this, 'loadClassmap'], true, true); // @phpstan-ignore-line
}

//--------------------------------------------------------------------

/**
* Registers namespaces with the autoloader.
*
Expand Down Expand Up @@ -170,8 +155,6 @@ public function addNamespace($namespace, string $path = null)
return $this;
}

//--------------------------------------------------------------------

/**
* Get namespaces with prefixes as keys and paths as values.
*
Expand All @@ -191,8 +174,6 @@ public function getNamespace(string $prefix = null)
return $this->prefixes[trim($prefix, '\\')] ?? [];
}

//--------------------------------------------------------------------

/**
* Removes a single namespace from the psr4 settings.
*
Expand All @@ -210,7 +191,24 @@ public function removeNamespace(string $namespace)
return $this;
}

//--------------------------------------------------------------------
/**
* Load a class using available class mapping.
*
* @param string $class
*
* @return string|false
*/
public function loadClassmap(string $class)
{
$file = $this->classmap[$class] ?? '';

if (is_string($file) && $file !== '')
Comment on lines +203 to +205
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a whole lot of effort not to use empty().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using phpstan/phpstan-strict-rules in my workflows a lot, and one of the strict rules I'm following is the disallowance of empty() which implies loose comparison.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. I'm not against this verbose, strict approach but I personally prefer empty() when it saves multiple conditions.

{
return $this->includeFile($file);
}

return false;
}

/**
* Loads the class file for a given class name.
Expand All @@ -228,8 +226,6 @@ public function loadClass(string $class)
return $this->loadInNamespace($class);
}

//--------------------------------------------------------------------

/**
* Loads the class file for a given class name.
*
Expand Down Expand Up @@ -276,8 +272,6 @@ protected function loadInNamespace(string $class)
return false;
}

//--------------------------------------------------------------------

/**
* A central way to include a file. Split out primarily for testing purposes.
*
Expand All @@ -299,8 +293,6 @@ protected function includeFile(string $file)
return false;
}

//--------------------------------------------------------------------

/**
* Sanitizes a filename, replacing spaces with dashes.
*
Expand All @@ -312,13 +304,12 @@ protected function includeFile(string $file)
*
* @param string $filename
*
* @return string The sanitized filename
* @return string The sanitized filename
*/
public function sanitizeFilename(string $filename): string
{
// Only allow characters deemed safe for POSIX portable filenames.
// Plus the forward slash for directory separators since this might
// be a path.
// Plus the forward slash for directory separators since this might be a path.
// http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_278
// Modified to allow backslash and colons for on Windows machines.
$filename = preg_replace('/[^0-9\p{L}\s\/\-\_\.\:\\\\]/u', '', $filename);
Expand All @@ -329,21 +320,23 @@ public function sanitizeFilename(string $filename): string
return $filename;
}

//--------------------------------------------------------------------

/**
* Locates all PSR4 compatible namespaces from Composer.
* Locates autoload information from Composer, if available.
*
* @return void
*/
protected function discoverComposerNamespaces()
{
if (! is_file(COMPOSER_PATH))
{
return false;
return;
}

/** @var ClassLoader $composer */
$composer = include COMPOSER_PATH;
$paths = $composer->getPrefixesPsr4();
$classes = $composer->getClassMap();

$paths = $composer->getPrefixesPsr4();
unset($composer);

// Get rid of CodeIgniter so we don't have duplicates
Expand All @@ -352,13 +345,14 @@ protected function discoverComposerNamespaces()
unset($paths['CodeIgniter\\']);
}

// Composer stores namespaces with trailing slash. We don't.
$newPaths = [];
foreach ($paths as $key => $value)
{
// Composer stores namespaces with trailing slash. We don't.
$newPaths[rtrim($key, '\\ ')] = $value;
}

$this->prefixes = array_merge($this->prefixes, $newPaths);
$this->classmap = array_merge($this->classmap, $classes);
}
}