Skip to content

Commit 81790cd

Browse files
TysonAndreTysonAndre
committed
Stricten error checking in parsing union types/intersection types
1 parent 18d7801 commit 81790cd

19 files changed

+1078
-93
lines changed

‎src/Parser.php‎

Lines changed: 73 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -893,45 +893,22 @@ private function parseAndSetReturnTypeDeclarationList($parentNode) {
893893
/**
894894
* Attempt to parse the return type after the `:` and optional `?` token.
895895
*
896+
* TODO: Consider changing the return type to a new class TypeList in a future major release?
897+
* ParenthesizedIntersectionType is not a qualified name.
896898
* @return DelimitedList\QualifiedNameList|null
897899
*/
898900
private function parseReturnTypeDeclarationList($parentNode) {
899-
// TODO: Forbid mixing `|` and `&` in another PR, that's a parse error.
900-
// TODO: Forbid mixing (A&B)&C in another PR, that's a parse error.
901-
$result = $this->parseDelimitedList(
902-
DelimitedList\QualifiedNameList::class,
903-
self::TYPE_DELIMITER_TOKENS,
904-
function ($token) {
901+
return $this->parseUnionTypeDeclarationList(
902+
$parentNode,
903+
function ($token): bool {
905904
return \in_array($token->kind, $this->returnTypeDeclarationTokens, true) ||
906-
$token->kind === TokenKind::OpenParenToken ||
907905
$this->isQualifiedNameStart($token);
908906
},
909907
function ($parentNode) {
910-
$openParen = $this->eatOptional(TokenKind::OpenParenToken);
911-
if ($openParen) {
912-
return $this->parseParenthesizedIntersectionType(
913-
$parentNode,
914-
$openParen,
915-
function ($token) {
916-
return \in_array($token->kind, $this->returnTypeDeclarationTokens, true) ||
917-
$this->isQualifiedNameStart($token);
918-
},
919-
function ($parentNode) {
920-
return $this->parseReturnTypeDeclaration($parentNode);
921-
}
922-
);
923-
}
924908
return $this->parseReturnTypeDeclaration($parentNode);
925909
},
926-
$parentNode,
927-
false);
928-
929-
// Add a MissingToken so that this will warn about `function () : T| {}`
930-
// TODO: Make this a reusable abstraction?
931-
if ($result && in_array(end($result->children)->kind ?? null, self::TYPE_DELIMITER_TOKENS)) {
932-
$result->children[] = new MissingToken(TokenKind::ReturnType, $this->token->fullStart);
933-
}
934-
return $result;
910+
TokenKind::ReturnType
911+
);
935912
}
936913

937914
private function parseReturnTypeDeclaration($parentNode) {
@@ -945,6 +922,68 @@ private function tryParseParameterTypeDeclaration($parentNode) {
945922
return $parameterTypeDeclaration;
946923
}
947924

925+
/**
926+
* Parse a union type such as A, A|B, A&B, A|(B&C), rejecting invalid syntax combinations.
927+
*
928+
* @param Node $parentNode
929+
* @param Closure(Token):bool $isTypeStart
930+
* @param Closure(Node):(Node|Token|null) $parseType
931+
* @param int $expectedTypeKind expected kind for token type
932+
* @return DelimitedList\QualifiedNameList|null
933+
*/
934+
private function parseUnionTypeDeclarationList($parentNode, Closure $isTypeStart, Closure $parseType, int $expectedTypeKind) {
935+
$result = new DelimitedList\QualifiedNameList();
936+
$token = $this->getCurrentToken();
937+
$delimiter = self::TYPE_DELIMITER_TOKENS;
938+
do {
939+
if ($token->kind === TokenKind::OpenParenToken || $isTypeStart($token)) {
940+
// Forbid mixing A&(B&C) if '&' was already seen
941+
$openParen = in_array(TokenKind::BarToken, $delimiter, true)
942+
? $this->eatOptional(TokenKind::OpenParenToken)
943+
: null;
944+
if ($openParen) {
945+
$element = $this->parseParenthesizedIntersectionType($parentNode, $openParen, $isTypeStart, $parseType);
946+
// Forbid mixing (A&B)&C by forbidding `&` separator after a parenthesized intersection type.
947+
$delimiter = [TokenKind::BarToken];
948+
} else {
949+
$element = $parseType($parentNode);
950+
}
951+
$result->addElement($element);
952+
} else {
953+
break;
954+
}
955+
956+
$delimiterToken = $this->eatOptional($delimiter);
957+
if ($delimiterToken !== null) {
958+
$result->addElement($delimiterToken);
959+
$delimiter = [$delimiterToken->kind];
960+
}
961+
$token = $this->getCurrentToken();
962+
// TODO ERROR CASE - no delimiter, but a param follows
963+
} while ($delimiterToken !== null);
964+
965+
$result->parent = $parentNode;
966+
if ($result->children === null) {
967+
return null;
968+
}
969+
970+
// Add a MissingToken so that this will warn about `function () : T| {}`
971+
// TODO: Make this a reusable abstraction?
972+
if (in_array(end($result->children)->kind ?? null, $delimiter, true)) {
973+
$result->children[] = new MissingToken($expectedTypeKind, $this->token->fullStart);
974+
} elseif (count($result->children) === 1 && $result->children[0] instanceof ParenthesizedIntersectionType) {
975+
// dnf types with parenthesized intersection types are a union type of at least 2 types.
976+
$result->children[] = new MissingToken(TokenKind::BarToken, $this->token->fullStart);
977+
}
978+
return $result;
979+
}
980+
981+
/**
982+
* @param Node $parentNode
983+
* @param Token $openParen
984+
* @param Closure(Token):bool $isTypeStart
985+
* @param Closure(Node):(Node|Token|null) $parseType
986+
*/
948987
private function parseParenthesizedIntersectionType($parentNode, Token $openParen, Closure $isTypeStart, Closure $parseType): ParenthesizedIntersectionType {
949988
$node = new ParenthesizedIntersectionType();
950989
$node->parent = $parentNode;
@@ -978,40 +1017,17 @@ private function parseParenthesizedIntersectionType($parentNode, Token $openPare
9781017
* @return DelimitedList\QualifiedNameList|null
9791018
*/
9801019
private function tryParseParameterTypeDeclarationList($parentNode) {
981-
$result = $this->parseDelimitedList(
982-
DelimitedList\QualifiedNameList::class,
983-
self::TYPE_DELIMITER_TOKENS,
1020+
return $this->parseUnionTypeDeclarationList(
1021+
$parentNode,
9841022
function ($token) {
9851023
return \in_array($token->kind, $this->parameterTypeDeclarationTokens, true) ||
986-
$token->kind === TokenKind::OpenParenToken ||
9871024
$this->isQualifiedNameStart($token);
9881025
},
9891026
function ($parentNode) {
990-
$openParen = $this->eatOptional(TokenKind::OpenParenToken);
991-
if ($openParen) {
992-
return $this->parseParenthesizedIntersectionType(
993-
$parentNode,
994-
$openParen,
995-
function ($token) {
996-
return \in_array($token->kind, $this->parameterTypeDeclarationTokens, true) ||
997-
$this->isQualifiedNameStart($token);
998-
},
999-
function ($parentNode) {
1000-
return $this->tryParseParameterTypeDeclaration($parentNode);
1001-
}
1002-
);
1003-
}
10041027
return $this->tryParseParameterTypeDeclaration($parentNode);
10051028
},
1006-
$parentNode,
1007-
true);
1008-
1009-
// Add a MissingToken so that this will Warn about `function (T| $x) {}`
1010-
// TODO: Make this a reusable abstraction?
1011-
if ($result && in_array(end($result->children)->kind ?? null, self::TYPE_DELIMITER_TOKENS)) {
1012-
$result->children[] = new MissingToken(TokenKind::Name, $this->token->fullStart);
1013-
}
1014-
return $result;
1029+
TokenKind::Name
1030+
);
10151031
}
10161032

10171033
private function parseCompoundStatement($parentNode) {

‎tests/cases/parser/dnfTypesParameter3.php.diag‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,11 @@
44
"message": "'Name' expected.",
55
"start": 21,
66
"length": 0
7+
},
8+
{
9+
"kind": 0,
10+
"message": "'|' expected.",
11+
"start": 22,
12+
"length": 0
713
}
814
]

‎tests/cases/parser/dnfTypesParameter3.php.tree‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@
5555
"textLength": 1
5656
}
5757
}
58+
},
59+
{
60+
"error": "MissingToken",
61+
"kind": "BarToken",
62+
"textLength": 0
5863
}
5964
]
6065
}

‎tests/cases/parser/dnfTypesParameter4.php.diag‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@
1111
"start": 20,
1212
"length": 0
1313
},
14+
{
15+
"kind": 0,
16+
"message": "'|' expected.",
17+
"start": 20,
18+
"length": 0
19+
},
1420
{
1521
"kind": 0,
1622
"message": "'VariableName' expected.",

‎tests/cases/parser/dnfTypesParameter4.php.tree‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@
5656
"textLength": 0
5757
}
5858
}
59+
},
60+
{
61+
"error": "MissingToken",
62+
"kind": "BarToken",
63+
"textLength": 0
5964
}
6065
]
6166
}

‎tests/cases/parser/dnfTypesParameter5.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
// dnf types must be of the form (A&B)|C, not &C.
3+
function example((A&B)&C $param) {}

‎tests/cases/parser/dnfTypesParameter5.php.diag‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
[
2+
{
3+
"kind": 0,
4+
"message": "'|' expected.",
5+
"start": 78,
6+
"length": 0
7+
},
8+
{
9+
"kind": 0,
10+
"message": "'VariableName' expected.",
11+
"start": 79,
12+
"length": 0
13+
},
14+
{
15+
"kind": 0,
16+
"message": "')' expected.",
17+
"start": 79,
18+
"length": 0
19+
},
20+
{
21+
"kind": 0,
22+
"message": "'{' expected.",
23+
"start": 79,
24+
"length": 0
25+
},
26+
{
27+
"kind": 0,
28+
"message": "';' expected.",
29+
"start": 80,
30+
"length": 0
31+
},
32+
{
33+
"kind": 0,
34+
"message": "';' expected.",
35+
"start": 87,
36+
"length": 0
37+
},
38+
{
39+
"kind": 0,
40+
"message": "Unexpected ')'",
41+
"start": 87,
42+
"length": 1
43+
},
44+
{
45+
"kind": 0,
46+
"message": "'}' expected.",
47+
"start": 91,
48+
"length": 0
49+
}
50+
]

0 commit comments

Comments
 (0)