From 52eadab971aa25bb21b38d5d2d170af4c25cf76b Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 5 Feb 2024 04:00:10 +0100 Subject: [PATCH] Late binding of enum cases Resolves a number of long-standing bugs ('Failed to infer case value ...') Fixes vimeo/psalm#10374 Fixes vimeo/psalm#10560 Fixes vimeo/psalm#10643 Fixes vimeo/psalm#8978 --- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 17 +++++----- .../Fetch/AtomicPropertyFetchAnalyzer.php | 9 ++--- .../Codebase/ConstantTypeResolver.php | 7 ++++ src/Psalm/Internal/Codebase/Methods.php | 8 +++-- .../Reflector/ClassLikeNodeScanner.php | 12 +++++-- .../GetObjectVarsReturnTypeProvider.php | 9 ++--- .../Type/SimpleAssertionReconciler.php | 11 +++--- src/Psalm/Storage/EnumCaseStorage.php | 33 ++++++++++++++++-- src/Psalm/Type/Atomic/TValueOf.php | 18 ++++++---- tests/EnumTest.php | 34 +++++++++++++++++++ 10 files changed, 124 insertions(+), 34 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index 3fcaa310d48..254c5cc53c0 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -2486,7 +2486,8 @@ private function checkEnum(): void $seen_values = []; foreach ($storage->enum_cases as $case_storage) { - if ($case_storage->value !== null && $storage->enum_type === null) { + $case_value = $case_storage->getValue($this->getCodebase()->classlikes); + if ($case_value !== null && $storage->enum_type === null) { IssueBuffer::maybeAdd( new InvalidEnumCaseValue( 'Case of a non-backed enum should not have a value', @@ -2494,7 +2495,7 @@ private function checkEnum(): void $storage->name, ), ); - } elseif ($case_storage->value === null && $storage->enum_type !== null) { + } elseif ($case_value === null && $storage->enum_type !== null) { IssueBuffer::maybeAdd( new InvalidEnumCaseValue( 'Case of a backed enum should have a value', @@ -2502,9 +2503,9 @@ private function checkEnum(): void $storage->name, ), ); - } elseif ($case_storage->value !== null) { - if ((is_int($case_storage->value) && $storage->enum_type === 'string') - || (is_string($case_storage->value) && $storage->enum_type === 'int') + } elseif ($case_value !== null) { + if ((is_int($case_value) && $storage->enum_type === 'string') + || (is_string($case_value) && $storage->enum_type === 'int') ) { IssueBuffer::maybeAdd( new InvalidEnumCaseValue( @@ -2516,8 +2517,8 @@ private function checkEnum(): void } } - if ($case_storage->value !== null) { - if (in_array($case_storage->value, $seen_values, true)) { + if ($case_value !== null) { + if (in_array($case_value, $seen_values, true)) { IssueBuffer::maybeAdd( new DuplicateEnumCaseValue( 'Enum case values should be unique', @@ -2526,7 +2527,7 @@ private function checkEnum(): void ), ); } else { - $seen_values[] = $case_storage->value; + $seen_values[] = $case_value; } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index f01f379e26a..6ad6983b76e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -1035,10 +1035,11 @@ private static function handleEnumValue( $case_values = []; foreach ($enum_cases as $enum_case) { - if (is_string($enum_case->value)) { - $case_values[] = Type::getAtomicStringFromLiteral($enum_case->value); - } elseif (is_int($enum_case->value)) { - $case_values[] = new TLiteralInt($enum_case->value); + $case_value = $enum_case->getValue($statements_analyzer->getCodebase()->classlikes); + if (is_string($case_value)) { + $case_values[] = Type::getAtomicStringFromLiteral($case_value); + } elseif (is_int($case_value)) { + $case_values[] = new TLiteralInt($case_value); } else { // this should never happen $case_values[] = new TMixed(); diff --git a/src/Psalm/Internal/Codebase/ConstantTypeResolver.php b/src/Psalm/Internal/Codebase/ConstantTypeResolver.php index 4bc718e3e49..fc6940b1cfb 100644 --- a/src/Psalm/Internal/Codebase/ConstantTypeResolver.php +++ b/src/Psalm/Internal/Codebase/ConstantTypeResolver.php @@ -344,6 +344,13 @@ public static function resolve( return Type::getString($value)->getSingleAtomic(); } elseif (is_int($value)) { return Type::getInt(false, $value)->getSingleAtomic(); + } elseif ($value instanceof UnresolvedConstantComponent) { + return self::resolve( + $classlikes, + $value, + $statements_analyzer, + $visited_constant_ids + [$c_id => true], + ); } } elseif ($c instanceof EnumNameFetch) { return Type::getString($c->case)->getSingleAtomic(); diff --git a/src/Psalm/Internal/Codebase/Methods.php b/src/Psalm/Internal/Codebase/Methods.php index 9648729c473..ad97dfbc65e 100644 --- a/src/Psalm/Internal/Codebase/Methods.php +++ b/src/Psalm/Internal/Codebase/Methods.php @@ -628,11 +628,13 @@ public function getMethodReturnType( ) { $types = []; foreach ($original_class_storage->enum_cases as $case_name => $case_storage) { + $case_value = $case_storage->getValue($this->classlikes); + if (UnionTypeComparator::isContainedBy( $source_analyzer->getCodebase(), - is_int($case_storage->value) ? - Type::getInt(false, $case_storage->value) : - Type::getString($case_storage->value), + is_int($case_value) ? + Type::getInt(false, $case_value) : + Type::getString($case_value), $first_arg_type, )) { $types[] = new TEnumCase($original_fq_class_name, $case_name); diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index 7589c018f98..999f95df554 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -32,6 +32,7 @@ use Psalm\Internal\Provider\NodeDataProvider; use Psalm\Internal\Scanner\ClassLikeDocblockComment; use Psalm\Internal\Scanner\FileScanner; +use Psalm\Internal\Scanner\UnresolvedConstantComponent; use Psalm\Internal\Type\TypeAlias; use Psalm\Internal\Type\TypeAlias\ClassTypeAlias; use Psalm\Internal\Type\TypeAlias\InlineTypeAlias; @@ -65,7 +66,6 @@ use Psalm\Type\Atomic\TString; use Psalm\Type\Atomic\TTemplateParam; use Psalm\Type\Union; -use RuntimeException; use UnexpectedValueException; use function array_merge; @@ -752,6 +752,9 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool $values_types[] = Type::getAtomicStringFromLiteral($enumCaseStorage->value); } elseif (is_int($enumCaseStorage->value)) { $values_types[] = new Type\Atomic\TLiteralInt($enumCaseStorage->value); + } elseif ($enumCaseStorage->value instanceof UnresolvedConstantComponent) { + // backed enum with a type yet unknown + $values_types[] = new Type\Atomic\TMixed; } } } @@ -1462,7 +1465,12 @@ private function visitEnumDeclaration( ); } } else { - throw new RuntimeException('Failed to infer case value for ' . $stmt->name->name); + $enum_value = ExpressionResolver::getUnresolvedClassConstExpr( + $stmt->expr, + $this->aliases, + $fq_classlike_name, + $storage->parent_class, + ); } } diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/GetObjectVarsReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/GetObjectVarsReturnTypeProvider.php index 55e4f38bd42..910300497fc 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/GetObjectVarsReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/GetObjectVarsReturnTypeProvider.php @@ -63,10 +63,11 @@ public static function getGetObjectVarsReturnType( return new TKeyedArray($properties); } $enum_case_storage = $enum_classlike_storage->enum_cases[$object_type->case_name]; - if (is_int($enum_case_storage->value)) { - $properties['value'] = new Union([new Atomic\TLiteralInt($enum_case_storage->value)]); - } elseif (is_string($enum_case_storage->value)) { - $properties['value'] = new Union([Type::getAtomicStringFromLiteral($enum_case_storage->value)]); + $case_value = $enum_case_storage->getValue($statements_source->getCodebase()->classlikes); + if (is_int($case_value)) { + $properties['value'] = new Union([new Atomic\TLiteralInt($case_value)]); + } elseif (is_string($case_value)) { + $properties['value'] = new Union([Type::getAtomicStringFromLiteral($case_value)]); } return new TKeyedArray($properties); } diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php index 28807b052b7..66cfc2f9e96 100644 --- a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php @@ -2971,11 +2971,12 @@ private static function reconcileValueOf( // For value-of, the assertion is meant to return *ANY* value of *ANY* enum case if ($enum_case_to_assert === null) { foreach ($class_storage->enum_cases as $enum_case) { + $enum_value = $enum_case->getValue($codebase->classlikes); assert( - $enum_case->value !== null, + $enum_value !== null, 'Verified enum type above, value can not contain `null` anymore.', ); - $reconciled_types[] = Type::getLiteral($enum_case->value); + $reconciled_types[] = Type::getLiteral($enum_value); } continue; @@ -2986,8 +2987,10 @@ private static function reconcileValueOf( return null; } - assert($enum_case->value !== null, 'Verified enum type above, value can not contain `null` anymore.'); - $reconciled_types[] = Type::getLiteral($enum_case->value); + $enum_value = $enum_case->getValue($codebase->classlikes); + + assert($enum_value !== null, 'Verified enum type above, value can not contain `null` anymore.'); + $reconciled_types[] = Type::getLiteral($enum_value); } if ($reconciled_types === []) { diff --git a/src/Psalm/Storage/EnumCaseStorage.php b/src/Psalm/Storage/EnumCaseStorage.php index 4c91419a69b..ec0e6ee74b6 100644 --- a/src/Psalm/Storage/EnumCaseStorage.php +++ b/src/Psalm/Storage/EnumCaseStorage.php @@ -3,13 +3,19 @@ namespace Psalm\Storage; use Psalm\CodeLocation; +use Psalm\Internal\Codebase\ClassLikes; +use Psalm\Internal\Codebase\ConstantTypeResolver; +use Psalm\Internal\Scanner\UnresolvedConstantComponent; +use Psalm\Type\Atomic\TLiteralInt; +use Psalm\Type\Atomic\TLiteralString; +use UnexpectedValueException; final class EnumCaseStorage { use UnserializeMemoryUsageSuppressionTrait; /** - * @var int|string|null + * @var int|string|null|UnresolvedConstantComponent */ public $value; @@ -22,7 +28,7 @@ final class EnumCaseStorage public $deprecated = false; /** - * @param int|string|null $value + * @param int|string|null|UnresolvedConstantComponent $value */ public function __construct( $value, @@ -31,4 +37,27 @@ public function __construct( $this->value = $value; $this->stmt_location = $location; } + + /** @return int|string|null */ + public function getValue(ClassLikes $classlikes) + { + $case_value = $this->value; + + if ($case_value instanceof UnresolvedConstantComponent) { + $case_value = ConstantTypeResolver::resolve( + $classlikes, + $case_value, + ); + + if ($case_value instanceof TLiteralString) { + $case_value = $case_value->value; + } elseif ($case_value instanceof TLiteralInt) { + $case_value = $case_value->value; + } else { + throw new UnexpectedValueException('Failed to infer case value'); + } + } + + return $case_value; + } } diff --git a/src/Psalm/Type/Atomic/TValueOf.php b/src/Psalm/Type/Atomic/TValueOf.php index 59941bc61e5..eb8df8ce03a 100644 --- a/src/Psalm/Type/Atomic/TValueOf.php +++ b/src/Psalm/Type/Atomic/TValueOf.php @@ -32,20 +32,24 @@ public function __construct(Union $type, bool $from_docblock = false) /** * @param non-empty-array $cases */ - private static function getValueTypeForNamedObject(array $cases, TNamedObject $atomic_type): Union - { + private static function getValueTypeForNamedObject( + array $cases, + TNamedObject $atomic_type, + Codebase $codebase + ): Union { if ($atomic_type instanceof TEnumCase) { assert(isset($cases[$atomic_type->case_name]), 'Should\'ve been verified in TValueOf#getValueType'); - $value = $cases[$atomic_type->case_name]->value; + $value = $cases[$atomic_type->case_name]->getValue($codebase->classlikes); assert($value !== null, 'Backed enum must have a value.'); return new Union([ConstantTypeResolver::getLiteralTypeFromScalarValue($value)]); } return new Union(array_map( - static function (EnumCaseStorage $case): Atomic { - assert($case->value !== null); + static function (EnumCaseStorage $case) use ($codebase): Atomic { + $case_value = $case->getValue($codebase->classlikes); + assert($case_value !== null); // Backed enum must have a value - return ConstantTypeResolver::getLiteralTypeFromScalarValue($case->value); + return ConstantTypeResolver::getLiteralTypeFromScalarValue($case_value); }, array_values($cases), )); @@ -141,7 +145,7 @@ public static function getValueType( continue; } - $value_atomics = self::getValueTypeForNamedObject($cases, $atomic_type); + $value_atomics = self::getValueTypeForNamedObject($cases, $atomic_type, $codebase); } else { continue; } diff --git a/tests/EnumTest.php b/tests/EnumTest.php index 49d1693054d..f66cdae3889 100644 --- a/tests/EnumTest.php +++ b/tests/EnumTest.php @@ -679,6 +679,40 @@ enum Bar: int 'ignored_issues' => [], 'php_version' => '8.1', ], + 'enumWithCasesReferencingClassConstantsWhereClassIsDefinedAfterTheEnum' => [ + 'code' => <<<'PHP' + value; + PHP, + 'assertions' => [ + '$a===' => "'foo'", + ], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], + 'enumWithCasesReferencingAnotherEnumCase' => [ + 'code' => <<<'PHP' + value; + } + enum Foo: string { + case FOO = "foo"; + } + $a = Bar::BAR->value; + PHP, + 'assertions' => [ + '$a===' => "'foo'", + ], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], ]; }