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

Convert ParameterType to enum #5548

Merged
merged 1 commit into from Aug 1, 2022
Merged

Conversation

morozov
Copy link
Member

@morozov morozov commented Jul 29, 2022

Q A
Type improvement

Positive outcomes:

  1. The driver-level statement API is now more robust. The UnknownParameterType exception is no longer possible since all possible values of the parameter type are constrained by the enum.
  2. The driver-level parameter types and the wrapper-level array types which were previously described by a single int type are now distinct (ParameterType and int respectively). This is important since the wrapper-level array types have non-obviously limited support. For instance, they can be used in Connection::executeQuery() but not in Statement::bindValue().

Negative outcomes:

  1. The PHP types describing parameter types became more complex (which reflects how complex they really are).

As the next step, we can simplify the type of parameter types by making them non-nullable. There is already a default of "string" (see #5550).

TODO:

Comment on lines +128 to +131
<!-- The union types representing prepared statement parameter types are too complex -->
<rule ref="Generic.Files.LineLength.TooLong">
<exclude-pattern>src/Cache/QueryCacheProfile.php</exclude-pattern>
</rule>
Copy link
Member Author

Choose a reason for hiding this comment

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

This chunk is dedicated to all fans of union types.

@morozov morozov merged commit 2dd0f52 into doctrine:4.0.x Aug 1, 2022
@morozov morozov deleted the parameter-type branch August 1, 2022 20:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2023
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.

None yet

2 participants