-
Notifications
You must be signed in to change notification settings - Fork 582
Narrow array&callable intersection to list{class-string|object, string} #5563
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
Changes from 2 commits
64fbc7b
0f240cc
fa7b3c6
65daf7c
fae693f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1683,6 +1683,68 @@ public static function intersect(Type ...$types): Type | |
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| $types[$i] instanceof ArrayType | ||
| && get_class($types[$i]) === ArrayType::class | ||
| && $types[$j] instanceof CallableType | ||
| ) { | ||
| $existingValueType = $types[$i]->getItemType(); | ||
| $offset0ValueType = self::intersect($existingValueType, new UnionType([new ClassStringType(), new ObjectWithoutClassType()])); | ||
| $offset1ValueType = self::intersect($existingValueType, new StringType()); | ||
| if ($offset0ValueType instanceof NeverType || $offset1ValueType instanceof NeverType) { | ||
| return new NeverType(); | ||
| } | ||
| $types[$i] = new ConstantArrayType( | ||
| [new ConstantIntegerType(0), new ConstantIntegerType(1)], | ||
| [$offset0ValueType, $offset1ValueType], | ||
| [2], | ||
| isList: TrinaryLogic::createYes(), | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| $types[$j] instanceof ArrayType | ||
| && get_class($types[$j]) === ArrayType::class | ||
| && $types[$i] instanceof CallableType | ||
| ) { | ||
| $existingValueType = $types[$j]->getItemType(); | ||
| $offset0ValueType = self::intersect($existingValueType, new UnionType([new ClassStringType(), new ObjectWithoutClassType()])); | ||
| $offset1ValueType = self::intersect($existingValueType, new StringType()); | ||
| if ($offset0ValueType instanceof NeverType || $offset1ValueType instanceof NeverType) { | ||
| return new NeverType(); | ||
| } | ||
| $types[$j] = new ConstantArrayType( | ||
| [new ConstantIntegerType(0), new ConstantIntegerType(1)], | ||
| [$offset0ValueType, $offset1ValueType], | ||
| [2], | ||
| isList: TrinaryLogic::createYes(), | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| $types[$i] instanceof ConstantArrayType | ||
| && $types[$j] instanceof CallableType | ||
| ) { | ||
| $types[$i] = self::narrowConstantArrayWithCallable($types[$i]); | ||
| if ($types[$i] instanceof NeverType) { | ||
| return new NeverType(); | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| $types[$j] instanceof ConstantArrayType | ||
| && $types[$i] instanceof CallableType | ||
| ) { | ||
| $types[$j] = self::narrowConstantArrayWithCallable($types[$j]); | ||
| if ($types[$j] instanceof NeverType) { | ||
| return new NeverType(); | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -1779,4 +1841,33 @@ public static function removeTruthy(Type $type): Type | |
| return self::remove($type, StaticTypeFactory::truthy()); | ||
| } | ||
|
|
||
| private static function narrowConstantArrayWithCallable(ConstantArrayType $constantArray): Type | ||
| { | ||
| $keyTypes = $constantArray->getKeyTypes(); | ||
| $valueTypes = $constantArray->getValueTypes(); | ||
| $newValueTypes = $valueTypes; | ||
|
|
||
| foreach ($keyTypes as $k => $keyType) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we might want to immediately return NeverType when we have less than 2 key-types, so we don't create new ConstantArrayType for "too small/empty" constant arrays
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. The change adds an early |
||
| if ((new ConstantIntegerType(0))->isSuperTypeOf($keyType)->yes()) { | ||
| $newValueTypes[$k] = self::intersect($valueTypes[$k], new UnionType([new ClassStringType(), new ObjectWithoutClassType()])); | ||
| if ($newValueTypes[$k] instanceof NeverType) { | ||
| return new NeverType(); | ||
| } | ||
| } elseif ((new ConstantIntegerType(1))->isSuperTypeOf($keyType)->yes()) { | ||
| $newValueTypes[$k] = self::intersect($valueTypes[$k], new StringType()); | ||
| if ($newValueTypes[$k] instanceof NeverType) { | ||
| return new NeverType(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return new ConstantArrayType( | ||
| $keyTypes, | ||
| $newValueTypes, | ||
| $constantArray->getNextAutoIndexes(), | ||
| $constantArray->getOptionalKeys(), | ||
| $constantArray->isList(), | ||
| ); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14549; | ||
|
|
||
| use function PHPStan\Testing\assertType; | ||
|
|
||
| class Foo | ||
| { | ||
| public function foo(array $task): void | ||
| { | ||
| if (\is_callable($task)) { | ||
| assertType('list{class-string|object, string}&callable(): mixed', $task); | ||
| assertType('class-string|object', $task[0]); | ||
| assertType('string', $task[1]); | ||
|
|
||
| foreach ($task as $key => $value) { | ||
| assertType('object|string', $value); | ||
| assertType('0|1', $key); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public function testCallableArrayIterableTypes(callable $value): void | ||
| { | ||
| if (is_array($value)) { | ||
| assertType('list{class-string|object, string}&callable(): mixed', $value); | ||
|
|
||
| foreach ($value as $key => $val) { | ||
| assertType('0|1', $key); | ||
| assertType('object|string', $val); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** @param array{string, string} $task */ | ||
| public function testConstantArrayNarrowing(array $task): void | ||
| { | ||
| if (\is_callable($task)) { | ||
| assertType('list{class-string, string}&callable(): mixed', $task); | ||
| assertType('class-string', $task[0]); | ||
| assertType('string', $task[1]); | ||
| } | ||
| } | ||
|
|
||
| /** @param array<string> $task */ | ||
| public function testTypedArrayNarrowing(array $task): void | ||
| { | ||
| if (\is_callable($task)) { | ||
| // When value type is string, intersect with class-string|object gives class-string | ||
| // and intersect with string gives string | ||
| assertType('list{class-string, string}&callable(): mixed', $task); | ||
| } | ||
| } | ||
|
|
||
| /** @param callable-array $value */ | ||
| public function testCallableArrayPhpDoc(array $value): void | ||
| { | ||
| assertType('list{class-string|object, string}&callable(): mixed', $value); | ||
| assertType('class-string|object', $value[0]); | ||
| assertType('string', $value[1]); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14549Rule; | ||
|
|
||
| class Foo | ||
| { | ||
| public function foo(array $task): void | ||
| { | ||
| if (\is_callable($task)) { | ||
| $this->call($task); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param array<int> $task | ||
| */ | ||
| public function call(array $task): void | ||
| { | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really know at this point that
$types[$i]did not have more offsets than 0/1?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed. Here's what was done to address staabm's review:
The concern: For the
ArrayType + CallableTypenarrowing, does the code correctly account for all type information when replacing anArrayTypewith a 2-elementConstantArrayType?The answer: A plain
ArrayTypedoesn't have specific offsets (that'sConstantArrayType), but it does carry a key type. Since callable arrays always require integer keys 0 and 1, the original code wasn't checking whether the key type was compatible with integers. For example,array<string, mixed> & callableshould beneverbecause string keys are incompatible with the integer keys a callable array requires.Changes:
narrowArrayTypeWithCallable()— moved the inline narrowing logic into a dedicated method (mirroringnarrowConstantArrayWithCallable) for clarity$arrayType->getKeyType()->isSuperTypeOf(new IntegerType())before narrowing; returnsNeverTypeif the key type can't include integersarray<string, mixed>+is_callable()now correctly resolves to*NEVER*All 11,942 tests pass and
make phpstanreports no errors.