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

Psalm should load extensions based on composer.json instead of local configuration #5482

Closed
AndrolGenhald opened this issue Mar 25, 2021 · 4 comments
Labels

Comments

@AndrolGenhald
Copy link
Collaborator

AndrolGenhald commented Mar 25, 2021

I was thinking about this recently although it hasn't directly affected me yet, but I saw it pop up for someone else so I thought I'd create an issue.

one of the purposes of the stubs is to enable the static analysis of the code that depends on an extension that is unavailable at runtime.

Oh, in that case you should instead provide your own stubs. If Psalm were to stub all classes it might miss situations where you're missing an extension.

Originally posted by @muglug in #5477 (comment)

With docker it's possible and probably even common for the local php configuration to be different from what's being used to run the code. If I have the PDO extension enabled locally, but composer doesn't require "ext-pdo" then psalm shouldn't load the PDO extension in case my local docker (or production!) environment doesn't use that extension.

It might also be a good idea to add a configuration option to enable or disable specific extensions in psalm.xml.

@orklah
Copy link
Collaborator

orklah commented Oct 14, 2021

That's a comprehensible take by Matt, but I'm not sure I follow because I'm pretty sure Callmap will be loaded no matter what. So why loading simple functionlike definitions but not stubs? (for example: https://psalm.dev/r/097c2a2bf5)

If we wanted to go this far, we would need to identify what part of the callmap belong to which extension to be able to enable/disable them.

That seem far fetched. Honestly, I think we should enable all extensions from the start, loading only stubs conditionally is weird

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/097c2a2bf5
<?php

new Soapclient();
Psalm output (using commit b8a2ba2):

ERROR: TooFewArguments - 3:1 - Too few arguments for Soapclient::__construct - expecting 1 but saw 0

@AndrolGenhald
Copy link
Collaborator Author

Note to self: the decimal extension can be removed from CI if/when this is implemented (#7103). When running tests all extensions should be enabled.

AndrolGenhald added a commit to AndrolGenhald/psalm that referenced this issue Dec 8, 2021
AndrolGenhald added a commit to AndrolGenhald/psalm that referenced this issue Dec 9, 2021
AndrolGenhald added a commit to AndrolGenhald/psalm that referenced this issue Jan 22, 2022
AndrolGenhald added a commit to AndrolGenhald/psalm that referenced this issue Jan 27, 2022
orklah added a commit that referenced this issue Jan 28, 2022
…ons-based-on-composer-config

Enable extensions based on composer.json instead of those loaded at runtime (fixes #5482).
@orklah
Copy link
Collaborator

orklah commented Jan 30, 2022

This can be closed now

@orklah orklah closed this as completed Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants