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

allow setting php version from config or composer.json #2715

Merged
merged 2 commits into from Jan 30, 2020

Conversation

pilif
Copy link
Contributor

@pilif pilif commented Jan 30, 2020

if a composer.json is present and a PHP version requirement is configured, we set the php version to the minimal PHP version that satisfies the composer requirement.

Additionally, this adds a phpVersion attribute to the <psalm> tag. If that's set, it takes precedence over what has been detected in composer.json.

And finally, the --php-version command line flag continues to work and takes precedence over the setting in the <psalm> tag

this fixes #2628

if a composer.json is present and a PHP version requirement is
configured, we set the php version to the minimal PHP version that
satisfies the composer requirement.

Additionally, this adds a `phpVersion` attribute to the <psalm> tag. If
that's set, it takes precedence over what has been detected in
composer.json.

And finally, the --php-version command line flag continues to work and
takes precedence over the setting in the <psalm> tag

this fixes vimeo#2628
Copy link
Collaborator

@muglug muglug left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@muglug muglug merged commit 40497e4 into vimeo:master Jan 30, 2020
$php_version = $composer_json['require']['php'] ?? null;

if ($php_version) {
foreach (['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0'] as $candidate) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like having to do this loop here hard-coding versions, but IMHO there's no way to deterministically reverse a composer range to end up with a minimum version.

What still is a problem is that the knowledge of what versions of PHP psalm has actual support for isn't centralized but so far only existed in this regex here:

if (!preg_match('/^(5\.[456]|7\.[01234]|8\.[0])(\..*)?$/', $version)) {

maybe for the future it would be worth to actually have proper API to get to this information so further duplication isn't needed.

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

Successfully merging this pull request may close these issues.

Add <psalm phpVersion="7.2" support to psalm.xml
2 participants