From 027354cabc1a6fbaf89de02680377b44788b5d14 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Tue, 13 Dec 2022 22:30:40 +0100 Subject: [PATCH] Improve parsing of list shapes (#8841) * Improve parsing of list shapes * Improve logic * One more test * Fix tests --- src/Psalm/Internal/Type/TypeParser.php | 39 ++++++++-- tests/FileUpdates/TemporaryUpdateTest.php | 2 +- tests/TypeParseTest.php | 94 +++++++++++++++++++++++ 3 files changed, 128 insertions(+), 7 deletions(-) diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index 867aa2e41ed..d12055a587b 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -1391,6 +1391,12 @@ private static function getTypeFromKeyedArrayTree( $type = $parse_tree->value; + $had_optional = false; + $had_explicit = false; + $had_implicit = false; + + $previous_property_key = -1; + $is_list = true; $sealed = true; @@ -1420,7 +1426,8 @@ private static function getTypeFromKeyedArrayTree( $from_docblock ); $property_maybe_undefined = false; - $property_key = (string)$i; + $property_key = $i; + $had_implicit = true; } elseif (count($property_branch->children) === 1) { $property_type = self::getTypeFromTree( $property_branch->children[0], @@ -1442,23 +1449,35 @@ private static function getTypeFromKeyedArrayTree( } else { $property_key = $property_branch->value; } - $is_list = false; + if ($is_list && ( + !is_numeric($property_key) + || ($had_optional && !$property_maybe_undefined) + || $type === 'array' + || $type === 'callable-array' + || $previous_property_key != ($property_key-1) + ) + ) { + $is_list = false; + } + $had_explicit = true; + $previous_property_key = $property_key; + + if ($property_key[0] === '\'' || $property_key[0] === '"') { + $property_key = stripslashes(substr($property_key, 1, -1)); + } } else { throw new TypeParseTreeException( 'Missing property type' ); } - if ($property_key[0] === '\'' || $property_key[0] === '"') { - $property_key = stripslashes(substr($property_key, 1, -1)); - } - if (!$property_type instanceof Union) { $property_type = new Union([$property_type], ['from_docblock' => $from_docblock]); } if ($property_maybe_undefined) { $property_type->possibly_undefined = true; + $had_optional = true; } $properties[$property_key] = $property_type; @@ -1467,6 +1486,10 @@ private static function getTypeFromKeyedArrayTree( } } + if ($had_explicit && $had_implicit) { + throw new TypeParseTreeException('Cannot mix explicit and implicit keys!'); + } + if ($type === 'object') { return new TObjectWithProperties($properties, [], [], $from_docblock); } @@ -1486,6 +1509,10 @@ private static function getTypeFromKeyedArrayTree( throw new TypeParseTreeException('Unexpected brace character'); } + if ($type === 'list' && !$is_list) { + throw new TypeParseTreeException('A list shape cannot describe a non-list!'); + } + if (!$properties) { return new TArray([Type::getNever($from_docblock), Type::getNever($from_docblock)], $from_docblock); } diff --git a/tests/FileUpdates/TemporaryUpdateTest.php b/tests/FileUpdates/TemporaryUpdateTest.php index 950d03521dd..24d3d2a6370 100644 --- a/tests/FileUpdates/TemporaryUpdateTest.php +++ b/tests/FileUpdates/TemporaryUpdateTest.php @@ -162,7 +162,7 @@ public function testErrorFix( } /** - * @return array>,error_positions:array>, ignored_issues?:array, test_save?:bool, check_unused_code?: bool}> + * @return array>,error_positions:array>, ignored_issues?:array, test_save?:bool, check_unused_code?: bool}> */ public function providerTestErrorFix(): array { diff --git a/tests/TypeParseTest.php b/tests/TypeParseTest.php index 36e04d18e80..194f93c3b03 100644 --- a/tests/TypeParseTest.php +++ b/tests/TypeParseTest.php @@ -427,6 +427,100 @@ public function testTKeyedArrayNotSealed(): void ); } + public function testTKeyedList(): void + { + $this->assertSame( + 'list{int, int, string}', + (string)Type::parseString('list{int, int, string}') + ); + } + + public function testTKeyedListOptional(): void + { + $this->assertSame( + 'list{0: int, 1?: int, 2?: string}', + (string)Type::parseString('list{0: int, 1?: int, 2?: string}') + ); + } + + + public function testTKeyedArrayList(): void + { + $this->assertSame( + 'list{int, int, string}', + (string)Type::parseString('array{int, int, string}') + ); + } + + + public function testTKeyedArrayNonList(): void + { + $this->assertSame( + 'array{0: int, 1: int, 2: string}', + (string)Type::parseString('array{0: int, 1: int, 2: string}') + ); + } + + + public function testTKeyedCallableArrayNonList(): void + { + $this->assertSame( + 'callable-array{0: class-string, 1: string}', + (string)Type::parseString('callable-array{0: class-string, 1: string}') + ); + } + + + public function testTKeyedListNonList(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{a: 0, b: 1, c: 2}'); + } + + + public function testTKeyedListNonListOptional(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{a: 0, b?: 1, c?: 2}'); + } + + public function testTKeyedListNonListOptionalWrongOrder1(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{0?: 0, 1: 1, 2: 2}'); + } + + public function testTKeyedListNonListOptionalWrongOrder2(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{0: 0, 1?: 1, 2: 2}'); + } + + + public function testTKeyedListWrongOrder(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{1: 1, 0: 0}'); + } + + public function testTKeyedListNonListKeys(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{1: 1, 2: 2}'); + } + + public function testTKeyedListNoExplicitAndImplicitKeys(): void + { + $this->expectExceptionMessage('Cannot mix explicit and implicit keys!'); + Type::parseString('list{0: 0, 1}'); + } + + public function testTKeyedArrayNoExplicitAndImplicitKeys(): void + { + $this->expectExceptionMessage('Cannot mix explicit and implicit keys!'); + Type::parseString('array{0, test: 1}'); + } + public function testSimpleCallable(): void { $this->assertSame(