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 proc_open stub for php >= 8.0 #7443

Merged
merged 2 commits into from Jan 21, 2022
Merged

Conversation

brainlock
Copy link
Contributor

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c0ebca79f2
<?php

proc_open(
    command: "ls",
    descriptor_spec: [],
    pipes: $pipes,
    cwd: null,
    env_vars: null,
    options: null
);
Psalm output (using commit 5a6dccd):

ERROR: UndefinedGlobalVariable - 6:12 - Cannot find referenced variable $pipes in global scope

ERROR: InvalidNamedArgument - 4:5 - Parameter $command does not exist on function proc_open

ERROR: InvalidNamedArgument - 5:5 - Parameter $descriptor_spec does not exist on function proc_open

ERROR: InvalidNamedArgument - 6:5 - Parameter $pipes does not exist on function proc_open

ERROR: InvalidNamedArgument - 8:5 - Parameter $env_vars does not exist on function proc_open

ERROR: InvalidNamedArgument - 9:5 - Parameter $options does not exist on function proc_open

@orklah
Copy link
Collaborator

orklah commented Jan 20, 2022

Thanks! Could you fix param names back to Callmap_historical instead of creating a delta on 8.0? Param names were rarely defined before and it didn't matter anyway so it's better to have some consistency across versions

Since before 8.0 the named arguments were not part of the interface, we
don't care about the intermediate steps of the proc_open definition. For
consistency, this makes the definition the same across all versions.

This also fixes the type for the `options` argument already in
CallMap_historical to be nullable.

The names of the arguments are now consistent across versions, while the
delta for 7.4 reflects the change of the `command` argument from
`string` to `string|array`.
@brainlock
Copy link
Contributor Author

Sure! I pushed another commit which unifies the argument names across all versions, and fixes the type for the options argument (should be nullable) already in the historical map.

Now the only delta for proc_open is in the 7.4 CallMap, where its type was changed from string to string|array.

Do you prefer the two commits to be squashed?

@orklah
Copy link
Collaborator

orklah commented Jan 21, 2022

Thanks! That's great!

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Jan 21, 2022
@orklah orklah merged commit f9ea275 into vimeo:4.x Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Needs work release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants