Skip to content
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

Infer ::from() and ::tryFrom() return types on backed enums #7011

Merged
merged 1 commit into from Nov 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
Expand Up @@ -2354,8 +2354,15 @@ private function checkImplementedInterfaces(
);
}

if ($storage->is_enum && $interface_method_name_lc === 'cases') {
continue;
if ($storage->is_enum) {
if ($interface_method_name_lc === 'cases') {
continue;
}
if ($storage->enum_type
&& in_array($interface_method_name_lc, ['from', 'tryfrom'], true)
) {
continue;
}
}

if (!$implementer_method_storage) {
Expand Down
66 changes: 54 additions & 12 deletions src/Psalm/Internal/Codebase/Methods.php
Expand Up @@ -121,8 +121,16 @@ public function methodExists(
return false;
}

if ($class_storage->is_enum && $method_name === 'cases') {
return true;
if ($class_storage->is_enum) {
if ($method_name === 'cases') {
return true;
}

if ($class_storage->enum_type
&& \in_array($method_name, ['from', 'tryFrom'], true)
) {
return true;
}
}

$source_file_path = $source ? $source->getFilePath() : $source_file_path;
Expand Down Expand Up @@ -686,21 +694,55 @@ public function getMethodReturnType(
$appearing_fq_class_storage = $this->classlike_storage_provider->get($appearing_fq_class_name);

if ($appearing_fq_class_name === 'UnitEnum'
&& $original_method_name === 'cases'
&& $original_class_storage->is_enum
&& $original_class_storage->enum_cases
) {
$types = [];
if ($original_method_name === 'cases') {
if ($original_class_storage->enum_cases === []) {
return Type::getEmptyArray();
}
$types = [];

foreach ($original_class_storage->enum_cases as $case_name => $_) {
$types[] = new Type\Union([new Type\Atomic\TEnumCase($original_fq_class_name, $case_name)]);
}
foreach ($original_class_storage->enum_cases as $case_name => $_) {
$types[] = new Type\Union([new Type\Atomic\TEnumCase($original_fq_class_name, $case_name)]);
}

$list = new Type\Atomic\TKeyedArray($types);
$list->is_list = true;
$list->sealed = true;
$list = new Type\Atomic\TKeyedArray($types);
$list->is_list = true;
$list->sealed = true;
return new Type\Union([$list]);
}
}

return new Type\Union([$list]);
if ($appearing_fq_class_name === 'BackedEnum'
&& $original_class_storage->is_enum
&& $original_class_storage->enum_type
) {
if (($original_method_name === 'from'
|| $original_method_name === 'tryfrom'
) && $source_analyzer
&& isset($args[0])
&& ($first_arg_type = $source_analyzer->getNodeTypeProvider()->getType($args[0]->value))
) {
$types = [];
foreach ($original_class_storage->enum_cases as $case_name => $case_storage) {
if (UnionTypeComparator::isContainedBy(
$source_analyzer->getCodebase(),
\is_int($case_storage->value) ?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use a constant to initialize a case? If it happens, is_int() will fail. Can't we access the type at this moment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible, and it fails. But not here.

https://3v4l.org/WHXIV#v8.1rc3
https://psalm.dev/r/3547ca91d3

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built-in constants work.
User-defined constants don't: https://3v4l.org/nhGRA#v8.1rc3
Class constants work if the class is defined before the enum: https://3v4l.org/bcYPN#v8.1rc3.
Class constants from autoloaded classes do not work: https://3v4l.org/AjOCi#v8.1rc3

3v4l doesn't have proper 8.1 yet, so it's based on RC3. Some of the above could have been fixed before final release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the above could have been fixed before final release.

I've just built 8.1 branch, and there are no changes for those cases.

Type::getInt(false, $case_storage->value) :
Type::getString($case_storage->value),
$first_arg_type
)) {
$types[] = new Type\Atomic\TEnumCase($original_fq_class_name, $case_name);
}
}
if ($types) {
if ($original_method_name === 'tryfrom') {
$types[] = new Type\Atomic\TNull();
}
return new Type\Union($types);
}
return $original_method_name === 'tryfrom' ? Type::getNull() : Type::getNever();
}
}

if (!$appearing_fq_class_storage->user_defined
Expand Down
12 changes: 11 additions & 1 deletion src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
Expand Up @@ -300,7 +300,17 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
$this->file_storage->has_visitor_issues = true;
$storage->has_visitor_issues = true;
}
// todo: $this->codebase->scanner->queueClassLikeForScanning('BackedEnum');
$storage->class_implements['backedenum'] = 'BackedEnum';
$storage->direct_class_interfaces['backedenum'] = 'BackedEnum';
$this->file_storage->required_interfaces['backedenum'] = 'BackedEnum';
$this->codebase->scanner->queueClassLikeForScanning('BackedEnum');
$storage->declaring_method_ids['from'] = new \Psalm\Internal\MethodIdentifier('BackedEnum', 'from');
$storage->appearing_method_ids['from'] = $storage->declaring_method_ids['from'];
$storage->declaring_method_ids['tryfrom'] = new \Psalm\Internal\MethodIdentifier(
'BackedEnum',
'tryfrom'
);
$storage->appearing_method_ids['tryfrom'] = $storage->declaring_method_ids['tryfrom'];
}

$this->codebase->scanner->queueClassLikeForScanning('UnitEnum');
Expand Down
9 changes: 8 additions & 1 deletion stubs/Php81.phpstub
Expand Up @@ -3,10 +3,17 @@ namespace {
interface UnitEnum {
/** @var non-empty-string $name */
public readonly string $name;

/** @return non-empty-list<static> */
public static function cases(): array;
}

interface BackedEnum
{
public readonly int|string $value;
public static function from(string|int $value): static;
public static function tryFrom(string|int $value): ?static;
}
}

namespace FTP {
Expand Down
103 changes: 103 additions & 0 deletions tests/EnumTest.php
Expand Up @@ -261,6 +261,109 @@ enum ParamType {
[],
'8.1',
],
'casesOnEnumWithNoCasesReturnEmptyArray' => [
'<?php
enum Status: int {}
$_z = Status::cases();
',
'assertions' => [
'$_z===' => 'array<empty, empty>',
],
[],
'8.1',
],
'backedEnumFromReturnsInstanceOfThatEnum' => [
'<?php
enum Status: int {
case Open = 1;
case Closed = 2;
}

function f(): Status {
return Status::from(1);
}
',
'assertions' => [],
[],
'8.1',
],
'backedEnumTryFromReturnsInstanceOfThatEnum' => [
'<?php
enum Status: int {
case Open = 1;
case Closed = 2;
}

function f(): Status {
return Status::tryFrom(rand(1, 10)) ?? Status::Open;
}
',
'assertions' => [],
[],
'8.1',
],
'backedEnumFromReturnsSpecificCase' => [
'<?php
enum Status: int {
case Open = 1;
case Closed = 2;
}

$_z = Status::from(2);
',
'assertions' => [
'$_z===' => 'enum(Status::Closed)',
],
[],
'8.1',
],
'backedEnumTryFromReturnsSpecificCase' => [
'<?php
enum Status: int {
case Open = 1;
case Closed = 2;
}

$_z = Status::tryFrom(2);
',
'assertions' => [
'$_z===' => 'enum(Status::Closed)|null',
],
[],
'8.1',
],
'backedEnumFromReturnsUnionOfCases' => [
'<?php
enum Status: int {
case Open = 1;
case Closed = 2;
case Busted = 3;
}

$_z = Status::from(rand(1, 2));
',
'assertions' => [
'$_z===' => 'enum(Status::Closed)|enum(Status::Open)',
],
[],
'8.1',
],
'backedEnumTryFromReturnsUnionOfCases' => [
'<?php
enum Status: int {
case Open = 1;
case Closed = 2;
case Busted = 3;
}

$_z = Status::tryFrom(rand(1, 2));
',
'assertions' => [
'$_z===' => 'enum(Status::Closed)|enum(Status::Open)|null',
],
[],
'8.1',
],
];
}

Expand Down