Skip to content

Conversation

@ryium
Copy link
Contributor

@ryium ryium commented Jun 17, 2023

Catch deprecated implicit type conversions that occur in PHP 8.1 and later.
The following BinaryOp that may cause deprecation.

This PR is part of phpstan/phpstan#8288 and change regarding the mod operator of #2450

※O = safe, D = deprecated, X = not safe

% Mod

L \ R int float string numeric-string Stringable array
int O D X D X X
float D D X D X X
string X X X X X X
numeric-string D D X D X X
Stringable X X X X X X
array X X X X X X
@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

Copy link
Member

Choose a reason for hiding this comment

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

How the type system responds shouldn't change because it's not actually true. The % operator still returns a number but only on some PHP versions it throws a deprecation notice.

Copy link
Member

Choose a reason for hiding this comment

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

So just change the rule please. The condition should be written differently.

  1. Please verify that all the X from table here Catch implicit conversions in mod operator #2466 are reported with 'Binary operation "%s" between %s and %s results in an error.'.
  2. Please write tests that all the D are reported with 'Deprecated in PHP 8.1: Implicit conversion from %s to %s loses precision.'.
  3. Please note that types can for example be int|float or mixed. You should be able to take advantage of RuleLevelHelper so that something that's always wrong like int % float is always reported, something that might be wrong like int % float|int is reported on level 7 and int % mixed is reported on level 9.

Please see how RuleLevelHelper::findTypeToCheck is used in many existing rules like InvalidCastRule. Basically there's a callback that checks the type and transforms the type based on the rule level. Then there's $type instanceof ErrorType with early return and then there's the callback called again on the transformed type.

@ryium ryium force-pushed the ryium/issue/8288-op-mod branch 2 times, most recently from 340a3ed to 264bbb3 Compare December 15, 2023 14:22
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

I like this much more! 👍

But the rule should report this only on 8.1+. Please add a new method in PhpVersion and ask about it in the rule.

@ryium ryium force-pushed the ryium/issue/8288-op-mod branch from e5e12f7 to 02eaab3 Compare February 2, 2024 12:14
@ryium ryium requested a review from ondrejmirtes February 2, 2024 12:41
@ryium ryium force-pushed the ryium/issue/8288-op-mod branch from fcda140 to 07916c7 Compare April 16, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants