diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 5f95b0d6ce0..ed7dcd2e981 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -522,7 +522,14 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult $isUnsealed = $this->isUnsealed(); if (!$isUnsealed->yes()) { if ($type instanceof self && count($this->keyTypes) === 0) { - return AcceptsResult::createFromBoolean(count($type->keyTypes) === 0); + if (count($type->keyTypes) === 0) { + return AcceptsResult::createYes(); + } + + return AcceptsResult::createNo($isUnsealed->no() ? array_map( + static fn (Type $extraKeyType): string => sprintf('Sealed array shape does not accept array with extra key %s.', $extraKeyType->describe(VerbosityLevel::precise())), + $type->keyTypes, + ) : []); } } @@ -537,19 +544,43 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult return $result; } - if ($result->no()) { - return $result; - } - [$unsealedKeyType, $unsealedValueType] = $this->unsealed; - if ($isUnsealed->no()) { - if (!$type->isConstantArray()->yes()) { + // Checked before bailing out on an already-failing $result: how the shape handles keys + // it does not declare explains the rejection on its own, and checkOurKeys() often + // returns "no" without giving any reason at all. + if (!$type->isArray()->no() && !$type->isConstantArray()->yes()) { + if ($isUnsealed->no()) { return $result->and(AcceptsResult::createNo([ 'Sealed array shape can only accept a constant array. Extra keys are not allowed.', ])); } + $otherKeyType = $type->getIterableKeyType(); + $otherValueType = $type->getIterableValueType(); + + return $result->and(self::decorateUnsealedReasons( + $unsealedKeyType->accepts($otherKeyType, $strictTypes), + static fn (): string => sprintf( + 'Unsealed array key type %s does not accept key type %s', + $unsealedKeyType->describe(VerbosityLevel::value()), + $otherKeyType->describe(VerbosityLevel::value()), + ), + ))->and(self::decorateUnsealedReasons( + $unsealedValueType->accepts($otherValueType, $strictTypes), + static fn (): string => sprintf( + 'Unsealed array value type %s does not accept value type %s', + $unsealedValueType->describe(VerbosityLevel::value()), + $otherValueType->describe(VerbosityLevel::value()), + ), + )); + } + + if ($result->no()) { + return $result; + } + + if ($isUnsealed->no()) { $constantArrays = $type->getConstantArrays(); if (count($constantArrays) !== 1) { throw new ShouldNotHappenException('Type with more than one constant array occurred, should have been eliminated with `instanceof CompoundType` above.'); @@ -579,11 +610,6 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult return $result; } - if (!$type->isConstantArray()->yes()) { - return $result->and($unsealedKeyType->accepts($type->getIterableKeyType(), $strictTypes)) - ->and($unsealedValueType->accepts($type->getIterableValueType(), $strictTypes)); - } - $constantArrays = $type->getConstantArrays(); if (count($constantArrays) !== 1) { throw new ShouldNotHappenException('Type with more than one constant array occurred, should have been eliminated with `instanceof CompoundType` above.'); @@ -600,94 +626,73 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult } foreach ($keys as [$i, $extraKeyType]) { - $acceptsKey = $unsealedKeyType->accepts($extraKeyType, $strictTypes)->decorateReasons( - static fn (string $reason) => sprintf( - 'Unsealed array key type %s does not accept extra key type %s: %s', + $result = $result->and(self::decorateUnsealedReasons( + $unsealedKeyType->accepts($extraKeyType, $strictTypes), + static fn (): string => sprintf( + 'Unsealed array key type %s does not accept extra key type %s', $unsealedKeyType->describe(VerbosityLevel::value()), $extraKeyType->describe(VerbosityLevel::value()), - $reason, ), - ); - if (!$acceptsKey->yes() && count($acceptsKey->reasons) === 0) { - $acceptsKey = new AcceptsResult($acceptsKey->result, [ - sprintf( - 'Unsealed array key type %s does not accept extra key type %s.', - $unsealedKeyType->describe(VerbosityLevel::value()), - $extraKeyType->describe(VerbosityLevel::value()), - ), - ]); - } - $result = $result->and($acceptsKey); + )); $extraValueType = $constantArray->getValueTypes()[$i]; - $acceptsValue = $unsealedValueType->accepts($extraValueType, $strictTypes)->decorateReasons( - static fn (string $reason) => sprintf( - 'Unsealed array value type %s does not accept extra offset %s with value type %s: %s', + $result = $result->and(self::decorateUnsealedReasons( + $unsealedValueType->accepts($extraValueType, $strictTypes), + static fn (): string => sprintf( + 'Unsealed array value type %s does not accept extra offset %s with value type %s', $unsealedValueType->describe(VerbosityLevel::value()), $extraKeyType->describe(VerbosityLevel::value()), $extraValueType->describe(VerbosityLevel::value()), - $reason, ), - ); - if (!$acceptsValue->yes() && count($acceptsValue->reasons) === 0) { - $acceptsValue = new AcceptsResult($acceptsValue->result, [ - sprintf( - 'Unsealed array value type %s does not accept extra offset %s with value type %s.', - $unsealedValueType->describe(VerbosityLevel::value()), - $extraKeyType->describe(VerbosityLevel::value()), - $extraValueType->describe(VerbosityLevel::value()), - ), - ]); - } - $result = $result->and($acceptsValue); + )); } $otherUnsealed = $constantArray->unsealed; if ($otherUnsealed !== null && !$constantArray->isUnsealed()->no()) { [$otherUnsealedKeyType, $otherUnsealedValueType] = $otherUnsealed; - $acceptsUnsealedKey = $unsealedKeyType->accepts($otherUnsealedKeyType, $strictTypes)->decorateReasons( - static fn (string $reason) => sprintf( - 'Unsealed array key type %s does not accept unsealed array key type %s: %s', + $result = $result->and(self::decorateUnsealedReasons( + $unsealedKeyType->accepts($otherUnsealedKeyType, $strictTypes), + static fn (): string => sprintf( + 'Unsealed array key type %s does not accept unsealed array key type %s', $unsealedKeyType->describe(VerbosityLevel::value()), $otherUnsealedKeyType->describe(VerbosityLevel::value()), - $reason, ), - ); - if (!$acceptsUnsealedKey->yes() && count($acceptsUnsealedKey->reasons) === 0) { - $acceptsUnsealedKey = new AcceptsResult($acceptsUnsealedKey->result, [ - sprintf( - 'Unsealed array key type %s does not accept unsealed array key type %s.', - $unsealedKeyType->describe(VerbosityLevel::value()), - $otherUnsealedKeyType->describe(VerbosityLevel::value()), - ), - ]); - } - $result = $result->and($acceptsUnsealedKey); + )); - $acceptsUnsealedValue = $unsealedValueType->accepts($otherUnsealedValueType, $strictTypes)->decorateReasons( - static fn (string $reason) => sprintf( - 'Unsealed array value type %s does not accept unsealed array value type %s: %s', + $result = $result->and(self::decorateUnsealedReasons( + $unsealedValueType->accepts($otherUnsealedValueType, $strictTypes), + static fn (): string => sprintf( + 'Unsealed array value type %s does not accept unsealed array value type %s', $unsealedValueType->describe(VerbosityLevel::value()), $otherUnsealedValueType->describe(VerbosityLevel::value()), - $reason, ), - ); - if (!$acceptsUnsealedValue->yes() && count($acceptsUnsealedValue->reasons) === 0) { - $acceptsUnsealedValue = new AcceptsResult($acceptsUnsealedValue->result, [ - sprintf( - 'Unsealed array value type %s does not accept unsealed array value type %s.', - $unsealedValueType->describe(VerbosityLevel::value()), - $otherUnsealedValueType->describe(VerbosityLevel::value()), - ), - ]); - } - $result = $result->and($acceptsUnsealedValue); + )); } return $result; } + /** + * Makes sure a failing unsealed-part check is never reported without an explanation: + * inner reasons are prefixed with $describeFailure, which also becomes the only reason + * when the inner check produced none. + * + * @param callable(): string $describeFailure + */ + private static function decorateUnsealedReasons(AcceptsResult $result, callable $describeFailure): AcceptsResult + { + if ($result->yes()) { + return $result; + } + + if (count($result->reasons) === 0) { + return new AcceptsResult($result->result, [sprintf('%s.', $describeFailure())]); + } + + return $result->decorateReasons(static fn (string $reason): string => sprintf('%s: %s', $describeFailure(), $reason)); + } + private function checkOurKeys(Type $type, bool $strictTypes): AcceptsResult { $result = AcceptsResult::createYes(); diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 4d2e9a4f764..a448ede7421 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1372,4 +1372,25 @@ public function testBug12984(): void $this->analyse([__DIR__ . '/data/bug-12984.php'], []); } + public function testBug15234(): void + { + $this->analyse([__DIR__ . '/data/bug-15234.php'], [ + [ + 'Method Bug15234\\SwitchApi::queryDeviceSignalStrength() should return array{eventTime: 0}|array{eventTime: int, indicatorType: string, signalLevel: string, signal: int} but returns array.', + 30, + "\u{2022} Type #1 from the union: Sealed array shape can only accept a constant array. Extra keys are not allowed.\n\u{2022} Type #2 from the union: Sealed array shape can only accept a constant array. Extra keys are not allowed.", + ], + [ + 'Method Bug15234\\SwitchApi::queryDeviceProperties() should return array but returns array>.', + 40, + 'Sealed array shape can only accept a constant array. Extra keys are not allowed.', + ], + [ + 'Method Bug15234\\SwitchApi::queryDeviceFunctions() should return array{category: string, functions: array{type: string, values: array|string>}} but returns non-empty-array.', + 62, + "\u{2022} Offset 'functions' (array{type: string, values: array|string>}) does not accept type array>: Sealed array shape can only accept a constant array. Extra keys are not allowed.\n\u{2022} Sealed array shape can only accept a constant array. Extra keys are not allowed.", + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-15234.php b/tests/PHPStan/Rules/Methods/data/bug-15234.php new file mode 100644 index 00000000000..ca6f894af08 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-15234.php @@ -0,0 +1,64 @@ + + */ + public function sendRequest(string $method, string $path) + { + return []; + } + + /** + * @return array{eventTime: 0}|array{eventTime: int, indicatorType: string, signalLevel: string, signal: int} + */ + public function queryDeviceSignalStrength(string $deviceId): array + { + return $this->sendRequest('get', '/v2.0/cloud/thing/' . $deviceId . '/WiFi/signal'); + } + + /** + * @return array + */ + public function queryDeviceProperties(string $deviceId): array + { + $response = $this->sendRequest('get', '/v2.0/cloud/thing/' . $deviceId . '/shadow/properties'); + + return array_combine( + array_map(static fn ($v) => $v['code'], $response['properties']), + array_map(static fn ($v) => array_diff_key($v, ['code' => true]), $response['properties']) + ); + } + + /** + * @return array{category: string, functions: array{type: string, values: array>}} + */ + public function queryDeviceFunctions(string $deviceId): array + { + $response = $this->sendRequest('get', '/v1.0/iot-03/devices/' . $deviceId . '/functions'); + + $response['functions'] = array_combine( + array_map(static fn ($v) => $v['code'], $response['functions']), + array_map(function ($v) { + $v['values'] = $this->jsonDecode($v['values']); + + return array_diff_key($v, ['code' => true, 'desc' => true, 'name' => true]); + }, $response['functions']) + ); + + return $response; + } +} diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index f749b4c010e..41499dd53e3 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -873,6 +873,7 @@ public function testBug13438d(): void [ 'Property Bug13438d\Test::$queue (array{}) does not accept array{1}.', 18, + 'Sealed array shape does not accept array with extra key 0.', ], ]); } @@ -884,6 +885,7 @@ public function testBug13438e(): void [ 'Property Bug13438e\Test::$queue (array{}) does not accept array{1}.', 18, + 'Sealed array shape does not accept array with extra key 0.', ], ]); } diff --git a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php index 3fadd06a60d..a0ee16d9f6a 100644 --- a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php +++ b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php @@ -9,6 +9,7 @@ use PHPStan\TrinaryLogic; use PHPStan\Type\Accessory\HasOffsetType; use PHPStan\Type\Accessory\HasOffsetValueType; +use PHPStan\Type\Accessory\NonEmptyArrayType; use PHPStan\Type\ArrayType; use PHPStan\Type\BooleanType; use PHPStan\Type\CallableType; @@ -460,7 +461,7 @@ public static function dataAcceptsWithBleedingEdge(): array new ConstantArrayType([], []), new ConstantArrayType([new ConstantStringType('a')], [new StringType()]), TrinaryLogic::createNo(), - [], + ['Sealed array shape does not accept array with extra key \'a\'.'], ], // non-empty array (sealed) does not accept extra keys @@ -485,6 +486,26 @@ public static function dataAcceptsWithBleedingEdge(): array ['Sealed array shape can only accept a constant array. Extra keys are not allowed.'], ], + // sealed array does not accept general array even when an offset already mismatches + [ + new ConstantArrayType([new ConstantStringType('a')], [new StringType()]), + new ArrayType(new StringType(), new IntegerType()), + TrinaryLogic::createNo(), + ['Sealed array shape can only accept a constant array. Extra keys are not allowed.'], + ], + + // sealed array does not accept general array with a known offset + [ + new ConstantArrayType([new ConstantStringType('a')], [new StringType()]), + new IntersectionType([ + new ArrayType(new StringType(), new IntegerType()), + new HasOffsetValueType(new ConstantStringType('a'), new IntegerType()), + new NonEmptyArrayType(), + ]), + TrinaryLogic::createNo(), + ['Sealed array shape can only accept a constant array. Extra keys are not allowed.'], + ], + // sealed array does not accept unsealed array [ new ConstantArrayType([new ConstantStringType('a')], [new StringType()]), @@ -493,6 +514,14 @@ public static function dataAcceptsWithBleedingEdge(): array ['Sealed array shape does not accept unsealed array shape.'], ], + // unsealed array does not accept general array with an incompatible key type + [ + new ConstantArrayType([new ConstantStringType('a')], [new StringType()], unsealed: [new IntegerType(), new StringType()]), + new ArrayType(new StringType(), new StringType()), + TrinaryLogic::createNo(), + ['Unsealed array key type int does not accept key type string.'], + ], + // unsealed array accepts compatible general array [ new ConstantArrayType([new ConstantStringType('a')], [new StringType()], unsealed: [new StringType(), new StringType()]), @@ -523,7 +552,9 @@ public static function dataAcceptsWithBleedingEdge(): array new HasOffsetValueType(new ConstantStringType('a'), new StringType()), ]), TrinaryLogic::createNo(), - [], + [ + 'Unsealed array value type int does not accept value type string.', + ], ], // unsealed array must check extra keys against its own unsealed types