From 5e9705d0cc0de652566b8208b9ab492249412df5 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Mon, 5 Dec 2022 11:54:40 +0100 Subject: [PATCH 1/4] Improve parsing of list shapes --- src/Psalm/Internal/Type/TypeParser.php | 15 +++++- tests/TypeParseTest.php | 64 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index a0045f3e898..6ea1893361d 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -1390,6 +1390,8 @@ private static function getTypeFromKeyedArrayTree( $type = $parse_tree->value; + $had_optional = false; + $is_list = true; $sealed = true; @@ -1441,7 +1443,13 @@ private static function getTypeFromKeyedArrayTree( } else { $property_key = $property_branch->value; } - $is_list = false; + if (!is_numeric($property_key) || ( + $had_optional && !$property_maybe_undefined + ) || $type === 'array' + || $type === 'callable-array' + ) { + $is_list = false; + } } else { throw new TypeParseTreeException( 'Missing property type' @@ -1458,6 +1466,7 @@ private static function getTypeFromKeyedArrayTree( if ($property_maybe_undefined) { $property_type->possibly_undefined = true; + $had_optional = true; } $properties[$property_key] = $property_type; @@ -1485,6 +1494,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/TypeParseTest.php b/tests/TypeParseTest.php index 36e04d18e80..5f1c1d88040 100644 --- a/tests/TypeParseTest.php +++ b/tests/TypeParseTest.php @@ -427,6 +427,70 @@ 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 testTKeyedListNonListOptionalWrongOrder(): void + { + $this->expectExceptionMessage('A list shape cannot describe a non-list!'); + Type::parseString('list{0?: 0, 1: 1, 2: 2}'); + } + + public function testSimpleCallable(): void { $this->assertSame( From be038ceb79fd4fdab4e252c596b4e5317c1f3b68 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Mon, 5 Dec 2022 16:43:31 +0100 Subject: [PATCH 2/4] Improve logic --- src/Psalm/Internal/Type/TypeParser.php | 32 ++++++++++++++++++-------- tests/TypeParseTest.php | 24 +++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index 6ea1893361d..0ebf944f8bd 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -1391,6 +1391,10 @@ private static function getTypeFromKeyedArrayTree( $type = $parse_tree->value; $had_optional = false; + $had_explicit = false; + $had_implicit = false; + + $previous_property_key = -1; $is_list = true; @@ -1421,7 +1425,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], @@ -1443,23 +1448,28 @@ private static function getTypeFromKeyedArrayTree( } else { $property_key = $property_branch->value; } - if (!is_numeric($property_key) || ( - $had_optional && !$property_maybe_undefined - ) || $type === 'array' - || $type === 'callable-array' + 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]); } @@ -1475,6 +1485,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); } diff --git a/tests/TypeParseTest.php b/tests/TypeParseTest.php index 5f1c1d88040..c33c2046880 100644 --- a/tests/TypeParseTest.php +++ b/tests/TypeParseTest.php @@ -491,6 +491,30 @@ public function testTKeyedListNonListOptionalWrongOrder(): void } + 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( From a816b435e3c47c7f3a8a498bc791b4750807a1c4 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Mon, 5 Dec 2022 16:45:19 +0100 Subject: [PATCH 3/4] One more test --- tests/TypeParseTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/TypeParseTest.php b/tests/TypeParseTest.php index c33c2046880..194f93c3b03 100644 --- a/tests/TypeParseTest.php +++ b/tests/TypeParseTest.php @@ -484,12 +484,18 @@ public function testTKeyedListNonListOptional(): void Type::parseString('list{a: 0, b?: 1, c?: 2}'); } - public function testTKeyedListNonListOptionalWrongOrder(): void + 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 { From e402a947b337540611f7acca0d799dd385f006c2 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Mon, 5 Dec 2022 16:47:30 +0100 Subject: [PATCH 4/4] Fix tests --- tests/FileUpdates/TemporaryUpdateTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 {