Skip to content

Catch implicit conversions in mod operator #2466

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.

Sign up for GitHub

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

Open
ryium wants to merge 2 commits into
base: 2.3.x
Choose a base branch
from
Open

Catch implicit conversions in mod operator #2466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bbc3d2
Report the deprecated implicit float to int conversion of the % operator
ryium Oct 31, 2026
b09992e
Add regression test for #11886
ondrejmirtes Oct 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/Rules/Operators/InvalidBinaryOperationRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use PHPStan\DependencyInjection\RegisteredRule;
use PHPStan\Node\Expr\TypeExpr;
use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\MultipleNodeTypesRule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Rules\RuleLevelHelper;
Expand All @@ -29,6 +31,7 @@
public function __construct(
private ExprPrinter $exprPrinter,
private RuleLevelHelper $ruleLevelHelper,
private PhpVersion $phpVersion,
)
{
}
Expand All @@ -44,6 +47,19 @@
}

public function processNode(Node $node, Scope $scope): array
{
$errors = $this->checkInvalidOperation($node, $scope);
if ($errors !== [] || !$node instanceof Node\Expr\BinaryOp\Mod || $scope->getType($node) instanceof ErrorType) {
return $errors;
}

return $this->checkModImplicitConversion($node, $scope);
}

/**
* @return list<IdentifierRuleError>
*/
private function checkInvalidOperation(Node $node, Scope $scope): array
{
if (
!$node instanceof Node\Expr\BinaryOp
Expand Down Expand Up @@ -144,4 +160,37 @@
];
}

/**
* `%` converts both operands to int: a float operand loses precision,
* which PHP 8.1+ reports as deprecated.
*
* @return list<IdentifierRuleError>
*/
private function checkModImplicitConversion(Node\Expr\BinaryOp\Mod $node, Scope $scope): array
{
if (!$this->phpVersion->deprecatesImplicitlyFloatConversionToInt()) {
return [];
}

$callback = static fn (Type $type): bool => !$type->isFloat()->no();

Check warning on line 175 in src/Rules/Operators/InvalidBinaryOperationRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 			return [];
 		}
 
-		$callback = static fn (Type $type): bool => !$type->isFloat()->no();
+		$callback = static fn (Type $type): bool => $type->isFloat()->yes();
 		foreach ([$node->left, $node->right] as $operand) {
 			$type = $this->ruleLevelHelper->findTypeToCheck($scope, $operand, '', $callback)->getType();
 			if ($type instanceof ErrorType || $type->toNumber()->isFloat()->no()) {

Check warning on line 175 in src/Rules/Operators/InvalidBinaryOperationRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 			return [];
 		}
 
-		$callback = static fn (Type $type): bool => !$type->isFloat()->no();
+		$callback = static fn (Type $type): bool => $type->isFloat()->yes();
 		foreach ([$node->left, $node->right] as $operand) {
 			$type = $this->ruleLevelHelper->findTypeToCheck($scope, $operand, '', $callback)->getType();
 			if ($type instanceof ErrorType || $type->toNumber()->isFloat()->no()) {
foreach ([$node->left, $node->right] as $operand) {
$type = $this->ruleLevelHelper->findTypeToCheck($scope, $operand, '', $callback)->getType();
if ($type instanceof ErrorType || $type->toNumber()->isFloat()->no()) {
continue;
}

return [
RuleErrorBuilder::message(sprintf(
'Deprecated in PHP 8.1: Implicit conversion from %s to int loses precision.',
$type->describe(VerbosityLevel::value()),
))
->line($node->left->getStartLine())
->identifier('binaryOp.mod.implicitConversion')
->build(),
];
}

return [];
}

}
197 changes: 195 additions & 2 deletions tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@

use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Node\Printer\Printer;
use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleLevelHelper;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;
use function array_merge;
use function usort;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<InvalidBinaryOperationRule>
Expand All @@ -33,6 +37,7 @@ protected function getRule(): Rule
checkBenevolentUnionTypes: false,
discoveringSymbolsTip: true,
),
new PhpVersion(PHP_VERSION_ID),
);
}

Expand Down Expand Up @@ -616,7 +621,7 @@ public function testBug10440(): void

public function testBenevolentUnion(): void
{
$this->analyse([__DIR__ . '/data/binary-op-benevolent-union.php'], [
$errors = [
[
'Binary operation "+" between (array<string, string>|bool|int|object|resource) and BinaryOpBenevolentUnion\Foo results in an error.',
12,
Expand Down Expand Up @@ -801,7 +806,15 @@ public function testBenevolentUnion(): void
'Binary operation "|=" between BinaryOpBenevolentUnion\\Foo and (array<string, string>|bool|int|object|resource) results in an error.',
365,
],
]);
];
if (PHP_VERSION_ID >= 80100) {
$errors[] = [
'Deprecated in PHP 8.1: Implicit conversion from 1.23 to int loses precision.',
138,
];
usort($errors, static fn (array $a, array $b): int => $a[1] <=> $b[1]);
}
$this->analyse([__DIR__ . '/data/binary-op-benevolent-union.php'], $errors);
}

public function testBug12123(): void
Expand Down Expand Up @@ -858,4 +871,184 @@ public function testBug1452(): void
$this->analyse([__DIR__ . '/data/bug-1452.php'], []);
}

public function testBug8288(): void
{
$expectedErrors = [];
if (PHP_VERSION_ID >= 80100) {
$expectedErrors = [
[
'Deprecated in PHP 8.1: Implicit conversion from float to int loses precision.',
17,
],
[
'Deprecated in PHP 8.1: Implicit conversion from numeric-string to int loses precision.',
18,
],
[
'Deprecated in PHP 8.1: Implicit conversion from float to int loses precision.',
19,
],
[
'Deprecated in PHP 8.1: Implicit conversion from float to int loses precision.',
20,
],
[
'Deprecated in PHP 8.1: Implicit conversion from float to int loses precision.',
21,
],
[
'Deprecated in PHP 8.1: Implicit conversion from numeric-string to int loses precision.',
22,
],
[
'Deprecated in PHP 8.1: Implicit conversion from numeric-string to int loses precision.',
23,
],
[
'Deprecated in PHP 8.1: Implicit conversion from numeric-string to int loses precision.',
24,
],
];
}
$expectedErrors = array_merge($expectedErrors, [
[
'Binary operation "%" between int and string results in an error.',
26,
],
[
'Binary operation "%" between int and Stringable results in an error.',
27,
],
[
'Binary operation "%" between int and array results in an error.',
28,
],
[
'Binary operation "%" between float and string results in an error.',
29,
],
[
'Binary operation "%" between float and Stringable results in an error.',
30,
],
[
'Binary operation "%" between float and array results in an error.',
31,
],
[
'Binary operation "%" between string and int results in an error.',
32,
],
[
'Binary operation "%" between string and float results in an error.',
33,
],
[
'Binary operation "%" between string and string results in an error.',
34,
],
[
'Binary operation "%" between string and numeric-string results in an error.',
35,
],
[
'Binary operation "%" between string and Stringable results in an error.',
36,
],
[
'Binary operation "%" between string and array results in an error.',
37,
],
[
'Binary operation "%" between numeric-string and string results in an error.',
38,
],
[
'Binary operation "%" between numeric-string and Stringable results in an error.',
39,
],
[
'Binary operation "%" between numeric-string and array results in an error.',
40,
],
[
'Binary operation "%" between Stringable and int results in an error.',
41,
],
[
'Binary operation "%" between Stringable and float results in an error.',
42,
],
[
'Binary operation "%" between Stringable and string results in an error.',
43,
],
[
'Binary operation "%" between Stringable and numeric-string results in an error.',
44,
],
[
'Binary operation "%" between Stringable and Stringable results in an error.',
45,
],
[
'Binary operation "%" between Stringable and array results in an error.',
46,
],
[
'Binary operation "%" between array and int results in an error.',
47,
],
[
'Binary operation "%" between array and float results in an error.',
48,
],
[
'Binary operation "%" between array and string results in an error.',
49,
],
[
'Binary operation "%" between array and numeric-string results in an error.',
50,
],
[
'Binary operation "%" between array and Stringable results in an error.',
51,
],
[
'Binary operation "%" between array and array results in an error.',
52,
],
]);
if (PHP_VERSION_ID >= 80100) {
$expectedErrors = array_merge($expectedErrors, [
[
'Deprecated in PHP 8.1: Implicit conversion from float|int<0, 15> to int loses precision.',
58,
],
[
'Deprecated in PHP 8.1: Implicit conversion from 6.0625 to int loses precision.',
59,
],
[
'Deprecated in PHP 8.1: Implicit conversion from 6.0625 to int loses precision.',
60,
],
]);
}
$this->analyse([__DIR__ . '/data/bug8288.php'], $expectedErrors);
}

public function testBug11886(): void
{
$errors = [];
if (PHP_VERSION_ID >= 80100) {
$errors[] = [
'Deprecated in PHP 8.1: Implicit conversion from 4.1 to int loses precision.',
3,
];
}
$this->analyse([__DIR__ . '/data/bug-11886.php'], $errors);
}

}
3 changes: 3 additions & 0 deletions tests/PHPStan/Rules/Operators/data/bug-11886.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php declare(strict_types = 1);

$x = 4.1 % 1;
70 changes: 70 additions & 0 deletions tests/PHPStan/Rules/Operators/data/bug8288.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php

namespace Bug8288;

use Stringable;

class Bug8288 {

/**
* @param numeric-string $numericString
*/
public function opMod(int $int, float $float, string $string, string $numericString, Stringable $stringable, array $array)
{
# Safe
$int % $int;
# Deprecated in PHP 8.1
$int % $float;
$int % $numericString;
$float % $int;
$float % $float;
$float % $numericString;
$numericString % $int;
$numericString % $float;
$numericString % $numericString;
# Not safe
$int % $string;
$int % $stringable;
$int % $array;
$float % $string;
$float % $stringable;
$float % $array;
$string % $int;
$string % $float;
$string % $string;
$string % $numericString;
$string % $stringable;
$string % $array;
$numericString % $string;
$numericString % $stringable;
$numericString % $array;
$stringable % $int;
$stringable % $float;
$stringable % $string;
$stringable % $numericString;
$stringable % $stringable;
$stringable % $array;
$array % $int;
$array % $float;
$array % $string;
$array % $numericString;
$array % $stringable;
$array % $array;

// The following three examples should all be flagged as resulting in implicit float to int conversion with potential loss of precision. This behaviour is deprecated in PHP 8.1, and results in a deprecation warning (if deprecation warnings are enabled, which they are by default PHP 8+), if encountered at runtime. It would be helpful if phpstan could catch and flag such cases during static analysis.
$singleByteCode = ord('a');
$float1 = ( $singleByteCode / 16 );
// This is not safe
$float1 % 15;
( 97 / 16 ) % 15;
6.0625 % 15;

// The following three examples should not result in any error
$singleByteCode = ord('a');
$int1 = intdiv( $singleByteCode, 16 );
// This is safe
$int1 % 15;
intdiv( 97, 16 ) % 15;
6 % 15;
}
}
Loading