Skip to content

Conversation

datamweb
Copy link
Collaborator

@datamweb datamweb commented Feb 2, 2023

CI4 is built for PHP 7.4+, and everything in the framework is namespaced,
except for the helper and lang files.

see codeigniter4/CodeIgniter4#7210

@datamweb datamweb added the refactor Pull requests that refactor code label Feb 2, 2023
@jozefrebjak
Copy link
Contributor

The Rector rule NormalizeNamespaceByPSR4ComposerAutoloadRector is trying to add a namespace to the not-needed classes. I had the same problem in my projects, where rector tried to add a namespace also to the app/Config files. I ended with the removal of that rule.

@datamweb datamweb requested a review from samsonasik February 2, 2023 08:43
@datamweb
Copy link
Collaborator Author

datamweb commented Feb 2, 2023

@jozefrebjak As you know, CI4 heavily uses the namespace. On the other hand, completely eliminating class NormalizeNamespaceByPSR4ComposerAutoloadRector does not seem like a good practice.

I don't understand why you want to not use namespace in app/Config. Anyway you can do as I did.
You can see a more complete explanation in the ignoring-rules-or-paths.

@datamweb
Copy link
Collaborator Author

datamweb commented Feb 2, 2023

@samsonasik Thank you for your quick review.

Kenjis, I prefer to merge after codeigniter4/CodeIgniter4#7210.

@jozefrebjak
Copy link
Contributor

jozefrebjak commented Feb 2, 2023

@jozefrebjak As you know, CI4 heavily uses the namespace. On the other hand, completely eliminating class NormalizeNamespaceByPSR4ComposerAutoloadRector does not seem like a good practice.

I don't understand why you want to not use namespace in app/Config. Anyway you can do as I did. You can see a more complete explanation in the ignoring-rules-or-paths.

By default is for example app/Config/App.php namespaced like:

namespace Config;

after enabling that rule as it's provided in devkit is rector trying to change it to:

namespace  App/Config;

and it ends with error:

Fatal error: Uncaught TypeError: CodeIgniter\CodeIgniter::__construct(): Argument #1 ($config) must be of type Config\App, App\Config\App given, called in /app/vendor/codeigniter4/framework/system/Config/Services.php on line 150 and defined in /app/vendor/codeigniter4/framework/system/CodeIgniter.php:170 

I know how to namespace and I was lazy so I decided to remove that rule till I find a solutions.

@jozefrebjak
Copy link
Contributor

@datamweb Oh, I checked the latest project which I am working on and I already did it in the past :D So sorry about my comment here.

        // Ignore files that should not be namespaced to their folder
        NormalizeNamespaceByPSR4ComposerAutoloadRector::class => [
            __DIR__ . '/app/Common.php',
            __DIR__ . '/app/Config',
            __DIR__ . '/app/Language',
            __DIR__ . '/app/Helpers',
            __DIR__ . '/tests/_support',
        ],

@datamweb
Copy link
Collaborator Author

datamweb commented Feb 2, 2023

So sorry about my comment here.

No problem, I'm glad you're here.

@kenjis kenjis merged commit 17bd6b3 into codeigniter4:develop Feb 3, 2023
@datamweb datamweb deleted the remove-namespace-from-lang branch February 3, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Pull requests that refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants