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
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
@@ -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
30 changes: 29 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
@@ -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;
@@ -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;
}
6 changes: 1 addition & 5 deletions src/Type/Accessory/AccessoryArrayListType.php
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions src/Type/Accessory/HasOffsetValueType.php
Original file line number Diff line number Diff line change
@@ -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);
}

9 changes: 1 addition & 8 deletions src/Type/Constant/ConstantArrayType.php
Original file line number Diff line number Diff line change
@@ -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();
}
4 changes: 4 additions & 0 deletions src/Type/IntersectionType.php
Original file line number Diff line number Diff line change
@@ -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;
}

1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
@@ -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';
}

/**
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/nsrt/bug-12274.php
Original file line number Diff line number Diff line change
@@ -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);
}
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
@@ -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) {
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
@@ -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
@@ -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>
@@ -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,
],
[
@@ -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
@@ -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