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

Assertion enhancements regarding value-of in combination with backed enums #10150

Merged
merged 10 commits into from
Aug 25, 2023
36 changes: 2 additions & 34 deletions src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use Psalm\Codebase;
use Psalm\Internal\MethodIdentifier;
use Psalm\Type;
use Psalm\Type\Atomic;
use Psalm\Type\Atomic\Scalar;
use Psalm\Type\Atomic\TArray;
Expand All @@ -18,12 +17,10 @@
use Psalm\Type\Atomic\TEmptyMixed;
use Psalm\Type\Atomic\TEnumCase;
use Psalm\Type\Atomic\TGenericObject;
use Psalm\Type\Atomic\TInt;
use Psalm\Type\Atomic\TIterable;
use Psalm\Type\Atomic\TKeyOf;
use Psalm\Type\Atomic\TKeyedArray;
use Psalm\Type\Atomic\TList;
use Psalm\Type\Atomic\TLiteralInt;
use Psalm\Type\Atomic\TLiteralString;
use Psalm\Type\Atomic\TMixed;
use Psalm\Type\Atomic\TNamedObject;
Expand All @@ -46,7 +43,6 @@
use function assert;
use function count;
use function get_class;
use function is_int;
use function strtolower;

/**
Expand Down Expand Up @@ -636,37 +632,9 @@ public static function isContainedBy(
}

if ($input_type_part instanceof TEnumCase
&& $codebase->classlike_storage_provider->has($input_type_part->value)
&& !$container_type_part instanceof TEnumCase
) {
if ($container_type_part instanceof TString || $container_type_part instanceof TInt) {
$input_type_classlike_storage = $codebase->classlike_storage_provider->get($input_type_part->value);
if ($input_type_classlike_storage->enum_type === null
|| !isset($input_type_classlike_storage->enum_cases[$input_type_part->case_name])
) {
// Not a backed enum or non-existent enum case
return false;
}

$input_type_enum_case_storage = $input_type_classlike_storage->enum_cases[$input_type_part->case_name];
assert(
$input_type_enum_case_storage->value !== null,
'Backed enums cannot have values without a value.',
);

if (is_int($input_type_enum_case_storage->value)) {
return self::isContainedBy(
$codebase,
new TLiteralInt($input_type_enum_case_storage->value),
$container_type_part,
);
}

return self::isContainedBy(
$codebase,
Type::getAtomicStringFromLiteral($input_type_enum_case_storage->value),
$container_type_part,
);
}
return false;
}

if ($container_type_part instanceof TString || $container_type_part instanceof TScalar) {
Expand Down
58 changes: 57 additions & 1 deletion src/Psalm/Internal/Type/SimpleAssertionReconciler.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
use function count;
use function explode;
use function get_class;
use function in_array;
use function is_int;
use function min;
use function strlen;
Expand Down Expand Up @@ -533,7 +534,10 @@
}

if ($assertion_type instanceof TValueOf) {
return $assertion_type->type;
return self::reconcileValueOf(
$codebase,
$assertion_type
);
}

return null;
Expand Down Expand Up @@ -2951,6 +2955,58 @@
return TypeCombiner::combine(array_values($matched_class_constant_types), $codebase);
}

private static function reconcileValueOf(
Codebase $codebase,
TValueOf $assertion_type
): ?Union {
$reconciled_types = [];

// For now, only enums are supported here
weirdan marked this conversation as resolved.
Show resolved Hide resolved
foreach ($assertion_type->type->getAtomicTypes() as $atomic_type) {
$class_name = null;

Check failure on line 2966 in src/Psalm/Internal/Type/SimpleAssertionReconciler.php

View workflow job for this annotation

GitHub Actions / build

UnusedVariable

src/Psalm/Internal/Type/SimpleAssertionReconciler.php:2966:13: UnusedVariable: $class_name is never referenced or the value is not used (see https://psalm.dev/077)
$enum_case_to_assert = null;
if ($atomic_type instanceof TClassConstant) {
$class_name = $atomic_type->fq_classlike_name;
$enum_case_to_assert = $atomic_type->const_name;
} elseif ($atomic_type instanceof TNamedObject) {
$class_name = $atomic_type->value;
} else {
return null;
}

if (!$codebase->classOrInterfaceOrEnumExists($class_name)) {
return null;
}

$class_storage = $codebase->classlike_storage_provider->get($class_name);
if (!$class_storage->is_enum) {
return null;
}

if (!in_array($class_storage->enum_type, ['string', 'int'], true)) {
return null;
}

// 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) {
$reconciled_types[] = Type::getLiteral($enum_case->value);

Check failure on line 2993 in src/Psalm/Internal/Type/SimpleAssertionReconciler.php

View workflow job for this annotation

GitHub Actions / build

PossiblyNullArgument

src/Psalm/Internal/Type/SimpleAssertionReconciler.php:2993:60: PossiblyNullArgument: Argument 1 of Psalm\Type::getLiteral cannot be null, possibly null value provided (see https://psalm.dev/078)
}

continue;
}

$enum_case = $class_storage->enum_cases[$atomic_type->const_name] ?? null;
if ($enum_case === null) {
return null;
}

$reconciled_types[] = Type::getLiteral($enum_case->value);

Check failure on line 3004 in src/Psalm/Internal/Type/SimpleAssertionReconciler.php

View workflow job for this annotation

GitHub Actions / build

PossiblyNullArgument

src/Psalm/Internal/Type/SimpleAssertionReconciler.php:3004:52: PossiblyNullArgument: Argument 1 of Psalm\Type::getLiteral cannot be null, possibly null value provided (see https://psalm.dev/078)
}

return new Union($reconciled_types);

Check failure on line 3007 in src/Psalm/Internal/Type/SimpleAssertionReconciler.php

View workflow job for this annotation

GitHub Actions / build

ArgumentTypeCoercion

src/Psalm/Internal/Type/SimpleAssertionReconciler.php:3007:26: ArgumentTypeCoercion: Argument 1 of Psalm\Type\Union::__construct expects non-empty-array<array-key, Psalm\Type\Atomic>, but parent type list{0?: Psalm\Type\Atomic\TLiteralInt|Psalm\Type\Atomic\TLiteralString, ...<Psalm\Type\Atomic\TLiteralInt|Psalm\Type\Atomic\TLiteralString>} provided (see https://psalm.dev/193)
}

/**
* @psalm-assert-if-true TCallableObject|TObjectWithProperties|TNamedObject $type
*/
Expand Down
14 changes: 14 additions & 0 deletions src/Psalm/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
use function explode;
use function get_class;
use function implode;
use function is_int;
use function preg_quote;
use function preg_replace;
use function stripos;
Expand Down Expand Up @@ -258,6 +259,19 @@
return new Union([$type]);
}

/**
* @param int|string $value
* @return TLiteralString|TLiteralInt
*/
public static function getLiteral($value)
weirdan marked this conversation as resolved.
Show resolved Hide resolved
{
if (is_int($value)) {
return new TLiteralInt($value);
}

return new TLiteralString($value);

Check failure on line 272 in src/Psalm/Type.php

View workflow job for this annotation

GitHub Actions / build

InternalMethod

src/Psalm/Type.php:272:16: InternalMethod: Constructor Psalm\Type\Atomic\TLiteralString::__construct is internal to Psalm\Type::getAtomicStringFromLiteral, Psalm\Type\Atomic\TLiteralClassString::__construct, and Psalm\Type\Atomic\TLiteralString::make but called from Psalm\Type::getLiteral (see https://psalm.dev/175)
}

public static function getString(?string $value = null): Union
{
return new Union([$value === null ? new TString() : self::getAtomicStringFromLiteral($value)]);
Expand Down
12 changes: 11 additions & 1 deletion tests/AssertAnnotationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2194,6 +2194,10 @@ function assertSomeString(string $foo): void
function assertSomeInt(int $foo): void
{}

/** @psalm-assert value-of<StringEnum|IntEnum> $foo */
function assertAnyEnumValue(string|int $foo): void
{}

/** @param "foo"|"bar" $foo */
function takesSomeStringFromEnum(string $foo): StringEnum
{
Expand All @@ -2216,8 +2220,14 @@ function takesSomeIntFromEnum(int $foo): IntEnum

assertSomeInt($int);
takesSomeIntFromEnum($int);

/** @var string|int $potentialEnumValue */
$potentialEnumValue = null;
assertAnyEnumValue($potentialEnumValue);
',
'assertions' => [],
'assertions' => [
'$potentialEnumValue===' => "'bar'|'baz'|'foo'|1|2|3",
],
'ignored_issues' => [],
'php_version' => '8.1',
],
Expand Down
15 changes: 15 additions & 0 deletions tests/EnumTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,21 @@ function withA(WithState $_): void {}
'ignored_issues' => [],
'php_version' => '8.1',
],
'backedEnumDoesNotPassNativeType' => [
'code' => '<?php
enum State: string
{
case A = "A";
case B = "B";
case C = "C";
}
function f(string $state): void {}
f(State::A);
',
'error_message' => 'InvalidArgument',
'ignored_issues' => [],
'php_version' => '8.1',
],
];
}
}