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

fix: [FileLocator] Cannot declare class XXX, because the name is already in use #8745

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Apr 10, 2024

Description
See codeigniter4/shield#1091

If you define a namespace in another namespace,

    public $psr4 = [
        APP_NAMESPACE => APPPATH,
        'Shield'      => APPPATH . 'ThirdParty/Shield',
    ];

The file path app/ThirdParty/Shield/Config/Auth.php could be

  1. App\ThirdParty\Shield\Config\Auth
  2. Shield\Config\Auth

findQualifiedNameFromPath() tries to find App\ThirdParty\Shield\Config\Auth more than once,
then the following error occurs:

Cannot declare class Shield\Config\Auth, because the name is already in use

The error does not occur if app/Config/Auth.php exists.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Apr 10, 2024
Copy link
Contributor

@datamweb datamweb left a comment

Choose a reason for hiding this comment

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

Nice!

@kenjis
Copy link
Member Author

kenjis commented Apr 12, 2024

See also https://github.com/codeigniter4/translations/actions/runs/8652234845/job/23735006874?pr=440

Run if [ "pull_request" = "pull_request" ]; then
PHP Fatal error:  Cannot declare class CodeIgniter\Config\Services, because the name is already in use in /home/runner/work/translations/translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php on line 111

Fatal error: Cannot declare class CodeIgniter\Config\Services, because the name is already in use in /home/runner/work/translations/translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php on line 111
Error: Process completed with exit code 255.

@michalsn
Copy link
Member

findQualifiedNameFromPath() tries to find App\ThirdParty\Shield\Config\Auth more than once,
then the following error occurs:

Cannot declare class Shield\Config\Auth, because the name is already in use

To be honest, I don't entirely understand this.

If the problem is that the file is trying to be loaded more than once, why are we collecting class names that don't exist in the $invalidClassnames variable?

Where is the source of the problem? In which place the error is generated?

@kenjis
Copy link
Member Author

kenjis commented Apr 12, 2024

If the problem is that the file is trying to be loaded more than once, why are we collecting class names that don't exist in the $invalidClassnames variable?

To prevent from loading twice.

Where is the source of the problem? In which place the error is generated?

Composer.

You can easily reproduce in translations repository.
Because it has Translations namespace.
See https://github.com/codeigniter4/translations/blob/2903b570df3c3526055e4d23c19291a90675607b/composer.json#L29-L33

codeigniter4-translations (develop $=)$ bin/test ja
PHP Fatal error:  Cannot declare class CodeIgniter\Config\Services, because the name is already in use in /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php on line 111
PHP Stack trace:
PHP   1. {main}() /Users/kenji/work/codeigniter/official/codeigniter4-translations/bin/test:0
PHP   2. require() /Users/kenji/work/codeigniter/official/codeigniter4-translations/bin/test:15
PHP   3. CodeIgniter\Boot::bootTest($paths = class Config\Paths { public string $systemDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../../system'; public string $appDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/..'; public string $writableDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../../writable'; public string $testsDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../../tests'; public string $viewDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../Views' }) /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Test/bootstrap.php:83
PHP   4. CodeIgniter\Boot::setExceptionHandler() /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Boot.php:120
PHP   5. CodeIgniter\Config\BaseService::__callStatic($name = 'exceptions', $arguments = []) /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Boot.php:241
PHP   6. CodeIgniter\Config\BaseService::serviceExists($name = 'exceptions') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/BaseService.php:314
PHP   7. CodeIgniter\Config\BaseService::buildServicesCache() /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/BaseService.php:329
PHP   8. CodeIgniter\Autoloader\FileLocator->findQualifiedNameFromPath($path = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/BaseService.php:394
PHP   9. class_exists($class = 'Translations\\vendor\\codeigniter4\\codeigniter4\\system\\Config\\Services') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Autoloader/FileLocator.php:296
PHP  10. Composer\Autoload\ClassLoader->loadClass($class = 'Translations\\vendor\\codeigniter4\\codeigniter4\\system\\Config\\Services') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Autoloader/FileLocator.php:296
PHP  11. Composer\Autoload\{closure:/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/ClassLoader.php:575-577}($file = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/../..//vendor/codeigniter4/codeigniter4/system/Config/Services.php') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/ClassLoader.php:427
PHP  12. include() /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/ClassLoader.php:576

Fatal error: Cannot declare class CodeIgniter\Config\Services, because the name is already in use in /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php on line 111

Call Stack:
    0.0011     630608   1. {main}() /Users/kenji/work/codeigniter/official/codeigniter4-translations/bin/test:0
    0.0027     645992   2. require('/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Test/bootstrap.php') /Users/kenji/work/codeigniter/official/codeigniter4-translations/bin/test:15
    0.0059     699520   3. CodeIgniter\Boot::bootTest($paths = class Config\Paths { public string $systemDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../../system'; public string $appDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/..'; public string $writableDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../../writable'; public string $testsDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../../tests'; public string $viewDirectory = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/app/Config/../Views' }) /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Test/bootstrap.php:83
    0.0391    2471640   4. CodeIgniter\Boot::setExceptionHandler() /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Boot.php:120
    0.0391    2471640   5. CodeIgniter\Config\BaseService::__callStatic($name = 'exceptions', $arguments = []) /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Boot.php:241
    0.0391    2471640   6. CodeIgniter\Config\BaseService::serviceExists($name = 'exceptions') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/BaseService.php:314
    0.0391    2471640   7. CodeIgniter\Config\BaseService::buildServicesCache() /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/BaseService.php:329
    0.0430    2524336   8. CodeIgniter\Autoloader\FileLocator->findQualifiedNameFromPath($path = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/BaseService.php:394
    0.0431    2543400   9. class_exists($class = 'Translations\\vendor\\codeigniter4\\codeigniter4\\system\\Config\\Services') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Autoloader/FileLocator.php:296
    0.0431    2543528  10. Composer\Autoload\ClassLoader->loadClass($class = 'Translations\\vendor\\codeigniter4\\codeigniter4\\system\\Config\\Services') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Autoloader/FileLocator.php:296
    0.0432    2543720  11. Composer\Autoload\{closure:/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/ClassLoader.php:575-577}($file = '/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/../..//vendor/codeigniter4/codeigniter4/system/Config/Services.php') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/ClassLoader.php:427
    0.0438    2620984  12. include('/Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/codeigniter4/codeigniter4/system/Config/Services.php') /Users/kenji/work/codeigniter/official/codeigniter4-translations/vendor/composer/ClassLoader.php:576

@kenjis
Copy link
Member Author

kenjis commented Apr 13, 2024

The current auto-discovery process flow is as follows:

  1. create all file paths to be checked from the list of namespaces
  2. guess a class name from a file path using namespace path list
  3. check if the class exists

In the sample case above, app/ThirdParty/Shield/Config/Auth.php should not be App\ThirdParty\Shield\Config\Auth.
So there is room for improvement in this flow, but the code needs to be rewritten to do so.

@kenjis kenjis merged commit 666c8fc into codeigniter4:develop Apr 13, 2024
40 checks passed
@kenjis kenjis deleted the fix-findQualifiedNameFromPath-Cannot-declare-class branch April 13, 2024 10:47
@kenjis
Copy link
Member Author

kenjis commented Apr 13, 2024

Hmmm....this solved the error in Shield, but not the error in Translations.

@kenjis
Copy link
Member Author

kenjis commented Apr 13, 2024

I sent #8775.
It seems rewriting is better, but I don't want a big change in the patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants