Skip to content

Add missing ldap_unbind mapping for php 8.1#8126

Merged
orklah merged 2 commits intovimeo:4.xfrom
othercorey:patch-1
Jun 20, 2022
Merged

Add missing ldap_unbind mapping for php 8.1#8126
orklah merged 2 commits intovimeo:4.xfrom
othercorey:patch-1

Conversation

@othercorey
Copy link
Copy Markdown
Contributor

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jun 19, 2022

Callmap.php should be updated too

@othercorey
Copy link
Copy Markdown
Contributor Author

othercorey commented Jun 19, 2022

All of the ldap functions in callmap.php seem to use the old resource type. How does this work with different php versions?

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jun 19, 2022

Please take a look at https://psalm.dev/docs/contributing/editing_callmaps/ for the full explanation. Short version: callmap.php contains current signature and versionned files alters it for each successive older version to match with the user's version

@othercorey
Copy link
Copy Markdown
Contributor Author

Updated ldap_unbind in callmap. It looks like there are other incorrect types for ldap functions. I can try to open another PR for that later, but it's a lot of manual checking.

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Jun 20, 2022
@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jun 20, 2022

Thanks! Follow up is welcome :)

Some contributor recently made a tool to detect inconsistencies in callmaps (through reflection) and flagged ldap_ functions as potentially wrong: https://github.com/vimeo/psalm/pull/8104/files#diff-b16377a21d55a13a48cbdbdfbdf7479b2d9dda80f93b3b81202f88bd1e230b13R97
Your fix was probably the reason that justified excluding this range of functions

You could try to remove this line in a new PR an look at CI results for failures in order to identify cases that would need fixing

/cc @SamMousa

@SamMousa
Copy link
Copy Markdown
Contributor

See #8121

@orklah orklah merged commit be4d0ff into vimeo:4.x Jun 20, 2022
@othercorey othercorey deleted the patch-1 branch June 20, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants