Skip to content

Keep list on unset() with nested dim-fetch #3964

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.

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
wants to merge 21 commits into
base: 2.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ parameters:
count: 2
path: src/Analyser/NodeScopeResolver.php

-
message: '#^Doing instanceof PHPStan\\Type\\Constant\\ConstantStringType is error\-prone and deprecated\. Use Type\:\:getConstantStrings\(\) instead\.$#'
identifier: phpstanApi.instanceofType
count: 1
path: src/Analyser/NodeScopeResolver.php

-
message: '#^Parameter \#2 \$node of method PHPStan\\BetterReflection\\SourceLocator\\Ast\\Strategy\\NodeToReflection\:\:__invoke\(\) expects PhpParser\\Node\\Expr\\ArrowFunction\|PhpParser\\Node\\Expr\\Closure\|PhpParser\\Node\\Expr\\FuncCall\|PhpParser\\Node\\Stmt\\Class_\|PhpParser\\Node\\Stmt\\Const_\|PhpParser\\Node\\Stmt\\Enum_\|PhpParser\\Node\\Stmt\\Function_\|PhpParser\\Node\\Stmt\\Interface_\|PhpParser\\Node\\Stmt\\Trait_, PhpParser\\Node\\Stmt\\ClassLike given\.$#'
identifier: argument.type
Expand Down
30 changes: 29 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,15 @@
use PHPStan\ShouldNotHappenException;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Accessory\AccessoryArrayListType;
use PHPStan\Type\Accessory\HasOffsetValueType;
use PHPStan\Type\Accessory\NonEmptyArrayType;
use PHPStan\Type\ArrayType;
use PHPStan\Type\ClosureType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\FileTypeMapper;
use PHPStan\Type\GeneralizePrecision;
Expand Down Expand Up @@ -5924,9 +5926,35 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
}
$offsetValueType = TypeCombinator::intersect($offsetValueType, TypeCombinator::union(...$types));
}
$valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $i === 0);

$arrayDimFetch = $dimFetchStack[$i] ?? null;
if (
$offsetType !== null
&& $arrayDimFetch !== null
&& $scope->hasExpressionType($arrayDimFetch)->yes()
) {
$hasOffsetType = null;
if ($offsetType instanceof ConstantStringType || $offsetType instanceof ConstantIntegerType) {
$hasOffsetType = new HasOffsetValueType($offsetType, $valueToWrite);
}
$valueToWrite = $offsetValueType->setExistingOffsetValueType($offsetType, $valueToWrite);

if ($hasOffsetType !== null) {
$valueToWrite = TypeCombinator::intersect(
$valueToWrite,
$hasOffsetType,
);
} elseif ($valueToWrite->isArray()->yes()) {
$valueToWrite = TypeCombinator::intersect(
$valueToWrite,
new NonEmptyArrayType(),
);
}

} else {
$valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $i === 0);
}

if ($arrayDimFetch === null || !$offsetValueType->isList()->yes()) {
continue;
}
Expand Down
6 changes: 1 addition & 5 deletions src/Type/Accessory/AccessoryArrayListType.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,7 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $uni

public function setExistingOffsetValueType(Type $offsetType, Type $valueType): Type
{
if ((new ConstantIntegerType(0))->isSuperTypeOf($offsetType)->yes()) {
return $this;
}

return new ErrorType();
return $this;
}

public function unsetOffset(Type $offsetType): Type
Expand Down
4 changes: 4 additions & 0 deletions src/Type/Accessory/HasOffsetValueType.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $uni

public function setExistingOffsetValueType(Type $offsetType, Type $valueType): Type
{
if (!$offsetType->equals($this->offsetType)) {
return $this;
}

return new self($this->offsetType, $valueType);
}

Expand Down
9 changes: 1 addition & 8 deletions src/Type/Constant/ConstantArrayType.php
Original file line number Diff line number Diff line change
Expand Up @@ -692,15 +692,8 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $uni

public function setExistingOffsetValueType(Type $offsetType, Type $valueType): Type
{
$offsetType = $offsetType->toArrayKey();
$builder = ConstantArrayTypeBuilder::createFromConstantArray($this);
foreach ($this->keyTypes as $keyType) {
if ($offsetType->isSuperTypeOf($keyType)->no()) {
continue;
}

$builder->setOffsetValueType($keyType, $valueType);
}
$builder->setOffsetValueType($offsetType, $valueType);

return $builder->getArray();
}
Expand Down
4 changes: 4 additions & 0 deletions src/Type/IntersectionType.php
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $uni
}
}

if ($this->isList()->yes() && $this->getIterableValueType()->isArray()->yes()) {
$result = TypeCombinator::intersect($result, new AccessoryArrayListType());
}

return $result;
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ private static function findTestFiles(): iterable
yield __DIR__ . '/../Rules/Arrays/data/bug-11679.php';
yield __DIR__ . '/../Rules/Methods/data/bug-4801.php';
yield __DIR__ . '/../Rules/Arrays/data/narrow-superglobal.php';
yield __DIR__ . '/../Rules/Methods/data/bug-12927.php';
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/nsrt/bug-12274.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ function testKeepNestedListAfterIssetIndex(array $nestedList, int $i, int $j): v
assertType('list<list<int>>', $nestedList);
assertType('list<int>', $nestedList[$i]);
$nestedList[$i][$j] = 21;
assertType('non-empty-list<non-empty-list<int>>', $nestedList);
assertType('non-empty-list<int>', $nestedList[$i]);
assertType('non-empty-list<list<int>>', $nestedList);
assertType('list<int>', $nestedList[$i]);
}
assertType('list<list<int>>', $nestedList);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,11 @@ public function testBug1O580(): void
]);
}

public function testBug12927(): void
{
$this->analyse([__DIR__ . '/data/bug-12927.php'], []);
}

public function testBug4443(): void
{
if (PHP_VERSION_ID < 80000) {
Expand Down
63 changes: 63 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-12927.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

namespace Bug12927;

use function PHPStan\Testing\assertType;

class HelloWorld
{
/**
* @param list<array{abc: string}> $list
* @return list<array<string>>
*/
public function sayHello(array $list): array
{
foreach($list as $k => $v) {
unset($list[$k]['abc']);
assertType('non-empty-list<array{}|array{abc: string}>', $list);
assertType('array{}|array{abc: string}', $list[$k]);
}
return $list;
}

/**
* @param list<array<string, string>> $list
*/
public function sayFoo(array $list): void
{
foreach($list as $k => $v) {
unset($list[$k]['abc']);
assertType('non-empty-list<array<string, string>>', $list);
assertType('array<string, string>', $list[$k]);
}
assertType('list<array<string, string>>', $list);
}

/**
* @param list<array<string, string>> $list
*/
public function sayFoo2(array $list): void
{
foreach($list as $k => $v) {
$list[$k]['abc'] = 'world';
assertType("non-empty-list<non-empty-array<string, string>&hasOffsetValue('abc', 'world')>", $list);
assertType("non-empty-array<string, string>&hasOffsetValue('abc', 'world')", $list[$k]);
}
assertType("list<non-empty-array<string, string>&hasOffsetValue('abc', 'world')>", $list);
}

/**
* @param list<array<string, string>> $list
*/
public function sayFooBar(array $list): void
{
foreach($list as $k => $v) {
if (rand(0,1)) {
unset($list[$k]);
}
assertType('array<int<0, max>, array<string, string>>', $list);
assertType('array<string, string>', $list[$k]);
}
assertType('array<string, string>', $list[$k]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -779,4 +779,20 @@ public function testPropertyHooks(): void
]);
}

public function testBug11171(): void
{
$this->checkExplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-11171.php'], []);
}

public function testBug8282(): void
{
if (PHP_VERSION_ID < 80000) {
$this->markTestSkipped('Test requires PHP 8.0.');
}

$this->checkExplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-8282.php'], []);
}

}
41 changes: 41 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-11171.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Bug11171;

class TypeExpression
{
public string $value;

/**
* @var list<array{start_index: int, expression: self}>
*/
public array $innerTypeExpressions = [];

/**
* @param \Closure(self): void $callback
*/
public function walkTypes(\Closure $callback): void
{
$startIndexOffset = 0;

foreach ($this->innerTypeExpressions as $k => ['start_index' => $startIndexOrig,
'expression' => $inner,]) {
$this->innerTypeExpressions[$k]['start_index'] += $startIndexOffset;

$innerLengthOrig = \strlen($inner->value);

$inner->walkTypes($callback);

$this->value = substr_replace(
$this->value,
$inner->value,
$startIndexOrig + $startIndexOffset,
$innerLengthOrig
);

$startIndexOffset += \strlen($inner->value) - $innerLengthOrig;
}

$callback($this);
}
}
31 changes: 31 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-8282.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php // lint >= 8.0

namespace Bug8282;

/**
* @phpstan-type record array{id: positive-int, name: string}
*/
class Collection
{
/** @param list<record> $list */
public function __construct(
public array $list
)
{
}

public function updateName(int $index, string $name): void
{
assert(isset($this->list[$index]));
$this->list[$index]['name'] = $name;
}

public function updateNameById(int $id, string $name): void
{
foreach ($this->list as $index => $entry) {
if ($entry['id'] === $id) {
$this->list[$index]['name'] = $name;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PHPStan\Rules\Rule as TRule;
use PHPStan\Rules\RuleLevelHelper;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<ParameterOutAssignedTypeRule>
Expand Down Expand Up @@ -43,7 +44,7 @@ public function testRule(): void
47,
],
[
'Parameter &$p @param-out type of method ParameterOutAssignedType\Foo::doBaz3() expects list<list<int>>, array<int<0, max>, array<int<0, max>, int>> given.',
'Parameter &$p @param-out type of method ParameterOutAssignedType\Foo::doBaz3() expects list<list<int>>, list<array<int<0, max>, int>> given.',
56,
],
[
Expand All @@ -64,4 +65,12 @@ public function testBenevolentArrayKey(): void
$this->analyse([__DIR__ . '/data/benevolent-array-key.php'], []);
}

public function testBug12754(): void
{
if (PHP_VERSION_ID < 80000) {
$this->markTestSkipped('PHP 8.0+ is required for this test.');
}
$this->analyse([__DIR__ . '/data/bug-12754.php'], []);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,9 @@ public function testBug11363(): void
$this->analyse([__DIR__ . '/data/bug-11363.php'], []);
}

public function testBug12330(): void
{
$this->analyse([__DIR__ . '/data/bug-12330.php'], []);
}

}
25 changes: 25 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-12330.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace Bug12330;

/**
* @param array{items: list<array<string, mixed>>} $options
* @param-out array{items: list<array<string, mixed>>} $options
*/
function alterItems(array &$options): void
{
foreach ($options['items'] as $i => $item) {
$options['items'][$i]['options']['title'] = $item['name'];
}
}

/**
* @param array{items: array<int, array<string, mixed>>} $options
* @param-out array{items: array<int, array<string, mixed>>} $options
*/
function alterItems2(array &$options): void
{
foreach ($options['items'] as $i => $item) {
$options['items'][$i]['options']['title'] = $item['name'];
}
}
Loading
Loading