Skip to content

Commit

Permalink
Narrow type on setting offsets of properties
Browse files Browse the repository at this point in the history
  • Loading branch information
herndlm committed Jan 4, 2025
1 parent b614f70 commit 1f9f463
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 65 deletions.
10 changes: 0 additions & 10 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
use PHPStan\Node\Expr\GetIterableValueTypeExpr;
use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
use PHPStan\Node\Expr\PropertyInitializationExpr;
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
Expand Down Expand Up @@ -687,15 +686,6 @@ public function getType(Expr $node): Type
return $node->getExprType();
}

if ($node instanceof OriginalPropertyTypeExpr) {
$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($node->getPropertyFetch(), $this);
if ($propertyReflection === null) {
return new ErrorType();
}

return $propertyReflection->getReadableType();
}

$key = $this->getNodeKey($node);

if (!array_key_exists($key, $this->resolvedTypes)) {
Expand Down
13 changes: 2 additions & 11 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
use PHPStan\Node\Expr\GetIterableValueTypeExpr;
use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
use PHPStan\Node\Expr\PropertyInitializationExpr;
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
Expand Down Expand Up @@ -5346,12 +5345,8 @@ private function processAssignVar(
$originalVar = $var;
$assignedPropertyExpr = $assignedExpr;
while ($var instanceof ArrayDimFetch) {
$varForSetOffsetValue = $var->var;
if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) {
$varForSetOffsetValue = new OriginalPropertyTypeExpr($varForSetOffsetValue);
}
$assignedPropertyExpr = new SetOffsetValueTypeExpr(
$varForSetOffsetValue,
$var->var,
$var->dim,
$assignedPropertyExpr,
);
Expand Down Expand Up @@ -5673,12 +5668,8 @@ static function (): void {
$dimFetchStack = [];
$assignedPropertyExpr = $assignedExpr;
while ($var instanceof ExistingArrayDimFetch) {
$varForSetOffsetValue = $var->getVar();
if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) {
$varForSetOffsetValue = new OriginalPropertyTypeExpr($varForSetOffsetValue);
}
$assignedPropertyExpr = new SetExistingOffsetValueTypeExpr(
$varForSetOffsetValue,
$var->getVar(),
$var->getDim(),
$assignedPropertyExpr,
);
Expand Down
6 changes: 0 additions & 6 deletions src/Node/Printer/Printer.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
use PHPStan\Node\Expr\GetIterableValueTypeExpr;
use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
use PHPStan\Node\Expr\PropertyInitializationExpr;
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
Expand Down Expand Up @@ -55,11 +54,6 @@ protected function pPHPStan_Node_ExistingArrayDimFetch(ExistingArrayDimFetch $ex
return sprintf('__phpstanExistingArrayDimFetch(%s, %s)', $this->p($expr->getVar()), $this->p($expr->getDim()));
}

protected function pPHPStan_Node_OriginalPropertyTypeExpr(OriginalPropertyTypeExpr $expr): string // phpcs:ignore
{
return sprintf('__phpstanOriginalPropertyType(%s)', $this->p($expr->getPropertyFetch()));
}

protected function pPHPStan_Node_SetOffsetValueTypeExpr(SetOffsetValueTypeExpr $expr): string // phpcs:ignore
{
return sprintf('__phpstanSetOffsetValueType(%s, %s, %s)', $this->p($expr->getVar()), $expr->getDim() !== null ? $this->p($expr->getDim()) : 'null', $this->p($expr->getValue()));
Expand Down
20 changes: 16 additions & 4 deletions tests/PHPStan/Rules/Arrays/data/appended-array-item.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,28 @@ class Foo
/** @var callable[] */
private $callables;

public function doFoo()
public function accepted()
{
$this->integers[] = 4;
$this->integers['foo'] = 5;
$this->integers[] = 'foo';
$this->callables[] = [$this, 'doFoo'];
$this->callables[] = [1, 2, 3];
$this->callables[] = ['Closure', 'bind'];
$this->callables[] = 'strpos';
$this->callables[] = [$this, 'accepted'];
}

public function notAccepted1()
{
$this->integers[] = 'foo';
$this->callables[] = [1, 2, 3];
}

public function notAccepted2()
{
$this->callables[] = [__CLASS__, 'classMethod'];
}

public function notAccepted3()
{
$world = 'world';
$this->callables[] = ['Foo', "Hello $world"];

Expand Down
32 changes: 19 additions & 13 deletions tests/PHPStan/Rules/Arrays/data/appended-array-key.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,34 @@ class Foo
/** @var array<int|string, mixed> */
private $bothArray;

/**
* @param int|string $intOrString
*/
public function doFoo(
int $int,
string $string,
$intOrString,
?string $stringOrNull
)
public function invalid()
{
$this->intArray[new \DateTimeImmutable()] = 1;
$this->intArray[$intOrString] = 1;
$this->intArray[$int] = 1;
}

/** @param int|string $intOrString */
public function notAccepted(int $int, string $string, $intOrString)
{
$this->intArray[$string] = 1;
$this->intArray[$intOrString] = 1;
$this->stringArray[$int] = 1;
$this->stringArray[$string] = 1;
$this->stringArray[$intOrString] = 1;
}

public function notAcceptedStringIntCase()
{
$this->stringArray['0'] = 1;
}

/** @param int|string $intOrString */
public function accepted(int $int, string $string, $intOrString, ?string $stringOrNull)
{
$this->intArray[$int] = 1;
$this->stringArray[$string] = 1;
$this->bothArray[$int] = 1;
$this->bothArray[$intOrString] = 1;
$this->bothArray[$string] = 1;
$this->stringArray[$stringOrNull] = 1; // will be cast to string
$this->stringArray['0'] = 1;
}

public function checkRewrittenArray()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,35 +253,35 @@ public function testAppendendArrayKey(): void
$this->analyse([__DIR__ . '/../Arrays/data/appended-array-key.php'], [
[
'Property AppendedArrayKey\Foo::$intArray (array<int, mixed>) does not accept array<int|string, mixed>.',
28,
25,
],
[
'Property AppendedArrayKey\Foo::$intArray (array<int, mixed>) does not accept array<int|string, mixed>.',
30,
26,
],
[
'Property AppendedArrayKey\Foo::$stringArray (array<string, mixed>) does not accept array<int|string, mixed>.',
31,
27,
],
[
'Property AppendedArrayKey\Foo::$stringArray (array<string, mixed>) does not accept array<int|string, mixed>.',
33,
28,
],
[
'Property AppendedArrayKey\Foo::$stringArray (array<string, mixed>) does not accept array<int|string, mixed>.',
38,
33,
],
[
'Property AppendedArrayKey\Foo::$stringArray (array<string, mixed>) does not accept array<int|string, mixed>.',
46,
'Property AppendedArrayKey\Foo::$stringArray (array<string, mixed>) does not accept array<int, false>.',
52,
],
[
'Property AppendedArrayKey\MorePreciseKey::$test (array<1|2|3, string>) does not accept non-empty-array<int, string>.',
80,
86,
],
[
'Property AppendedArrayKey\MorePreciseKey::$test (array<1|2|3, string>) does not accept non-empty-array<1|2|3|4, string>.',
85,
91,
],
]);
}
Expand All @@ -303,35 +303,35 @@ public function testAppendedArrayItemType(): void
[
[
'Property AppendedArrayItem\Foo::$integers (array<int>) does not accept array<int|string>.',
18,
25,
],
[
'Property AppendedArrayItem\Foo::$callables (array<callable(): mixed>) does not accept non-empty-array<array{1, 2, 3}|(callable(): mixed)>.',
20,
26,
],
[
'Property AppendedArrayItem\Foo::$callables (array<callable(): mixed>) does not accept non-empty-array<array{\'AppendedArrayItem\\\\Foo\', \'classMethod\'}|(callable(): mixed)>.',
23,
31,
],
[
'Property AppendedArrayItem\Foo::$callables (array<callable(): mixed>) does not accept non-empty-array<array{\'Foo\', \'Hello world\'}|(callable(): mixed)>.',
25,
37,
],
[
'Property AppendedArrayItem\Foo::$integers (array<int>) does not accept array<int|string>.',
27,
39,
],
[
'Property AppendedArrayItem\Foo::$integers (array<int>) does not accept array<int|string>.',
32,
44,
],
[
'Property AppendedArrayItem\Bar::$stringCallables (array<callable(): string>) does not accept non-empty-array<(callable(): string)|(Closure(): 1)>.',
45,
57,
],
[
'Property AppendedArrayItem\Baz::$staticProperty (array<AppendedArrayItem\Lorem>) does not accept array<AppendedArrayItem\Baz>.',
79,
91,
],
],
);
Expand All @@ -355,7 +355,7 @@ public function testBug6286(): void
{
$this->analyse([__DIR__ . '/data/bug-6286.php'], [
[
'Property Bug6286\HelloWorld::$details (array{name: string, age: int}) does not accept array{name: string, age: \'Forty-two\'}.',
'Property Bug6286\HelloWorld::$details (array{name: string, age: int}) does not accept array{name: \'Douglas Adams\', age: \'Forty-two\'}.',
19,
"Offset 'age' (int) does not accept type string.",
],
Expand Down Expand Up @@ -389,7 +389,7 @@ public function testBug3703(): void
18,
],
[
'Property Bug3703\Foo::$bar (array<string, array<string, array<int>>>) does not accept array<string, array<string, array<int>>|string>.',
'Property Bug3703\Foo::$bar (array<string, array<string, array<int>>>) does not accept array<string, array<string, array<int|string>|int>|string>.',
21,
],
]);
Expand Down Expand Up @@ -488,7 +488,7 @@ public function testBug6356b(): void
$this->checkExplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-6356b.php'], [
[
'Property Bug6356b\HelloWorld2::$details (array{name: string, age: int}) does not accept array{name: string, age: \'Forty-two\'}.',
'Property Bug6356b\HelloWorld2::$details (array{name: string, age: int}) does not accept array{name: \'Douglas Adams\', age: \'Forty-two\'}.',
19,
"Offset 'age' (int) does not accept type string.",
],
Expand All @@ -498,7 +498,7 @@ public function testBug6356b(): void
"Offset 'age' (int) does not accept type int|string.",
],
[
'Property Bug6356b\HelloWorld2::$nestedDetails (array<array{name: string, age: int}>) does not accept non-empty-array<array{name: string, age: \'Twelve\'|int}>.',
'Property Bug6356b\HelloWorld2::$nestedDetails (array<array{name: string, age: int}>) does not accept non-empty-array<array{name: string, age: \'Eleventy-one\'|\'Twelve\'|int}>.',
26,
"Offset 'age' (int) does not accept type int|string.",
],
Expand Down Expand Up @@ -688,6 +688,24 @@ public function testBug12131(): void
]);
}

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

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

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

public function testShortBodySetHook(): void
{
if (PHP_VERSION_ID < 80400) {
Expand Down
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-6398.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace Bug6398;

class AsyncTask{
/**
* @phpstan-var \ArrayObject<int, array<string, mixed>>|null
*/
private static $threadLocalStorage = null;

/**
* @param mixed $complexData the data to store
*/
protected function storeLocal(string $key, $complexData) : void{
if(self::$threadLocalStorage === null){
self::$threadLocalStorage = new \ArrayObject();
}
self::$threadLocalStorage[spl_object_id($this)][$key] = $complexData;
}

/**
* @return mixed
*/
protected function fetchLocal(string $key){
$id = spl_object_id($this);
if(self::$threadLocalStorage === null or !isset(self::$threadLocalStorage[$id][$key])){
throw new \InvalidArgumentException("No matching thread-local data found on this thread");
}

return self::$threadLocalStorage[$id][$key];
}
}
31 changes: 31 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-6571.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php declare(strict_types = 1); // lint >= 7.4

namespace Bug6571;

interface ClassLoader{}

class HelloWorld
{
/** @var \Threaded|\ClassLoader[]|null */
private ?\Threaded $classLoaders = null;

/**
* @param \ClassLoader[] $autoloaders
*/
public function setClassLoaders(?array $autoloaders = null) : void{
if($autoloaders === null){
$autoloaders = [];
}

if($this->classLoaders === null){
$this->classLoaders = new \Threaded();
}else{
foreach($this->classLoaders as $k => $autoloader){
unset($this->classLoaders[$k]);
}
}
foreach($autoloaders as $autoloader){
$this->classLoaders[] = $autoloader;
}
}
}
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-7880.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php declare(strict_types = 1);

namespace Bug7880;

class C {
/** @var array{a: string, b: string}|null */
private $a = null;

public function foo(): void {
if ($this->a !== null) {
$this->a['b'] = "baz";
}
}
}

0 comments on commit 1f9f463

Please sign in to comment.