make optimized autoloader respect PSR standards #8397
Conversation
|
Thanks, looking good |
|
@and800 I just ran into this in a production build. I build with Chef and use the composer cookbook to install. It's installing automatically with the latest 1.10-dev for some reason, but that's not why I'm here. I'm getting the "Warning: class CLASSNAME located in PATH doesn't comply with psr-4 autoloading standard. Skipping." This burned me pretty bad in production because my build passed, but none of my classes were loaded. I highly suggest you switch this to either 1) display warning but do not skip the import or 2) throw an error and bail. Ignoring an import and not bailing out can lead to bad things, per my example. Hopefully this is helpful before you push to a stable version. |
That is similar to what I wrote here. I wonder though, do you use optimized autoloader in dev as well? Otherwise you would have already run into it. The only way this could escape your attention in dev AFAICT is that you used the old version of composer combined with an optimized autoloader. FWIW, this is a problem in Laravel Boilerplate as well. Sigh boilerplate burned me. |
|
I run optimized autoloader in all environments. The change happened 20 minutes before I was deploying so hadn't even ran it between that time locally. My local version wasn't updated that far ahead either, self-update only updates to stable versions. But yeah what you're saying is correct. I would have caught it in staging since I do canary deployments, but I deployed staging hours before the 1.10-dev rollout. Fun times! |
|
ok it was rather stupid stuff from me, maybe it shouldn't have been so hardcore.
Option (1) seems like the best solution for now. Option (2) has an issue that it would throw the warnings even if a class was never meant to be autoloaded. So we may stick to throwing the warnings for several releases, and decide its destiny later. Formally it isn't a breaking api change, no need to wait for the next major release, but let's give some time to users. |
|
@and800 Honestly, I disagree. You fixed a bug. Some people were relying on said bug. Most people will notice once they update their composer to the fixed version in DEV. Even if they don't, the chance of them being bitten by it still isn't large, as they would need to run an old version of composer up until production. @crearc Basically did the opposite of what's desirable: using an older version of composer in dev and a brand spanking new version in prod (unstable master version, no less!). Again, you fixed a bug. Let's not be backwards compatible with bugs, that should be left to Microsoft. |
|
f6b8643 changes it to only warn for now, given this can break apps it's probably best to give a little bit of a warning period. |
…-installed * github-composer/2.0: (63 commits) Fix PSR warnings for optimized autoloader, refs composer#8397, refs composer#8403 Prepare 1.9.1 changelog Output a hint that maybe you are not in the right directory, fixes composer#8404 Fix PSR warnings for optimized autoloader, refs composer#8397, refs composer#8403 Fix tests for PSR-fix in optimized autoloader, refs composer#8397 Fix tests for PSR-fix in optimized autoloader, refs composer#8397 Change PSR-fix for optimized autoloader to only warn for now, refs composer#8397 Fix output of dump-autoload command to avoid interfering with warnings, refs composer#8397 Remove credentials from git remotes in cache and vendor dirs Avoid overwriting credentials with existing ones from git repos, refs composer#8293 Fix github auth to try https with pwd also, fixes composer#8356 Fix gitlab support for basic-auth fallback from ssh URLs Avoid clearing the error output during removeDirectory execution, losing git error output, fixes composer#8351 Move test file parsing into try/catch block to avoid phpunit swallowing errors make optimized autoloader respect PSR standards Validate composer show with --tree and --path options set (composer#8390) Don't show root warning for docker containers Added phpdoc for ComposerAutoloaderInit$SHA1::getLoader() (composer#8393) Validate schema name, type and version Fix require command to allow working on network mounts, fixes composer#8231 ...
* Debug: display used authentication for http calls * Command::execute() should always return an integer. * Check that if the getUrlMatches method returns an empty value which means the path is incorrect * Test to check there is a RuntimeException thrown when a path repository doesn't exist * Add details of the path to aid debugging * Remove extra line in method following CS-Fixer * Remove unused variable * added package homepage information to the command 'show' * changed homepage information position * Consider replaces when checking package dependents * Remove invalid array keys * Fix misc phpdoc and strpos arg order nits https://www.php.net/strpos has the signature `strpos ( string $haystack , mixed $needle [, int $offset = 0 ] ) : int` (The needle is usually the constant) `strpos('/', $suggestion)` would only be `false` for `''` and `'/'` So the existing check would just not suggest **anything** that was already installed (from pecl, built-in, or composer). The intent seems to be to not suggest non-vendored php packages that were already installed. (b20cc22) * Added comment why source link check is necessary * Add option to disable the lock file When the `lock` option is set to false, composer will not write a `composer.lock` file to disk. This signals that the package is meant to be developed with unlocked and always updated dependencies. At the moment, both `install` and `update` are allowed to install the dependencies for such a package. If composer#6822 is implemented, only `update` should be used for packages without a lockfile. composer#8354 * Add tests for installer with lock: false composer#8354 * Added clear cache for windows, fix tests * Don't necessarily mention Google There's other search engines as well. * Add messages to junction tests to see failures * HgDriver: don't run command in non-existing directory * Add Windows proc-open errors to troubleshooting.md As per these issues: https://github.com/composer/composer/issues/7186 composer#8152 * Fix composer outdated command on PHP 7.4; fixes composer#8346 * Avoid calling findPackage for non-platform packages * Update safeguard code, fixes composer#8383 * 5.3 support :/ * Fix require command to allow working on network mounts, fixes composer#8231 * Validate schema name, type and version * Added phpdoc for ComposerAutoloaderInit$SHA1::getLoader() (composer#8393) * Don't show root warning for docker containers Signed-off-by: Viacheslav Sychov <viacheslav.sychov@gmail.com> * Validate composer show with --tree and --path options set (composer#8390) * make optimized autoloader respect PSR standards * Move test file parsing into try/catch block to avoid phpunit swallowing errors * Avoid clearing the error output during removeDirectory execution, losing git error output, fixes composer#8351 * Fix gitlab support for basic-auth fallback from ssh URLs * Fix github auth to try https with pwd also, fixes composer#8356 * Avoid overwriting credentials with existing ones from git repos, refs composer#8293 * Remove credentials from git remotes in cache and vendor dirs This only removes the credentials if they are managed by composer auth.json or equivalent, if the credentials were present in the package URL to begin with they might remain Refs composer#8293 Fixes composer#3644 Closes composer#3608 * Fix output of dump-autoload command to avoid interfering with warnings, refs composer#8397 * Change PSR-fix for optimized autoloader to only warn for now, refs composer#8397 * Fix tests for PSR-fix in optimized autoloader, refs composer#8397 * Fix tests for PSR-fix in optimized autoloader, refs composer#8397 * Fix PSR warnings for optimized autoloader, refs composer#8397, refs composer#8403 * Output a hint that maybe you are not in the right directory, fixes composer#8404 * Prepare 1.9.1 changelog * Fix PSR warnings for optimized autoloader, refs composer#8397, refs composer#8403 * Update dependencies * Fix: Xdebug vs xdebug * Fix: Add environment variables related to Xdebug to documentation
fix #7352
This is a revived pull request #7421. I accidentally deleted my fork repo, so that PR became readonly. This one is ready for review now.
Quick recap: optimized dump-autoload adds every found class to the classmap, even if it is not under psr0/psr4 rules. My proposition is to filter the class list extracted from every file and fire a warning for any class with misaligned namespace and location. If there is a proper class in the file, improper classes from that file are skipped silently. If a class has even different namespace, it is also skipped with no warnings(that's the current behavior, I kept it).
I've also written a simple test for it.