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

PHPStan 1.0.2 #4943

Merged
merged 1 commit into from
Nov 4, 2021
Merged

PHPStan 1.0.2 #4943

merged 1 commit into from
Nov 4, 2021

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Nov 1, 2021

Q A
Type improvement
BC Break no
Fixed issues N/A

Summary

I've bumped PHPStan to the new 1.0.0 and made the necessary adjustments.

I've also tried out the new level 9 and decided that we probably can live with level 8 on old branches. 😇

[ERROR] Found 742 errors

@derrabus derrabus added this to the 2.13.5 milestone Nov 1, 2021
-
message: '~^Parameter #2 \$count of function array_fill expects int<0, max>, int given\.$~'
message: '~Method Doctrine\\DBAL\\Driver\\Mysqli\\MysqliStatement::rowCount\(\) should return int but returns int\|string\.~'
Copy link
Member Author

@derrabus derrabus Nov 1, 2021

Choose a reason for hiding this comment

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

@morozov This error seems to be raised because of the stub you contributed in phpstan/phpstan-src#700.

The mysqli_stmt::$affected_rows property is documented as int|string which is also what the documentation on php.net says. The documentation does not say in which cases we can expect a string here, but my guess is that this happens if the number of affected rows is larger than PHP_INT_MAX.

However, this also applies to mysqli_stmt::$num_rows which your stub documents as 0|positive-int.

Copy link
Member

Choose a reason for hiding this comment

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

So the stub probably should be 0|positive-int|string, as well as the return type in the DBAL driver.

Copy link
Member

@morozov morozov Nov 2, 2021

Choose a reason for hiding this comment

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

I.e. we also need to change the @return annotation on (Statement|Result)::rowCount() from int to int|string because it will return a string if the value is greater than PHP_INT_MAX.

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 is a bit of a problem because on 3.1.x, we already have an int return type declaration.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Then I think it's acceptable. Unlike the discussion in doctrine/orm#8963 (comment), updating or deleting a number of rows larger than PHP_INT_MAX is close to impossible.

@derrabus derrabus requested a review from morozov November 1, 2021 11:10
@derrabus derrabus force-pushed the bump/phpstan-on-2.13 branch from c6e163f to 52eb197 Compare November 1, 2021 11:35
@derrabus derrabus mentioned this pull request Nov 1, 2021
@derrabus derrabus force-pushed the bump/phpstan-on-2.13 branch from 52eb197 to 2dc8ff1 Compare November 1, 2021 13:58
-
message: '~^Parameter #2 \$count of function array_fill expects int<0, max>, int given\.$~'
message: '~Method Doctrine\\DBAL\\Driver\\Mysqli\\MysqliStatement::rowCount\(\) should return int but returns int\|string\.~'
Copy link
Member

Choose a reason for hiding this comment

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

So the stub probably should be 0|positive-int|string, as well as the return type in the DBAL driver.

@derrabus derrabus force-pushed the bump/phpstan-on-2.13 branch 3 times, most recently from 6362aff to 255ad58 Compare November 1, 2021 23:42
@derrabus derrabus force-pushed the bump/phpstan-on-2.13 branch 2 times, most recently from 8e21a5d to a39bfc8 Compare November 2, 2021 19:36
@derrabus derrabus changed the title PHPStan 1.0.0 PHPStan 1.0.1 Nov 2, 2021
Signed-off-by: Alexander M. Turek <me@derrabus.de>
@derrabus derrabus force-pushed the bump/phpstan-on-2.13 branch from a39bfc8 to fd088f1 Compare November 4, 2021 12:06
@derrabus derrabus changed the title PHPStan 1.0.1 PHPStan 1.0.2 Nov 4, 2021
@greg0ire greg0ire merged commit 01595c0 into doctrine:2.13.x Nov 4, 2021
@greg0ire
Copy link
Member

greg0ire commented Nov 4, 2021

Thanks @derrabus !

@derrabus derrabus deleted the bump/phpstan-on-2.13 branch November 4, 2021 21:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 5, 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.

4 participants