Skip to content

Do not assign ErrorType back to array when assign-op result is an error #5476

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
phpstan-bot wants to merge 1 commit into
base: 2.1.x
Choose a base branch
from
+261 −1
Open

Do not assign ErrorType back to array when assign-op result is an error #5476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 44 additions & 1 deletion src/Analyser/ExprHandler/AssignHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,12 @@ public function processAssignVar(
$assignedExpr = $this->unwrapAssign($assignedExpr);
$type = $scopeBeforeAssignEval->getType($assignedExpr);

$nativeType = $scope->getNativeType($assignedExpr);
if ($isAssignOp && $type instanceof ErrorType) {
$type = $scopeBeforeAssignEval->getType($var);
$nativeType = $scopeBeforeAssignEval->getNativeType($var);
}

$conditionalExpressions = [];
if ($assignedExpr instanceof Ternary) {
$if = $assignedExpr->if;
Expand Down Expand Up @@ -314,7 +320,7 @@ public function processAssignVar(
}

$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage);
$scope = $scope->assignVariable($var->name, $type, $scope->getNativeType($assignedExpr), TrinaryLogic::createYes());
$scope = $scope->assignVariable($var->name, $type, $nativeType, TrinaryLogic::createYes());
foreach ($conditionalExpressions as $exprString => $holders) {
$scope = $scope->addConditionalExpressions($exprString, $holders);
}
Expand Down Expand Up @@ -414,6 +420,28 @@ public function processAssignVar(

$valueToWrite = $scope->getType($assignedExpr);
$nativeValueToWrite = $scope->getNativeType($assignedExpr);

if ($isAssignOp && $valueToWrite instanceof ErrorType) {
$originalType = $scope->getType($var);
foreach ($offsetTypes as [$offsetType]) {
if ($offsetType === null) {
break;
}
$originalType = $originalType->getOffsetValueType($offsetType);
}
$valueToWrite = $originalType;
}
if ($isAssignOp && $nativeValueToWrite instanceof ErrorType) {
$originalNativeType = $scope->getNativeType($var);
foreach ($offsetNativeTypes as [$offsetNativeType]) {
if ($offsetNativeType === null) {
break;
}
$originalNativeType = $originalNativeType->getOffsetValueType($offsetNativeType);
}
$nativeValueToWrite = $originalNativeType;
}

$scopeBeforeAssignEval = $scope;

// 3. eval assigned expr
Expand Down Expand Up @@ -781,6 +809,21 @@ public function processAssignVar(
$varType = $scope->getType($var);
$varNativeType = $scope->getNativeType($var);

if ($isAssignOp && $valueToWrite instanceof ErrorType) {
$originalType = $varType;
foreach ($offsetTypes as [$offsetType]) {
$originalType = $originalType->getOffsetValueType($offsetType);
}
$valueToWrite = $originalType;
}
if ($isAssignOp && $nativeValueToWrite instanceof ErrorType) {
$originalNativeType = $varNativeType;
foreach ($offsetNativeTypes as [$offsetNativeType]) {
$originalNativeType = $originalNativeType->getOffsetValueType($offsetNativeType);
}
$nativeValueToWrite = $originalNativeType;
}

$offsetValueType = $varType;
$offsetNativeValueType = $varNativeType;
$offsetValueTypeStack = [$offsetValueType];
Expand Down
34 changes: 34 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-10349.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php declare(strict_types = 1);

namespace Bug10349Nsrt;

use function PHPStan\Testing\assertType;

class Foo
{
/**
* @param array<string, array<string, bool|float|int|string>> $expected
*/
public function testTypePreservationAfterErrorAssignOp(array $expected, int $ptr, string $key): void
{
assertType('array<string, array<string, bool|float|int|string>>', $expected);

$expected[$key]['number-1'] += $ptr;

// After += with ErrorType result, the array type should be preserved
assertType('bool|float|int|string', $expected[$key]['number-2']);
}

/**
* @param array<string, bool|float|int|string> $arr
*/
public function testSimpleArrayTypePreservation(array $arr, int $ptr): void
{
assertType('bool|float|int|string', $arr['a']);

$arr['a'] += $ptr;

// After += with ErrorType result, sibling keys should keep their type
assertType('bool|float|int|string', $arr['b']);
}
}
82 changes: 82 additions & 0 deletions tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,4 +835,86 @@ public function testBug14080(): void
$this->analyse([__DIR__ . '/data/bug-14080.php'], []);
}

public function testBug10349(): void
{
$this->analyse([__DIR__ . '/data/bug-10349.php'], [
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
15,
],
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
20,
],
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
37,
],
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
47,
],
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
49,
],
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
57,
],
[
'Binary operation "+=" between bool|float|int|string and int results in an error.',
59,
],
[
'Binary operation "-=" between bool|float|int|string and int results in an error.',
67,
],
[
'Binary operation "-=" between bool|float|int|string and int results in an error.',
68,
],
[
'Binary operation "*=" between bool|float|int|string and int results in an error.',
69,
],
[
'Binary operation "*=" between bool|float|int|string and int results in an error.',
70,
],
[
'Binary operation ".=" between array<int>|int and \'foo\' results in an error.',
78,
],
[
'Binary operation ".=" between array<int>|int and \'foo\' results in an error.',
79,
],
[
'Binary operation "/=" between bool|float|int|string and int results in an error.',
87,
],
[
'Binary operation "/=" between bool|float|int|string and int results in an error.',
88,
],
[
'Binary operation "%=" between bool|float|int|string and int results in an error.',
89,
],
[
'Binary operation "%=" between bool|float|int|string and int results in an error.',
90,
],
[
'Binary operation "<<=" between bool|float|int|string and int results in an error.',
98,
],
[
'Binary operation "<<=" between bool|float|int|string and int results in an error.',
99,
],
]);
}

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

namespace Bug10349;

class Foo
{
/**
* @param array<string, array<string, bool|float|int|string>> $expected
*/
public function issue1A(array $expected, int $ptr): void
{
foreach ($expected as $key => $param) {
if ($param['number-1'] !== false) {
// This gets flagged
$expected[$key]['number-1'] += $ptr;
}

if ($param['number-2'] !== false) {
// This should also get flagged but doesn't
$expected[$key]['number-2'] += $ptr;
}
}
}

/**
* @param array<string, array<string, bool|float|int|string>> $expected
*/
public function issue1B(array $expected, int $ptr): void
{
foreach ($expected as $key => $param) {
if (is_int($expected[$key]['number-1'])) {
$expected[$key]['number-1'] += $ptr;
}

// Even after fixing the first, the second one still doesn't get flagged
if ($param['number-2'] !== false) {
$expected[$key]['number-2'] += $ptr;
}
}
}

/**
* @param array<string, array<string, bool|float|int|string>> $expected
*/
public function multipleOpsNoLoop(array $expected, int $ptr, string $key): void
{
$expected[$key]['number-1'] += $ptr;
// After the first += corrupts the array type, this should still be flagged
$expected[$key]['number-2'] += $ptr;
}

/**
* @param array<string, bool|float|int|string> $arr
*/
public function simpleArray(array $arr, int $ptr): void
{
$arr['a'] += $ptr;
// After the first += corrupts the array type, this should still be flagged
$arr['b'] += $ptr;
}

/**
* @param array<string, bool|float|int|string> $arr
*/
public function otherAssignOps(array $arr, int $ptr): void
{
$arr['a'] -= $ptr;
$arr['b'] -= $ptr;
$arr['c'] *= $ptr;
$arr['d'] *= $ptr;
}

/**
* @param array<string, array<int>|int> $arr
*/
public function concatAssignOps(array $arr): void
{
$arr['a'] .= 'foo';
$arr['b'] .= 'foo';
}

/**
* @param array<string, bool|float|int|string> $arr
*/
public function divAndModAssignOps(array $arr, int $ptr): void
{
$arr['a'] /= $ptr;
$arr['b'] /= $ptr;
$arr['c'] %= $ptr;
$arr['d'] %= $ptr;
}

/**
* @param array<string, bool|float|int|string> $arr
*/
public function bitwiseAssignOps(array $arr, int $ptr): void
{
$arr['a'] <<= $ptr;
$arr['b'] <<= $ptr;
}
}
Loading