Skip to content

Commit

Permalink
Late binding of enum cases
Browse files Browse the repository at this point in the history 
Resolves a number of long-standing bugs ('Failed to infer case value ...')

Fixes #10374
Fixes #10560
Fixes #10643
Fixes #8978
  • Loading branch information
@weirdan
weirdan committed Feb 5, 2024
1 parent 59df6a7 commit e99f997
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 34 deletions.
17 changes: 9 additions & 8 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2486,25 +2486,26 @@ 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',
$case_storage->stmt_location,
$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',
$case_storage->stmt_location,
$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(
Expand All @@ -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',
Expand All @@ -2526,7 +2527,7 @@ private function checkEnum(): void
),
);
} else {
$seen_values[] = $case_storage->value;
$seen_values[] = $case_value;
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions src/Psalm/Internal/Codebase/ConstantTypeResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 5 additions & 3 deletions src/Psalm/Internal/Codebase/Methods.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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,
);
}
}

Expand Down
9 changes: 5 additions & 4 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/GetObjectVarsReturnTypeProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 7 additions & 4 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2971,11 +2971,12 @@ private static function reconcileValueOf(
// For value-of<MyBackedEnum>, 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;
Expand All @@ -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 === []) {
Expand Down
33 changes: 31 additions & 2 deletions src/Psalm/Storage/EnumCaseStorage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand All @@ -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;
}
}
18 changes: 11 additions & 7 deletions src/Psalm/Type/Atomic/TValueOf.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,24 @@ public function __construct(Union $type, bool $from_docblock = false)
/**
* @param non-empty-array<string,EnumCaseStorage> $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),
));
Expand Down Expand Up @@ -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;
}
Expand Down
34 changes: 34 additions & 0 deletions tests/EnumTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,40 @@ enum Bar: int
'ignored_issues' => [],
'php_version' => '8.1',
],
'enumWithCasesReferencingClassConstantsWhereClassIsDefinedAfterTheEnum' => [
'code' => <<<'PHP'
<?php
enum Bar: string {
case FOO = Foo::FOO;
}
class Foo {
const FOO = "foo";
}
$a = Bar::FOO->value;
PHP,
'assertions' => [
'$a===' => "'foo'",
],
'ignored_issues' => [],
'php_version' => '8.1',
],
'enumWithCasesReferencingAnotherEnumCase' => [
'code' => <<<'PHP'
<?php
enum Bar: string {
case BAR = Foo::FOO->value;
}
enum Foo: string {
case FOO = "foo";
}
$a = Bar::BAR->value;
PHP,
'assertions' => [
'$a===' => "'foo'",
],
'ignored_issues' => [],
'php_version' => '8.1',
],
];
}

Expand Down

0 comments on commit e99f997

Please sign in to comment.