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

Fix string|float-to-int convertion errors #1271

Merged

Conversation

jchaffraix
Copy link
Collaborator

@jchaffraix jchaffraix commented Jul 7, 2024

There were 3 major errors:

  • int fields from the DB that are returned as string by mysqli.
  • date('s|d|y|U') that returns an int as a string.
  • PHP float truncating functions that returns a float.

Update by cpeel: Sandbox avail at https://www.pgdp.org/~cpeel/c.branch/julien_fix_int_to_str_conv/

@jchaffraix jchaffraix marked this pull request as draft July 7, 2024 07:37
There were 3 major errors:
- int fields from the DB that are returned as string by mysqli.
- date('s|d|y|U') that returns a int as a string.
- PHP float truncating functions that returns a float.
@@ -24,7 +24,7 @@ parameters:
path: pinc/3rdparty/mediawiki/DairikiDiff.php
- message: '#Comparison operation .* results in an error#'
path: pinc/3rdparty/mediawiki/DiffEngine.php
- message: '#^Parameter \#2 \$num of function array_fill expects int, float given\.$#'
- message: '#^Parameter \#2 \$.* of function array_fill expects int, float given\.$#'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a small mistake I made in the previous PR that makes PHPStan fails: PHP 8 does actually see the error but with a different parameter name.

$foo_Header,
$foo_field_name,
$blather
string $id,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this type to prove that that the $id parameter is a string and thus we need to manually ignore some issue in PHPStan on pinc/stages.inc.

The other types in the constructors were added bonuses.

All the types are copied from the class member's.

@jchaffraix jchaffraix marked this pull request as ready for review July 7, 2024 07:50
@cpeel cpeel requested review from chrismiceli and srjfoo July 7, 2024 13:44
@@ -307,7 +307,7 @@ new Stage(
// -----------------------------------------------------------------------------

new Pool(
'PPV',
'PPV', // @phpstan-ignore-line PHPStan 1.10 reports a string-to-int conv here, seems like a bug
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it worth setting up a minimal example and creating an issue against phpstan?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I just did! Also the reproduction showed that the issue is that we reuse the name Pool that exists in the PHP standard library and confuses PHPStan:

phpstan/phpstan#11303

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's awesome, thanks.

@cpeel cpeel added the ready-for-merge PR is ready for merging after freeze/slush label Jul 8, 2024
@cpeel cpeel merged commit 54488d0 into DistributedProofreaders:master Jul 11, 2024
12 checks passed
@jchaffraix jchaffraix deleted the julien_fix_int_to_str_conv branch July 14, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge PR is ready for merging after freeze/slush
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants