Skip to content

Missing arginfo arginfo_pdostatement_setfetchmode #3114

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

Closed
wants to merge 1 commit into from
Closed

Missing arginfo arginfo_pdostatement_setfetchmode #3114

wants to merge 1 commit into from

Conversation

carusogabriel
Copy link
Contributor

Another found with PHPStan:

Method PDOStatement::setFetchMode() invoked with 3 parameters, 1-2 required.

By the docs, this method has variadics arguments, so how should we call it?

@carusogabriel carusogabriel changed the base branch from master to PHP-7.1 February 17, 2018 00:29
PDO::setFetchMode receives up to 3 params
@nikic
Copy link
Member

nikic commented Mar 23, 2018

Unfortunately, this is a BC breaking change: https://3v4l.org/KgHR3 Given that these types of changes pretty much always seems to break one library or another, I'm going to close this.

Maybe the LSP rules could be changed to allow these changes in the future though...

@nikic nikic closed this Mar 23, 2018
@carusogabriel
Copy link
Contributor Author

@nikic Thanks for the feedback.

Btw, LSP is the Liskov Substitution Principle?

@carusogabriel carusogabriel deleted the patch-1 branch March 23, 2018 18:58
@nikic
Copy link
Member

nikic commented Mar 23, 2018

@carusogabriel Yes. In particular I'm referring to our signature checks during inheritance. I think the current handling of parameters additions/removals is not ideal (though it's a somewhat tricky situation, especially considering the differences in handling of additional arguments for internal and user functions).

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.

2 participants