From 1f28d025c388b421b9bf7c665e8a0cc878590fe1 Mon Sep 17 00:00:00 2001 From: Patrick Remy Date: Thu, 20 Jan 2022 22:41:33 +0100 Subject: [PATCH] feat: allow unions for key-of/value-of Add tests for TValueOfArray. --- .../Type/Comparator/ScalarTypeComparator.php | 3 +- .../Type/TemplateInferredTypeReplacer.php | 1 - src/Psalm/Internal/Type/TypeExpander.php | 52 +++-- src/Psalm/Internal/Type/TypeParser.php | 23 +- src/Psalm/Type/Atomic/TKeyOfArray.php | 29 ++- src/Psalm/Type/Atomic/TValueOfArray.php | 22 +- tests/ArrayKeysTest.php | 18 +- tests/KeyOfArrayTest.php | 58 +++-- tests/Template/KeyOfTemplateTest.php | 33 ++- tests/ValueOfArrayTest.php | 200 ++++++++++++++++++ 10 files changed, 334 insertions(+), 105 deletions(-) create mode 100644 tests/ValueOfArrayTest.php diff --git a/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php b/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php index e96b7d559..3e87325cf 100644 --- a/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php @@ -295,8 +295,7 @@ class ScalarTypeComparator if ($container_type_part instanceof TArrayKey && ($input_type_part instanceof TInt - || $input_type_part instanceof TString - || $input_type_part instanceof TTemplateKeyOf) + || $input_type_part instanceof TString) ) { return true; } diff --git a/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php b/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php index 956d1861c..40f6130ee 100644 --- a/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php +++ b/src/Psalm/Internal/Type/TemplateInferredTypeReplacer.php @@ -239,7 +239,6 @@ class TemplateInferredTypeReplacer : null; if ($template_type) { - $template_type = $template_type->getSingleAtomic(); if (TKeyOfArray::isViableTemplateType($template_type)) { $keys_to_unset[] = $key; $new_types[] = new TKeyOfArray(clone $template_type); diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php index 5b6827738..e80b2a394 100644 --- a/src/Psalm/Internal/Type/TypeExpander.php +++ b/src/Psalm/Internal/Type/TypeExpander.php @@ -887,9 +887,14 @@ class TypeExpander bool $evaluate_class_constants, bool $throw_on_unresolvable_constant ): array { - $type_param = $return_type->type; + // Expand class constants to their atomics + $type_atomics = []; + foreach ($return_type->type->getAtomicTypes() as $type_param) { + if (!$evaluate_class_constants || !$type_param instanceof TClassConstant) { + array_push($type_atomics, $type_param); + continue; + } - if ($evaluate_class_constants && $type_param instanceof TClassConstant) { if ($type_param->fq_classlike_name === 'self' && $self_class) { $type_param->fq_classlike_name = $self_class; } @@ -900,8 +905,9 @@ class TypeExpander throw new UnresolvableConstantException($type_param->fq_classlike_name, $type_param->const_name); } + $constant_type = null; try { - $type_param = $codebase->classlikes->getClassConstantType( + $constant_type = $codebase->classlikes->getClassConstantType( $type_param->fq_classlike_name, $type_param->const_name, ReflectionProperty::IS_PRIVATE @@ -909,38 +915,44 @@ class TypeExpander } catch (CircularReferenceException $e) { return [$return_type]; } - if (!$type_param) { + + if (!$constant_type + || ( + $return_type instanceof TKeyOfArray + && !TKeyOfArray::isViableTemplateType($constant_type) + ) + || ( + $return_type instanceof TValueOfArray + && !TValueOfArray::isViableTemplateType($constant_type) + ) + ) { if ($throw_on_unresolvable_constant) { - throw new UnresolvableConstantException($return_type->type->fq_classlike_name, $return_type->type->const_name); + throw new UnresolvableConstantException($type_param->fq_classlike_name, $type_param->const_name); } else { return [$return_type]; } } + + $type_atomics = array_merge( + $type_atomics, + $constant_type->getAtomicTypes() + ); } - if (!$type_param instanceof Union) { - $type_param = new Union([$type_param]); - } + // Types inside union are checked on instantiation and above on + // expansion, currently there is no Union for typing here. + /** @var list $type_atomics */ // Merge keys/values of provided array types $new_return_types = []; - foreach ($type_param->getAtomicTypes() as $type_atomic) { - // Abort if any type of the param's union is invalid - if (!$type_atomic instanceof TKeyedArray - && !$type_atomic instanceof TArray - && !$type_atomic instanceof TList - ) { - break; - } - + foreach ($type_atomics as $type_atomic) { // Transform all types to TArray if needed if ($type_atomic instanceof TList) { $type_atomic = new TArray([ new Union([new TInt()]), $type_atomic->type_param ]); - } - if ($type_atomic instanceof TKeyedArray) { + } elseif ($type_atomic instanceof TKeyedArray) { $type_atomic = $type_atomic->getGenericArrayType(); } @@ -958,7 +970,7 @@ class TypeExpander } } - if (empty($new_return_types)) { + if ($new_return_types === []) { return [$return_type]; } return $new_return_types; diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index 3740c904c..a8116a9bd 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -696,40 +696,25 @@ class TypeParser ); } - $param_union_types = array_values($generic_params[0]->getAtomicTypes()); - - if (count($param_union_types) > 1) { - throw new TypeParseTreeException('Union types are not allowed in key-of type'); - } - - if (!TKeyOfArray::isViableTemplateType($param_union_types[0])) { + if (!TKeyOfArray::isViableTemplateType($generic_params[0])) { throw new TypeParseTreeException( 'Untemplated key-of param ' . $param_name . ' should be a class constant or an array' ); } - return new TKeyOfArray($param_union_types[0]); + return new TKeyOfArray($generic_params[0]); } if ($generic_type_value === 'value-of') { $param_name = $generic_params[0]->getId(false); - $param_union_types = array_values($generic_params[0]->getAtomicTypes()); - - if (count($param_union_types) > 1) { - throw new TypeParseTreeException('Union types are not allowed in value-of type'); - } - - if (!$param_union_types[0] instanceof TArray - && !$param_union_types[0] instanceof TList - && !$param_union_types[0] instanceof TKeyedArray - && !$param_union_types[0] instanceof TClassConstant) { + if (!TValueOfArray::isViableTemplateType($generic_params[0])) { throw new TypeParseTreeException( 'Untemplated value-of param ' . $param_name . ' should be a class constant or an array' ); } - return new TValueOfArray($param_union_types[0]); + return new TValueOfArray($generic_params[0]); } if ($generic_type_value === 'int-mask') { diff --git a/src/Psalm/Type/Atomic/TKeyOfArray.php b/src/Psalm/Type/Atomic/TKeyOfArray.php index 93cd98de1..b4f59c70a 100644 --- a/src/Psalm/Type/Atomic/TKeyOfArray.php +++ b/src/Psalm/Type/Atomic/TKeyOfArray.php @@ -3,21 +3,17 @@ namespace Psalm\Type\Atomic; use Psalm\Type\Atomic; +use Psalm\Type\Union; /** * Represents an offset of an array. - * - * @psalm-type ArrayLikeTemplateType = TClassConstant|TKeyedArray|TList|TArray */ class TKeyOfArray extends TArrayKey { - /** @var ArrayLikeTemplateType */ + /** @var Union */ public $type; - /** - * @param ArrayLikeTemplateType $type - */ - public function __construct(Atomic $type) + public function __construct(Union $type) { $this->type = $type; } @@ -49,14 +45,17 @@ class TKeyOfArray extends TArrayKey return 'mixed'; } - /** - * @psalm-assert-if-true ArrayLikeTemplateType $template_type - */ - public static function isViableTemplateType(Atomic $template_type): bool + public static function isViableTemplateType(Union $template_type): bool { - return $template_type instanceof TArray - || $template_type instanceof TClassConstant - || $template_type instanceof TKeyedArray - || $template_type instanceof TList; + foreach ($template_type->getAtomicTypes() as $type) { + if (!$type instanceof TArray + && !$type instanceof TClassConstant + && !$type instanceof TKeyedArray + && !$type instanceof TList + ) { + return false; + } + } + return true; } } diff --git a/src/Psalm/Type/Atomic/TValueOfArray.php b/src/Psalm/Type/Atomic/TValueOfArray.php index ad76824e2..d8f3480ce 100644 --- a/src/Psalm/Type/Atomic/TValueOfArray.php +++ b/src/Psalm/Type/Atomic/TValueOfArray.php @@ -3,19 +3,17 @@ namespace Psalm\Type\Atomic; use Psalm\Type\Atomic; +use Psalm\Type\Union; /** * Represents a value of an array. */ class TValueOfArray extends Atomic { - /** @var TClassConstant|TKeyedArray|TList|TArray */ + /** @var Union */ public $type; - /** - * @param TClassConstant|TKeyedArray|TList|TArray $type - */ - public function __construct(Atomic $type) + public function __construct(Union $type) { $this->type = $type; } @@ -46,4 +44,18 @@ class TValueOfArray extends Atomic { return 'mixed'; } + + public static function isViableTemplateType(Union $template_type): bool + { + foreach ($template_type->getAtomicTypes() as $type) { + if (!$type instanceof TArray + && !$type instanceof TClassConstant + && !$type instanceof TKeyedArray + && !$type instanceof TList + ) { + return false; + } + } + return true; + } } diff --git a/tests/ArrayKeysTest.php b/tests/ArrayKeysTest.php index 39545743d..0e2185e4c 100644 --- a/tests/ArrayKeysTest.php +++ b/tests/ArrayKeysTest.php @@ -28,7 +28,7 @@ class ArrayKeysTest extends TestCase ], 'arrayKeysOfKeyedArrayReturnsNonEmptyListOfStrings' => [ 'code' => ' \'bar\']); + $keys = array_keys(["foo" => "bar"]); ', 'assertions' => [ '$keys' => 'non-empty-list', @@ -36,7 +36,7 @@ class ArrayKeysTest extends TestCase ], 'arrayKeysOfListReturnsNonEmptyListOfInts' => [ 'code' => ' [ '$keys' => 'non-empty-list', @@ -44,7 +44,7 @@ class ArrayKeysTest extends TestCase ], 'arrayKeysOfKeyedStringIntArrayReturnsNonEmptyListOfIntsOrStrings' => [ 'code' => ' \'bar\', 42]); + $keys = array_keys(["foo" => "bar", 42]); ', 'assertions' => [ '$keys' => 'non-empty-list', @@ -63,10 +63,10 @@ class ArrayKeysTest extends TestCase 'arrayKeysOfKeyedArrayConformsToCorrectLiteralStringList' => [ 'code' => ' + * @return non-empty-list<"foo"|"bar"> */ function getKeys() { - return array_keys([\'foo\' => 42, \'bar\' => 42]); + return array_keys(["foo" => 42, "bar" => 42]); } ' ], @@ -76,7 +76,7 @@ class ArrayKeysTest extends TestCase * @return non-empty-list<0|1> */ function getKeys() { - return array_keys([\'foo\', \'bar\']); + return array_keys(["foo", "bar"]); } ' ], @@ -86,7 +86,7 @@ class ArrayKeysTest extends TestCase * @return 0|1 */ function getKey() { - return array_key_first([\'foo\', \'bar\']); + return array_key_first(["foo", "bar"]); } ' ], @@ -96,7 +96,7 @@ class ArrayKeysTest extends TestCase * @return 0|1 */ function getKey() { - return array_key_last([\'foo\', \'bar\']); + return array_key_last(["foo", "bar"]); } ' ], @@ -127,7 +127,7 @@ class ArrayKeysTest extends TestCase * @return list */ function getKeys() { - return array_keys([\'foo\' => 42, \'bar\' => 42]); + return array_keys(["foo" => 42, "bar" => 42]); } ', 'error_message' => 'InvalidReturnStatement' diff --git a/tests/KeyOfArrayTest.php b/tests/KeyOfArrayTest.php index 3893e9e49..e69f94055 100644 --- a/tests/KeyOfArrayTest.php +++ b/tests/KeyOfArrayTest.php @@ -22,7 +22,7 @@ class KeyOfArrayTest extends TestCase 'code' => ' */ public function getKey() { @@ -35,11 +35,11 @@ class KeyOfArrayTest extends TestCase 'code' => ' 42 + "bar" => 42 ]; /** @return key-of */ public function getKey() { - return \'bar\'; + return "bar"; } } ' @@ -48,15 +48,15 @@ class KeyOfArrayTest extends TestCase 'code' => ' 42, - \'adams\' => 43, + "bar" => 42, + "adams" => 43, ]; /** @return key-of */ public function getKey(bool $adams) { if ($adams) { - return \'adams\'; + return "adams"; } - return \'bar\'; + return "bar"; } } ' @@ -66,11 +66,11 @@ class KeyOfArrayTest extends TestCase class A { /** @var array */ const FOO = [ - \'bar\' => 42, - \'adams\' => 43, + "bar" => 42, + "adams" => 43, ]; /** @return key-of[] */ - public function getKey(bool $adams) { + public function getKey() { return array_keys(self::FOO); } } @@ -99,6 +99,19 @@ class KeyOfArrayTest extends TestCase } ' ], + 'keyOfUnionListAndKeyedArray' => [ + 'code' => '|array{a: int, b: int}> + */ + function getKey(bool $asInt) { + if ($asInt) { + return 42; + } + return "a"; + } + ', + ], 'keyOfListArrayLiteral' => [ 'code' => '>[] */ - $keys2 = [\'foo\']; + $keys2 = ["foo"]; return $keys2[0]; } ' @@ -134,11 +147,11 @@ class KeyOfArrayTest extends TestCase 'code' => ' 42 + "bar" => 42 ]; /** @return key-of */ - public function getKey(bool $adams) { - return \'adams\'; + public function getKey() { + return "adams"; } } ', @@ -151,7 +164,7 @@ class KeyOfArrayTest extends TestCase * @return key-of> */ public function getKey() { - return \'foo\'; + return "foo"; } } ', @@ -164,7 +177,7 @@ class KeyOfArrayTest extends TestCase * @return key-of> */ public function getKey() { - return \'42\'; + return "42"; } } ', @@ -179,7 +192,18 @@ class KeyOfArrayTest extends TestCase if ($asFloat) { return 42.0; } - return \'42\'; + return "42"; + } + ', + 'error_message' => 'InvalidReturnStatement' + ], + 'noLiteralCAllowedInKeyOfUnionListAndKeyedArray' => [ + 'code' => '|array{a: int, b: int}> + */ + function getKey() { + return "c"; } ', 'error_message' => 'InvalidReturnStatement' diff --git a/tests/Template/KeyOfTemplateTest.php b/tests/Template/KeyOfTemplateTest.php index b856ff1da..9aec719e4 100644 --- a/tests/Template/KeyOfTemplateTest.php +++ b/tests/Template/KeyOfTemplateTest.php @@ -54,22 +54,21 @@ class KeyOfTemplateTest extends TestCase } ' ], - // Currently not works! - // 'acceptsIfArrayKeyExistsFn' => [ - // 'code' => '|null - // */ - // function getKey(string $key, $array) { - // if (array_key_exists($key, $array)) { - // return $key; - // } - // return null; - // } - // ' - // ], + 'SKIP-acceptsIfArrayKeyExistsFn' => [ + 'code' => '|null + */ + function getKey(string $key, $array) { + if (array_key_exists($key, $array)) { + return $key; + } + return null; + } + ' + ], ]; } @@ -87,7 +86,7 @@ class KeyOfTemplateTest extends TestCase * @return key-of */ function getKey($array) { - return \'foo\'; + return "foo"; } ', 'error_message' => 'InvalidReturnStatement' diff --git a/tests/ValueOfArrayTest.php b/tests/ValueOfArrayTest.php new file mode 100644 index 000000000..e04c70569 --- /dev/null +++ b/tests/ValueOfArrayTest.php @@ -0,0 +1,200 @@ +,ignored_issues?:list,php_version?:string}> + */ + public function providerValidCodeParse(): iterable + { + return [ + 'valueOfListClassConstant' => [ + 'code' => ' */ + public function getKey() { + return "bar"; + } + } + ' + ], + 'valueOfAssociativeArrayClassConstant' => [ + 'code' => ' 42 + ]; + /** @return value-of */ + public function getValue() { + return 42; + } + } + ' + ], + 'allValuesOfAssociativeArrayPossible' => [ + 'code' => ' 42, + "adams" => 43, + ]; + /** @return value-of */ + public function getValue(bool $adams) { + if ($adams) { + return 42; + } + return 43; + } + } + ' + ], + 'valueOfAsArray' => [ + 'code' => ' 42, + "adams" => 43, + ]; + /** @return value-of[] */ + public function getValues() { + return array_values(self::FOO); + } + } + ' + ], + 'valueOfArrayLiteral' => [ + 'code' => '> + */ + function getKey() { + return "42"; + } + ' + ], + 'valueOfUnionArrayLiteral' => [ + 'code' => '|array> + */ + function getValue(bool $asFloat) { + if ($asFloat) { + return 42.0; + } + return 42; + } + ' + ], + 'valueOfStringArrayConformsToString' => [ + 'code' => '>[] */ + $keys2 = ["foo"]; + return $keys2[0]; + } + ' + ], + 'acceptLiteralIntInValueOfUnionLiteralInts' => [ + 'code' => '|array{0: 3, 1: 4}> + */ + function getValue(int $i) { + if ($i >= 0 && $i <= 4) { + return $i; + } + return 0; + } + ', + ], + ]; + } + + /** + * @return iterable,php_version?:string}> + */ + public function providerInvalidCodeParse(): iterable + { + return [ + 'onlyDefinedValuesOfConstantList' => [ + 'code' => ' */ + public function getValue() { + return "adams"; + } + } + ', + 'error_message' => 'InvalidReturnStatement' + ], + 'noIntForValueOfStringArrayLiteral' => [ + 'code' => '> + */ + public function getValue() { + return 42; + } + } + ', + 'error_message' => 'InvalidReturnStatement' + ], + 'noStringForValueOfIntList' => [ + 'code' => '> + */ + public function getValue() { + return "42"; + } + } + ', + 'error_message' => 'InvalidReturnStatement' + ], + 'noOtherStringAllowedForValueOfKeyedArray' => [ + 'code' => ' + */ + function getValue() { + return "adams"; + } + ', + 'error_message' => 'InvalidReturnStatement' + ], + 'noOtherIntAllowedInValueOfUnionLiteralInts' => [ + 'code' => '|array{0: 3, 1: 4}> + */ + function getValue() { + return 5; + } + ', + 'error_message' => 'InvalidReturnStatement' + ], + ]; + } +}