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

Documentation on DBAL\Driver\Statement::bindValue is incorrect #4212

Closed
wants to merge 3 commits into from

Conversation

gjdanis
Copy link
Contributor

@gjdanis gjdanis commented Aug 18, 2020

This change updates the documentation on the bindValue function. Per the implementation of DBAL\Statement this function can take a string, int, or Type instance.

Fixes #4210

Q A
Type improvement
BC Break no
Fixed issues #4210

Summary

This change updates the documentation on the bindValue to fix problems with static analyzers. Since this doesn't appear to be a problem in 3.x versions of this library, I've only fixed the issue in 2.10.x.

Joe Danis added 3 commits August 17, 2020 20:10
This change updates the documentation on the `bindValue` function. Per the implementation this function can take a string, int, or type instance.

Fixes doctrine#4210
@greg0ire greg0ire requested a review from morozov August 18, 2020 06:44
@@ -23,8 +23,8 @@ interface Statement extends ResultStatement
* this will be a parameter name of the form :name. For a prepared statement
* using question mark placeholders, this will be the 1-indexed position of the parameter.
* @param mixed $value The value to bind to the parameter.
* @param int $type Explicit data type for the parameter using the {@link ParameterType}
* constants.
* @param mixed $type Explicit data type for the parameter using the {@link ParameterType}
Copy link
Member

Choose a reason for hiding this comment

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

This change is invalid. All implementations of this interface must accept integers. A specific implementation (Doctrine\DBAL\Statement) accepts other types but they are not part of the interface.

Copy link
Contributor Author

@gjdanis gjdanis Aug 18, 2020

Choose a reason for hiding this comment

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

Thanks @morozov - is the documentation incorrect then on Doctrine\DBAL\Connection::prepare? FWIW changing the documentation on prepare to return a Doctrine\DBAL\Statement (and not a driver statement) fixes the issue for us.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like it's already fixed in 15d9be0 but not yet released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there plans to create a release in the near future?

Copy link
Member

Choose a reason for hiding this comment

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

We're waiting for #3980 to be resolved in order to release 2.10.3, 2.11.0, and 3.0.0. We can release 2.10.3 earlier but there's no reason for that right now.

@gjdanis gjdanis requested a review from morozov August 18, 2020 22:01
@morozov morozov self-assigned this Aug 20, 2020
@morozov morozov closed this Aug 20, 2020
@morozov morozov removed their request for review October 23, 2020 17:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation on DBAL\Driver\Statement.php is incorrect
2 participants