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

CI should fail on PHP deprecated errors and report errors for deprecated implicit float to int precision loss #10958

Draft
wants to merge 5 commits into
base: 5.x
Choose a base branch
from
Draft

CI should fail on PHP deprecated errors and report errors for deprecated implicit float to int precision loss #10958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f0af63
CI should fail on PHP deprecated errors
kkmuffme May 5, 2024
a6f3a4f
set php.ini error reporting correctly
kkmuffme May 5, 2024
7d7ca29
Fix PHP notices and add error for implicit float to int precision loss
kkmuffme May 5, 2024
6e60f62
CI: use newest PHP version where possible
kkmuffme May 5, 2024
896e115
Run CI with lowest supported PHP version too
kkmuffme May 5, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/bcc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: '8.1'
ini-values: zend.assertions=1, assert.exception=1, opcache.enable_cli=1, opcache.jit=function, opcache.jit_buffer_size=512M, error_reporting=E_ALL
tools: composer:v2
coverage: none
env:
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ jobs:
fail-fast: false
matrix:
php-version:
- "7.4"
- "8.0"
- "8.1"
- "8.2"
Expand All @@ -143,7 +144,7 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: "${{ matrix.php-version }}"
ini-values: zend.assertions=1, assert.exception=1, opcache.enable_cli=1, opcache.jit=function, opcache.jit_buffer_size=512M
ini-values: zend.assertions=1, assert.exception=1, opcache.enable_cli=1, opcache.jit=function, opcache.jit_buffer_size=512M, error_reporting=E_ALL
tools: composer:v2
coverage: none
extensions: none, curl, dom, filter, intl, json, libxml, mbstring, opcache, openssl, pcre, phar, reflection, simplexml, spl, tokenizer, xml, xmlwriter
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/shepherd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ jobs:
- uses: actions/checkout@v4
- uses: shivammathur/setup-php@v2
with:
php-version: '8.2'
ini-values: zend.assertions=1
php-version: '8.3'
ini-values: zend.assertions=1, error_reporting=E_ALL
tools: composer:v2
coverage: none
env:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/windows-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ jobs:
- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.0'
ini-values: zend.assertions=1, assert.exception=1, opcache.enable_cli=1, opcache.jit=function, opcache.jit_buffer_size=512M
php-version: '7.4'
ini-values: zend.assertions=1, assert.exception=1, opcache.enable_cli=1, opcache.jit=function, opcache.jit_buffer_size=512M, error_reporting=E_ALL
tools: composer:v2
coverage: none
extensions: none, curl, dom, filter, intl, json, libxml, mbstring, openssl, opcache, pcre, phar, reflection, simplexml, spl, tokenizer, xml, xmlwriter
Expand Down
1 change: 1 addition & 0 deletions phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
beStrictAboutTestsThatDoNotTestAnything="false"
beStrictAboutTodoAnnotatedTests="true"
colors="true"
convertDeprecationsToExceptions="true"
verbose="false"
executionOrder="random"
>
Expand Down
180 changes: 151 additions & 29 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php
View file
Open in desktop
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
Open in desktop
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
Loading