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.

Sign up for GitHub

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

Jump to bottom

Detect duplicate keys in array shapes #9177

Merged
merged 2 commits into from
Jan 25, 2023
Merged

Detect duplicate keys in array shapes #9177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
247d30f
Detect duplicate keys in array shapes
danog Jan 25, 2023
40a05d5
Improve error messages
danog Jan 25, 2023
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
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/CommentAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public static function arrayToDocblocks(
throw new DocblockParseException(
$line_parts[0] .
' is not a valid type' .
' (from ' .
' ('.$e->getMessage().' in ' .
$source->getFilePath() .
':' .
$comment->getStartLine() .
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,7 +1895,7 @@ private static function getTypeAliasesFromCommentLines(
$self_fqcln,
);
} catch (TypeParseTreeException $e) {
throw new DocblockParseException($type_string . ' is not a valid type');
throw new DocblockParseException($type_string . ' is not a valid type: '.$e->getMessage());
}

$type_alias_tokens[$type_alias] = new InlineTypeAlias($type_tokens);
Expand Down
12 changes: 8 additions & 4 deletions src/Psalm/Internal/Type/TypeParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,14 +1478,18 @@ private static function getTypeFromKeyedArrayTree(
$had_optional = true;
}

if (isset($properties[$property_key])) {
throw new TypeParseTreeException("Duplicate key $property_key detected");
}

$properties[$property_key] = $property_type;
if ($class_string) {
$class_strings[$property_key] = true;
}
}

if ($had_explicit && $had_implicit) {
throw new TypeParseTreeException('Cannot mix explicit and implicit keys!');
throw new TypeParseTreeException('Cannot mix explicit and implicit keys');
}

if ($type === 'object') {
Expand All @@ -1500,15 +1504,15 @@ private static function getTypeFromKeyedArrayTree(
}

if ($callable && !$properties) {
throw new TypeParseTreeException('A callable array cannot be empty!');
throw new TypeParseTreeException('A callable array cannot be empty');
}

if ($type !== 'array' && $type !== 'list') {
throw new TypeParseTreeException('Unexpected brace character');
}

if ($type === 'list' && !$is_list) {
throw new TypeParseTreeException('A list shape cannot describe a non-list!');
throw new TypeParseTreeException('A list shape cannot describe a non-list');
}

if (!$properties) {
Expand All @@ -1520,7 +1524,7 @@ private static function getTypeFromKeyedArrayTree(
$class_strings,
$sealed
? null
: [$is_list ? Type::getInt() : Type::getArrayKey(), Type::getMixed()],
: [$is_list ? Type::getListKey() : Type::getArrayKey(), Type::getMixed()],
$is_list,
$from_docblock,
);
Expand Down
22 changes: 14 additions & 8 deletions tests/TypeParseTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,54 +473,60 @@ public function testTKeyedCallableArrayNonList(): void

public function testTKeyedListNonList(): void
{
$this->expectExceptionMessage('A list shape cannot describe a non-list!');
$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!');
$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!');
$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!');
$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!');
$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!');
$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!');
$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!');
$this->expectExceptionMessage('Cannot mix explicit and implicit keys');
Type::parseString('array{0, test: 1}');
}

public function testTKeyedArrayNoDuplicateKeys(): void
{
$this->expectExceptionMessage('Duplicate key a detected');
Type::parseString('array{a: int, a: int}');
}

public function testSimpleCallable(): void
{
$this->assertSame(
Expand Down