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

Configured class aliases #2982

Closed
wants to merge 4 commits into from

Conversation

micaherne
Copy link

This patch adds the ability for users to configure known class aliases in their config files as an alternative to creating real class aliases in the bootstrap file. This can help reduce the amount of the analysed code that has to be actually executed, and in particular for legacy applications where ensuring that all the aliased classes can be loaded is tricky.

I was hoping to do this as a plugin (as discussed in #10705) but it doesn't appear to be possible without using non-API internals of PHPStan.

I understand that this is a feature that may only be useful for a minority of users but I think it would be extremely useful in certain situations. I hope you'll consider it.

@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@ondrejmirtes
Copy link
Member

This patch adds the ability for users to configure known class aliases in their config files as an alternative to creating real class aliases in the bootstrap file. This can help reduce the amount of the analysed code that has to be actually executed, and in particular for legacy applications where ensuring that all the aliased classes can be loaded is tricky.

Can you describe these problems more? The class_alias() call needs to happen somewhere in your application so why is this a problem?

but it would save a lot of real class loading if I can.

What's the real-world implication of this that you don't want to do it?

I feel like when no one asked for this up until this point, it's not really worth it. I don't want to add options that would be usable just for a single person in the world.

Also note that you can avoid using class aliases in the analysed code altogether - you can use the actual class names instead.

@micaherne
Copy link
Author

micaherne commented Mar 20, 2024

Can you describe these problems more? The class_alias() call needs to happen somewhere in your application so why is this a problem?

I can certainly describe my particular use case, although I know it's probably an unusual one. I develop plugins for Moodle, which is an online learning platform (used by over half of the world's universities, so it's not just my code). It's a great bit of software but it is a very large codebase has been around for over twenty years so has a number of legacy issues.

The main one for me is that the classloader is initialised as part of the bootstrapping of the application, which also sets up database connections and various other things so it can't easily be loaded on its own. The classloader also only loads a large subset of classes but there are others that are loaded by various other means - mainly includes, but there are also a couple of individual Composer autoloaders that are loaded as needed. Many of the autoloadable classes have dependencies on non-autoloaded ones or global constants that are assumed to be already available in the context they're used.

Even on this mess PHPStan works perfectly if I ask it to scan the whole codebase for symbols statically, but as soon as I have to create real class aliases in my bootstrap file I need to fake the bootstapping of the application and ensure that all dependencies of all 80+ aliased classes are already loaded, which is extremely diifficult. (The aliases in the codebase are only for backward compatibility between versions.)

So being able to statically tell PHPStan what aliases are used in the codebase would be incredibly useful.

I feel like when no one asked for this up until this point, it's not really worth it. I don't want to add options that would be usable just for a single person in the world.

I completely understand that! I figured that there might be other people dealing with legacy systems out there who would be having similar issues but if you haven't encountered that then it possibly isn't worthwhile.

Also note that you can avoid using class aliases in the analysed code altogether - you can use the actual class names instead.

That's actually how I ended up here - I was looking create a plugin to use PHPStan to help me find uses of the old class names so I can update my code. (Amongst other things - it has helped me improve my code tremendously even without the class aliases set up properly.)

@ondrejmirtes
Copy link
Member

One idea - if you referenced the class aliases only in PHPDocs, you could already use a very similar option: https://phpstan.org/writing-php-code/phpdoc-types#global-type-aliases

Another thing - bootstrapping an application for static analysis is a pretty comon practice. I know of several extensions that already do it - Larastan, phpstan-doctrine, phpstan-symfony. About what downsides can you think of when doing it?

@micaherne
Copy link
Author

One idea - if you referenced the class aliases only in PHPDocs, you could already use a very similar option: https://phpstan.org/writing-php-code/phpdoc-types#global-type-aliases

That's a very interesting idea, thanks! I'd looked at type aliases but it hadn't occurred to me they might be usable for this. I'll give it a try.

Another thing - bootstrapping an application for static analysis is a pretty comon practice. I know of several extensions that already do it - Larastan, phpstan-doctrine, phpstan-symfony. About what downsides can you think of when doing it?

There are two downsides for the Moodle software I'm working with:

Firstly, the script that bootstraps the application connects to databases and things like that so isn't appropriate for static analysis. I've been working on a custom script to do this though.

Secondly, the classloader loads many classes which aren't autoloaded, which results in fatal errors. For example all tests extend from a custom testcase class which is loaded only by the PHPUnit bootstrap script. There are tens if not hundreds of other examples though so it's complicated even just to have the classloader running as part of the static analysis.

I do appreciate these are limitations of the software itself rather than anything PHPStan should be working around though, so I'll continue to look at what I need to do to get the application bootstrapped successfully if that is the normal way that frameworks like this are doing it.

@micaherne micaherne closed this Mar 21, 2024
@ondrejmirtes
Copy link
Member

Yeah, Larastan even connects to a database so as long as it's configured properly, it should be fine. Other extensions like https://github.com/staabm/phpstan-dba also benefit from connecting to the database.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants