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.

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

Jump to bottom

modulo '1' is always zero #973

Merged
merged 5 commits into from Feb 6, 2022
+49 −4
Merged

modulo '1' is always zero #973

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9277dd
modulo '1' is always zero
clxmstaab Feb 1, 2022
1ddeddc
partly revert automated rename
clxmstaab Feb 1, 2022
cbc90ec
cover 1.0
staabm Feb 1, 2022
2c0306e
cover float 1.0
staabm Feb 1, 2022
578e96d
cover division by false-values
staabm Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 19 additions & 7 deletions src/Analyser/MutatingScope.php
View file
Open in desktop
Expand Up @@ -1051,12 +1051,24 @@ private function resolveType(Expr $node): Type
} else {
$right = $node->right;
}
$rightType = $this->getType($right);

$stringType = $rightType->toString();
if (
$node instanceof Node\Expr\BinaryOp\Mod
|| $node instanceof Node\Expr\AssignOp\Mod
) {
if ($stringType instanceof ConstantScalarType && $stringType->getValue() === '1') {
staabm marked this conversation as resolved.
Show resolved Hide resolved
return new ConstantIntegerType(0);
}
}

$rightScalarTypes = TypeUtils::getConstantScalars($rightType->toNumber());
foreach ($rightScalarTypes as $scalarType) {

$rightTypes = TypeUtils::getConstantScalars($this->getType($right)->toNumber());
foreach ($rightTypes as $rightType) {
if (
$rightType->getValue() === 0
|| $rightType->getValue() === 0.0
$scalarType->getValue() === 0
|| $scalarType->getValue() === 0.0
Copy link
Member

Choose a reason for hiding this comment

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

What about '0' here? Again, you can try ->toInteger().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this code was existant before this PR and covers a different use-case.
should I change it with this PR nevertheless , or better do so in a different PR?

Copy link
Member

Choose a reason for hiding this comment

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

You can do it here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

newly added testcases in 578e96d show that division by '0' and other falsy values work as expected.
so IMO we just leave this as is, right?

) {
return new ErrorType();
}
Expand All @@ -1078,15 +1090,15 @@ private function resolveType(Expr $node): Type
}

$leftTypes = TypeUtils::getConstantScalars($this->getType($left));
$rightTypes = TypeUtils::getConstantScalars($this->getType($right));
$rightScalarTypes = TypeUtils::getConstantScalars($this->getType($right));

$leftTypesCount = count($leftTypes);
$rightTypesCount = count($rightTypes);
$rightTypesCount = count($rightScalarTypes);
if ($leftTypesCount > 0 && $rightTypesCount > 0) {
$resultTypes = [];
$generalize = $leftTypesCount * $rightTypesCount > self::CALCULATE_SCALARS_LIMIT;
foreach ($leftTypes as $leftType) {
foreach ($rightTypes as $rightType) {
foreach ($rightScalarTypes as $rightType) {
$resultType = $this->calculateFromScalars($node, $leftType, $rightType);
if ($generalize) {
$resultType = TypeUtils::generalizeType($resultType, GeneralizePrecision::lessSpecific());
Expand Down
22 changes: 22 additions & 0 deletions tests/PHPStan/Analyser/data/modulo-operator.php
View file
Open in desktop
Expand Up @@ -45,4 +45,26 @@ function doBar(int $i, int $j, $p, $range, $zeroOrMore, $intConst, $unionRange,

assertType('int', $j % $i);
}

function moduleOne(int $i, float $f) {
assertType('0', true % '1');
assertType('0', false % '1');
assertType('0', null % '1');
assertType('0', -1 % '1');
assertType('0', 0 % '1');
assertType('0', 1 % '1');
assertType('0', '1' % '1');
assertType('0', 1.24 % '1');

assertType('0', $i % '1');
assertType('0', $f % '1');

assertType('0', $i % true);
assertType('0', $f % true);

$i %= '1';
$f %= '1';
assertType('0', $i);
assertType('0', $f);
}
}