Skip to content

Fix PDO::PARAM_* constant values#1273

Merged
Ivan Fedorov (isfedorov) merged 3 commits into
JetBrains:masterfrom
staabm:patch-3
Nov 12, 2021
Merged

Fix PDO::PARAM_* constant values#1273
Ivan Fedorov (isfedorov) merged 3 commits into
JetBrains:masterfrom
staabm:patch-3

Conversation

@staabm

@staabm Markus Staab (staabm) commented Nov 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

fixed incorrect constant values and re-ordered the constant ascending by their value

see https://3v4l.org/O0hDh

refs phpstan/phpstan#5985

@staabm

Markus Staab (staabm) commented Nov 12, 2021

Copy link
Copy Markdown
Contributor Author

could someone give me a hint what the test-failures are about and how I am supposed to fix them?

There were 4 failures:

1) StubTests\StubsConstantsAndParametersValuesTest::testMethodsDefaultParametersValue with data set "PDO::quote(type)" (StubTests\Model\PHPClass Object (...), StubTests\Model\PHPMethod Object (...), StubTests\Model\PHPParameter Object (...))
Reflection method PDO::quote has optional parameter type with default value 2 but stub parameter has value 1
Failed asserting that 1 matches expected '2'.

/opt/project/phpstorm-stubs/tests/StubsConstantsAndParametersValuesTest.php:79

2) StubTests\StubsConstantsAndParametersValuesTest::testMethodsDefaultParametersValue with data set "PDOStatement::bindColumn(type)" (StubTests\Model\PHPClass Object (...), StubTests\Model\PHPMethod Object (...), StubTests\Model\PHPParameter Object (...))
Reflection method PDOStatement::bindColumn has optional parameter type with default value 2 but stub parameter has value 1
Failed asserting that 1 matches expected '2'.

/opt/project/phpstorm-stubs/tests/StubsConstantsAndParametersValuesTest.php:79

3) StubTests\StubsConstantsAndParametersValuesTest::testMethodsDefaultParametersValue with data set "PDOStatement::bindParam(type)" (StubTests\Model\PHPClass Object (...), StubTests\Model\PHPMethod Object (...), StubTests\Model\PHPParameter Object (...))
Reflection method PDOStatement::bindParam has optional parameter type with default value 2 but stub parameter has value 1
Failed asserting that 1 matches expected '2'.

/opt/project/phpstorm-stubs/tests/StubsConstantsAndParametersValuesTest.php:79

4) StubTests\StubsConstantsAndParametersValuesTest::testMethodsDefaultParametersValue with data set "PDOStatement::bindValue(type)" (StubTests\Model\PHPClass Object (...), StubTests\Model\PHPMethod Object (...), StubTests\Model\PHPParameter Object (...))
Reflection method PDOStatement::bindValue has optional parameter type with default value 2 but stub parameter has value 1
Failed asserting that 1 matches expected '2'.

/opt/project/phpstorm-stubs/tests/StubsConstantsAndParametersValuesTest.php:79

@isfedorov

Ivan Fedorov (isfedorov) commented Nov 12, 2021

Copy link
Copy Markdown
Contributor

Markus Staab (@staabm) the failures happened due to changed constants are being used as default values in some methods

PDO::quote -> parameter $type
PDOStatement::bindColumn -> parameter $type
PDOStatement::bindParam -> parameter $type
PDOStatement::bindValue -> parameter $type

According to reflection, those default parameters have to have a value equal to 2. Signatures used to use PDO::PARAM_INT with value 2 but after your changes the value of constant has been changed and as result signatures should be updated as well

@staabm

Markus Staab (staabm) commented Nov 12, 2021

Copy link
Copy Markdown
Contributor Author

thx for the feedback.

the value of constant has been changed and as result signatures should be updated as well

which signatures ? I don't see where a hardcoded value needs an update?

@staabm

Markus Staab (staabm) commented Nov 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

According to reflection, those default parameters have to have a value equal to 2

where does this reflection data come from?

if you mean by the php-runtime, the following testsnippets shows the values match with the changes from this PR:
https://3v4l.org/BVYCL

could you give me a hint? I have no idea what todo

@isfedorov

Ivan Fedorov (isfedorov) commented Nov 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

which signatures ? I don't see where a hardcoded value needs an update?

Next methods in stubs.

PDO::quote -> parameter $type
PDOStatement::bindColumn -> parameter $type
PDOStatement::bindParam -> parameter $type
PDOStatement::bindValue -> parameter $type

You have to just update default values of parameters because currently they are declared like $type = PDO::PARAM_INT but should be $type = PDO::PARAM_STR

if you mean by the php-runtime, the following testsnippets shows the values match with the changes from this PR:

I don't argue with that, I'm just saying that some additional changes required.

@staabm

Markus Staab (staabm) commented Nov 12, 2021

Copy link
Copy Markdown
Contributor Author

You have to just update default values of parameters because currently they are declared like $type = PDO::PARAM_INT but should be $type = PDO::PARAM_STR

ohh lol, thx for the hint. didn't realize that the used constant is wrong. fixed it now.

BTW, why did you remove PARAM_LOB and PARAM_STMT?

I did not remove any constant. just re-ordered them according to their values.

@staabm Markus Staab (staabm) changed the title Fix PDO::PARAM_* constant values Fix PDO::PARAM_* constant values Nov 12, 2021
@isfedorov

Ivan Fedorov (isfedorov) commented Nov 12, 2021

Copy link
Copy Markdown
Contributor

I did not remove any constant. just re-ordered them according to their values.

Yeah..I've found that later :) my bad. Thank you for the PR!

@isfedorov Ivan Fedorov (isfedorov) merged commit b66b768 into JetBrains:master Nov 12, 2021
@staabm Markus Staab (staabm) deleted the patch-3 branch November 12, 2021 16:56
@staabm Markus Staab (staabm) mentioned this pull request Nov 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@staabm @isfedorov @clxmstaab