diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 6cff7ef3af..cc1a545e25 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -741,7 +741,14 @@ public function unsetOffset(Type $offsetType): Type $k++; } - return new self($newKeyTypes, $newValueTypes, $this->nextAutoIndexes, $newOptionalKeys, TrinaryLogic::createNo()); + $newIsList = self::isListAfterUnset( + $newKeyTypes, + $newOptionalKeys, + $this->isList, + in_array($i, $this->optionalKeys, true), + ); + + return new self($newKeyTypes, $newValueTypes, $this->nextAutoIndexes, $newOptionalKeys, $newIsList); } return $this; @@ -751,6 +758,7 @@ public function unsetOffset(Type $offsetType): Type if (count($constantScalars) > 0) { $optionalKeys = $this->optionalKeys; + $arrayHasChanged = false; foreach ($constantScalars as $constantScalar) { $constantScalar = $constantScalar->toArrayKey(); if (!$constantScalar instanceof ConstantIntegerType && !$constantScalar instanceof ConstantStringType) { @@ -762,6 +770,7 @@ public function unsetOffset(Type $offsetType): Type continue; } + $arrayHasChanged = true; if (in_array($i, $optionalKeys, true)) { continue 2; } @@ -770,21 +779,77 @@ public function unsetOffset(Type $offsetType): Type } } - return new self($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $optionalKeys, TrinaryLogic::createNo()); + if (!$arrayHasChanged) { + return $this; + } + + $newIsList = self::isListAfterUnset( + $this->keyTypes, + $optionalKeys, + $this->isList, + count($optionalKeys) === count($this->optionalKeys), + ); + + return new self($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $optionalKeys, $newIsList); } $optionalKeys = $this->optionalKeys; - $isList = $this->isList; + $arrayHasChanged = false; foreach ($this->keyTypes as $i => $keyType) { if (!$offsetType->isSuperTypeOf($keyType)->yes()) { continue; } + $arrayHasChanged = true; $optionalKeys[] = $i; - $isList = TrinaryLogic::createNo(); } $optionalKeys = array_values(array_unique($optionalKeys)); - return new self($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $optionalKeys, $isList); + if (!$arrayHasChanged) { + return $this; + } + + $newIsList = self::isListAfterUnset( + $this->keyTypes, + $optionalKeys, + $this->isList, + count($optionalKeys) === count($this->optionalKeys), + ); + + return new self($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $optionalKeys, $newIsList); + } + + /** + * When we're unsetting something not on the array, it will be untouched, + * So the nextAutoIndexes won't change, and the array might still be a list even with PHPStan definition. + * + * @param list $newKeyTypes + * @param int[] $newOptionalKeys + */ + private static function isListAfterUnset(array $newKeyTypes, array $newOptionalKeys, TrinaryLogic $arrayIsList, bool $unsetOptionalKey): TrinaryLogic + { + if (!$unsetOptionalKey || $arrayIsList->no()) { + return TrinaryLogic::createNo(); + } + + $isListOnlyIfKeysAreOptional = false; + foreach ($newKeyTypes as $k2 => $newKeyType2) { + if (!$newKeyType2 instanceof ConstantIntegerType || $newKeyType2->getValue() !== $k2) { + // We found a non-optional key that implies that the array is never a list. + if (!in_array($k2, $newOptionalKeys, true)) { + return TrinaryLogic::createNo(); + } + + // The array can still be a list if all the following keys are also optional. + $isListOnlyIfKeysAreOptional = true; + continue; + } + + if ($isListOnlyIfKeysAreOptional && !in_array($k2, $newOptionalKeys, true)) { + return TrinaryLogic::createNo(); + } + } + + return TrinaryLogic::createMaybe(); } public function chunkArray(Type $lengthType, TrinaryLogic $preserveKeys): Type diff --git a/tests/PHPStan/Analyser/nsrt/bug-14177.php b/tests/PHPStan/Analyser/nsrt/bug-14177.php new file mode 100644 index 0000000000..1610725170 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14177.php @@ -0,0 +1,203 @@ +', + $id, + array_key_exists(3, $matches) ? sprintf(' class="%s"', $matches[3]) : '', + ); + + assertType('list{0: non-falsy-string, 1: numeric-string, 2?: string, 3?: string}', $matches); + + return array_key_exists(2, $matches) && $matches[2] !== '' + ? sprintf('%s', $matches[2], $replacement) + : $replacement; + }, + $html, + ); + } + + public function placeholderToEditor2(string $html): void + { + $result = preg_replace_callback( + '~\[image\\sid="(\\d+)?"(?:\\shref="([^"]*)")?(?:\\sclass="([^"]*)")?\]~', + function (array $matches): string { + $id = (int) $matches[0]; + + assertType('list{0: non-falsy-string, 1?: \'\'|numeric-string, 2?: string, 3?: string}', $matches); + + $replacement = sprintf( + '', + $id, + array_key_exists(2, $matches) ? sprintf(' class="%s"', $matches[2]) : '', + ); + + assertType('list{0: non-falsy-string, 1?: \'\'|numeric-string, 2?: string, 3?: string}', $matches); + + return array_key_exists(1, $matches) && $matches[1] !== '' + ? sprintf('%s', $matches[1], $replacement) + : $replacement; + }, + $html, + ); + } +} + +class HelloWorld2 +{ + /** + * @param list{0: string, 1: string, 2?: string, 3?: string} $b + */ + public function testUnset0OnList(array $b): void + { + assertType('true', array_is_list($b)); + unset($b[0]); + assertType('false', array_is_list($b)); + $b[] = 'foo'; + assertType('false', array_is_list($b)); + } + + /** + * @param list{0: string, 1: string, 2?: string, 3?: string} $b + */ + public function testUnset1OnList(array $b): void + { + assertType('true', array_is_list($b)); + unset($b[1]); + assertType('false', array_is_list($b)); + $b[] = 'foo'; + assertType('false', array_is_list($b)); + } + + /** + * @param list{0: string, 1: string, 2?: string, 3?: string} $b + */ + public function testUnset2OnList(array $b): void + { + assertType('true', array_is_list($b)); + unset($b[2]); + assertType('bool', array_is_list($b)); + $b[] = 'foo'; + assertType('bool', array_is_list($b)); + } + + /** + * @param list{0: string, 1: string, 2?: string, 3?: string} $b + */ + public function testUnset3OnList(array $b): void + { + assertType('true', array_is_list($b)); + unset($b[3]); + assertType('bool', array_is_list($b)); + $b[] = 'foo'; + assertType('bool', array_is_list($b)); + } + + /** + * @param array{0: string, 1?: string, 2: string, 3?: string} $b + */ + public function testUnset0OnArray(array $b): void + { + assertType('bool', array_is_list($b)); + unset($b[0]); + assertType('false', array_is_list($b)); + $b[] = 'foo'; + assertType('false', array_is_list($b)); + } + + /** + * @param array{0: string, 1?: string, 2: string, 3?: string} $b + */ + public function testUnset1OnArray(array $b): void + { + assertType('bool', array_is_list($b)); + unset($b[1]); + assertType('false', array_is_list($b)); + $b[] = 'foo'; + assertType('false', array_is_list($b)); + } + + /** + * @param array{0: string, 1?: string, 2: string, 3?: string} $b + */ + public function testUnset2OnArray(array $b): void + { + assertType('bool', array_is_list($b)); + unset($b[2]); + assertType('false', array_is_list($b)); + $b[] = 'foo'; + assertType('false', array_is_list($b)); + } + + /** + * @param array{0: string, 1?: string, 2: string, 3?: string} $b + */ + public function testUnset3OnArray(array $b): void + { + assertType('bool', array_is_list($b)); + unset($b[3]); + assertType('bool', array_is_list($b)); + $b[] = 'foo'; + assertType('bool', array_is_list($b)); + } + + /** + * @param list{0: string, 1: string, 2?: string, 3?: string} $a + * @param list{0: string, 1?: string, 2?: string, 3?: string} $b + * @param list{0: string, 1?: string, 2?: string, 3: string} $c + * @param 1|2 $int + */ + public function testUnsetNonConstant(array $a, array $b, array $c, int $int): void + { + assertType('true', array_is_list($a)); + assertType('true', array_is_list($b)); + assertType('true', array_is_list($c)); + unset($a[$int]); + unset($b[$int]); + unset($c[$int]); + assertType('false', array_is_list($a)); + assertType('bool', array_is_list($b)); + assertType('bool', array_is_list($c)); + } + + /** + * @param list{0?: string, 1?: string, 2?: string, 3?: string} $a + * @param list{0: string, 1?: string, 2?: string, 3?: string} $b + */ + public function testUnsetInt(array $a, array $b, array $c, int $int): void + { + assertType('true', array_is_list($a)); + assertType('true', array_is_list($b)); + unset($a[$int]); + unset($b[$int]); + assertType('bool', array_is_list($a)); + assertType('false', array_is_list($b)); + } +} diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index b6f79eeeeb..580c6f466a 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -1145,4 +1145,10 @@ public function testPr4375(): void $this->analyse([__DIR__ . '/data/pr-4375.php'], []); } + public function testBug14177(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-14177.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14177.php b/tests/PHPStan/Rules/Comparison/data/bug-14177.php new file mode 100644 index 0000000000..9c45152e4e --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14177.php @@ -0,0 +1,27 @@ +', + $id, + array_key_exists(3, $matches) ? sprintf(' class="%s"', $matches[3]) : '', + ); + + return array_key_exists(2, $matches) && $matches[2] !== '' + ? sprintf('%s', $matches[2], $replacement) + : $replacement; + }, + $html, + ); + } +}