Skip to content

Commit

Permalink
Fix PHP notices and add error for implicit float to int precision loss
Browse files Browse the repository at this point in the history 
Fix #10950
Implement #10949
  • Loading branch information
@kkmuffme
kkmuffme committed May 5, 2024
1 parent a6f3a4f commit 9784367
Show file tree
Hide file tree
Showing 3 changed files with 251 additions and 35 deletions.
180 changes: 151 additions & 29 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,17 @@
use function array_diff_key;
use function array_values;
use function count;
use function filter_var;
use function get_class;
use function is_int;
use function is_numeric;
use function max;
use function min;
use function preg_match;
use function strtolower;

use const FILTER_VALIDATE_FLOAT;
use const FILTER_VALIDATE_INT;

/**
* @internal
*/
Expand Down Expand Up @@ -311,6 +314,9 @@ private static function analyzeOperands(
bool &$has_string_increment,
?Union &$result_type = null
): ?Union {
$implicit_left = self::noFloatLikeToIntPrecisionLoss($parent, $left_type_part, $invalid_left_messages);
$implicit_right = self::noFloatLikeToIntPrecisionLoss($parent, $right_type_part, $invalid_right_messages);

if (($left_type_part instanceof TLiteralInt || $left_type_part instanceof TLiteralFloat)
&& ($right_type_part instanceof TLiteralInt || $right_type_part instanceof TLiteralFloat)
&& (
Expand Down Expand Up @@ -349,8 +355,8 @@ private static function analyzeOperands(
$result_type,
);

$has_valid_left_operand = true;
$has_valid_right_operand = true;
$has_valid_left_operand = $implicit_left;
$has_valid_right_operand = $implicit_right;

return null;
}
Expand Down Expand Up @@ -406,6 +412,10 @@ private static function analyzeOperands(
if (count($combined_atomic_types) <= 2) {
$left_type_part = $combined_atomic_types[0];
$right_type_part = $combined_atomic_types[1] ?? $combined_atomic_types[0];

// check again, since the type changed
self::noFloatLikeToIntPrecisionLoss($parent, $left_type_part, $invalid_left_messages);
self::noFloatLikeToIntPrecisionLoss($parent, $right_type_part, $invalid_right_messages);
}
}

Expand Down Expand Up @@ -476,7 +486,9 @@ private static function analyzeOperands(
$statements_source->getSuppressedIssues(),
);
}
} elseif ($right_type_part instanceof TTemplateParam
}

if ($right_type_part instanceof TTemplateParam
&& !$right_type_part->as->isInt()
&& !$right_type_part->as->isFloat()
) {
Expand All @@ -491,6 +503,18 @@ private static function analyzeOperands(
}
}

if ($left_type_part instanceof TTemplateParam && !$left_type_part->as->isInt()) {
foreach ($left_type_part->as->getAtomicTypes() as $atomic) {
self::noFloatLikeToIntPrecisionLoss($parent, $atomic, $invalid_left_messages);
}
}

if ($right_type_part instanceof TTemplateParam && !$right_type_part->as->isInt()) {
foreach ($right_type_part->as->getAtomicTypes() as $atomic) {
self::noFloatLikeToIntPrecisionLoss($parent, $atomic, $invalid_right_messages);
}
}

return null;
}

Expand Down Expand Up @@ -711,19 +735,63 @@ private static function analyzeOperands(
}
}

$left_from_literal_string = false;
if ($left_type_part instanceof TLiteralString) {
if (preg_match('/^\-?\d+$/', $left_type_part->value)) {
$left_type_part = new TLiteralInt((int) $left_type_part->value);
} elseif (preg_match('/^\-?\d?\.\d+$/', $left_type_part->value)) {
$left_type_part = new TLiteralFloat((float) $left_type_part->value);
}
$left_from_literal_string = $left_type_part;
$left_type_part = self::literalStringToIntFloat($left_type_part);
}

$right_from_literal_string = false;
if ($right_type_part instanceof TLiteralString) {
if (preg_match('/^\-?\d+$/', $right_type_part->value)) {
$right_type_part = new TLiteralInt((int) $right_type_part->value);
} elseif (preg_match('/^\-?\d?\.\d+$/', $right_type_part->value)) {
$right_type_part = new TLiteralFloat((float) $right_type_part->value);
$right_from_literal_string = $right_type_part;
$right_type_part = self::literalStringToIntFloat($right_type_part);
}

if (($left_from_literal_string || $right_from_literal_string)
&& ($left_type_part instanceof TLiteralInt || $left_type_part instanceof TLiteralFloat)
&& ($right_type_part instanceof TLiteralInt || $right_type_part instanceof TLiteralFloat)
&& (
//we don't try to do arithmetics on variables in loops
$context === null
|| $context->inside_loop === false
|| (!$left instanceof PhpParser\Node\Expr\Variable && !$right instanceof PhpParser\Node\Expr\Variable)
)
) {
// get_class is fine here because both classes are final.
if ($statements_source !== null
&& $config->strict_binary_operands
&& (!$left_from_literal_string
|| !$right_from_literal_string
|| get_class($left_from_literal_string) !== get_class($right_from_literal_string))
) {
IssueBuffer::maybeAdd(
new InvalidOperand(
'Cannot process numeric types together in strict operands mode, '.
'please cast explicitly',
new CodeLocation($statements_source, $parent),
),
$statements_source->getSuppressedIssues(),
);
}

// time for some arithmetic!
$calculated_type = self::arithmeticOperation(
$parent,
$left_type_part->value,
$right_type_part->value,
true,
);

if ($calculated_type) {
$result_type = Type::combineUnionTypes(
$calculated_type,
$result_type,
);

$has_valid_left_operand = $implicit_left;
$has_valid_right_operand = $implicit_right;

return null;
}
}

Expand Down Expand Up @@ -752,8 +820,8 @@ private static function analyzeOperands(

$result_type = Type::combineUnionTypes($new_result_type, $result_type);

$has_valid_right_operand = true;
$has_valid_left_operand = true;
$has_valid_right_operand = $implicit_right;
$has_valid_left_operand = $implicit_left;

return null;
}
Expand Down Expand Up @@ -847,8 +915,8 @@ private static function analyzeOperands(
$result_type = Type::combineUnionTypes(Type::getFloat(), $result_type);
}

$has_valid_right_operand = true;
$has_valid_left_operand = true;
$has_valid_right_operand = $implicit_right;
$has_valid_left_operand = $implicit_left;

return null;
}
Expand All @@ -875,8 +943,8 @@ private static function analyzeOperands(
$result_type = Type::combineUnionTypes(Type::getFloat(), $result_type);
}

$has_valid_right_operand = true;
$has_valid_left_operand = true;
$has_valid_right_operand = $implicit_right;
$has_valid_left_operand = $implicit_left;

return null;
}
Expand All @@ -901,20 +969,20 @@ private static function analyzeOperands(
$result_type = new Union([new TInt, new TFloat]);
}

$has_valid_right_operand = true;
$has_valid_left_operand = true;
$has_valid_right_operand = $implicit_right;
$has_valid_left_operand = $implicit_left;

return null;
}

if (!$left_type_part->isNumericType()) {
$invalid_left_messages[] = 'Cannot perform a numeric operation with a non-numeric type '
. $left_type_part;
$has_valid_right_operand = true;
$has_valid_right_operand = $implicit_right;
} else {
$invalid_right_messages[] = 'Cannot perform a numeric operation with a non-numeric type '
. $right_type_part;
$has_valid_left_operand = true;
$has_valid_left_operand = $implicit_left;
}
} else {
$invalid_left_messages[] =
Expand Down Expand Up @@ -944,21 +1012,21 @@ public static function arithmeticOperation(
return Type::getNever();
}

$result = $operand1 % $operand2;
$result = (int) $operand1 % (int) $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\Mul) {
$result = $operand1 * $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\Pow) {
$result = $operand1 ** $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\BitwiseOr) {
$result = $operand1 | $operand2;
$result = (int) $operand1 | (int) $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\BitwiseAnd) {
$result = $operand1 & $operand2;
$result = (int) $operand1 & (int) $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\BitwiseXor) {
$result = $operand1 ^ $operand2;
$result = (int) $operand1 ^ (int) $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\ShiftLeft) {
$result = $operand1 << $operand2;
$result = (int) $operand1 << (int) $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\ShiftRight) {
$result = $operand1 >> $operand2;
$result = (int) $operand1 >> (int) $operand2;
} elseif ($operation instanceof PhpParser\Node\Expr\BinaryOp\Div) {
if ($operand2 === 0 || $operand2 === 0.0) {
return Type::getNever();
Expand Down Expand Up @@ -1413,4 +1481,58 @@ private static function analyzeModBetweenIntRange(
$result_type,
);
}

/** @return TLiteralInt|TLiteralFloat|TLiteralString */
private static function literalStringToIntFloat(
TLiteralString $literal_atomic
) {
$int = filter_var($literal_atomic->value, FILTER_VALIDATE_INT);
if ($int !== false) {
return new TLiteralInt($int);
}

$float = filter_var($literal_atomic->value, FILTER_VALIDATE_FLOAT);
if ($float !== false) {
return new TLiteralFloat($float);
}

return $literal_atomic;
}

/**
* @param string[] $invalid_messages
*/
public static function noFloatLikeToIntPrecisionLoss(
PhpParser\Node $parent,
Atomic $type_part,
array &$invalid_messages
): bool {
if ($type_part instanceof TLiteralString) {
$type_part = self::literalStringToIntFloat($type_part);
}

if ($type_part instanceof TLiteralFloat
&& (int) $type_part->value === filter_var($type_part->value, FILTER_VALIDATE_INT)
) {
// if we don't have precision loss, e.g. 2.0
return true;
}

if (($type_part instanceof TFloat
|| $type_part instanceof TNumeric
|| $type_part instanceof TNumericString)
&& ($parent instanceof PhpParser\Node\Expr\BinaryOp\Mod
|| $parent instanceof PhpParser\Node\Expr\BinaryOp\BitwiseOr
|| $parent instanceof PhpParser\Node\Expr\BinaryOp\BitwiseAnd
|| $parent instanceof PhpParser\Node\Expr\BinaryOp\BitwiseXor
|| $parent instanceof PhpParser\Node\Expr\BinaryOp\ShiftLeft
|| $parent instanceof PhpParser\Node\Expr\BinaryOp\ShiftRight)
) {
// deprecated since PHP 8, will trigger a notice
$invalid_messages[] = 'Implicit conversion from float to int';
return false;
}

return true;
}
}
19 changes: 18 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/BitwiseNotAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
use Psalm\Type\Atomic\TString;
use Psalm\Type\Union;

use function filter_var;

use const FILTER_VALIDATE_INT;

/**
* @internal
*/
Expand Down Expand Up @@ -56,8 +60,21 @@ public static function analyze(
$acceptable_types[] = $type_part;
$has_valid_operand = true;
} elseif ($type_part instanceof TFloat) {
if (!$type_part instanceof TLiteralFloat
|| (int) $type_part->value !== filter_var($type_part->value, FILTER_VALIDATE_INT)
) {
// deprecated since PHP 8, will trigger a notice
IssueBuffer::maybeAdd(
new InvalidOperand(
'Implicit conversion from float to int',
new CodeLocation($statements_analyzer, $stmt->expr),
),
$statements_analyzer->getSuppressedIssues(),
);
}

$type_part = ($type_part instanceof TLiteralFloat) ?
new TLiteralInt(~$type_part->value) :
new TLiteralInt(~((int) $type_part->value)) :
new TInt;

$stmt_expr_type->removeType($type_string);
Expand Down
Loading

0 comments on commit 9784367

Please sign in to comment.