Skip to content

Commit 816ec34

Browse files
TysonAndreTysonAndre
committed
Fixes #189 : Fix parsing edge cases in yield statements
Update tests diagnostics and expected generated ASTs For backwards compatibility, continue to make `yield from expr` have an ArrayElement as a child node. To maintain the invariant that all Nodes must have at least one child, make $yieldExpression->arrayElement be null when parsing `yield;` Aside: `yield &$a;` is (and should be) parsed as the binary operator `(yield) & ($a)`. This is surprising, but makes sense, being the only sane parse tree. - Add a unit test that `yield & &$a;` is invalid. There is no way to parse that. Verified with the PHP module nikic/php-ast ```php var_export(ast_dump( ast\parse_code('<?php function test() { yield & $x; }', 50) )); ```
1 parent faca5af commit 816ec34

28 files changed

+414
-108
lines changed

‎docs/ApiDocumentation.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public function getEndPosition ( )
226226
> TODO: add doc comment
227227
228228
```php
229-
public static function getTokenKindNameFromValue ( $kindName )
229+
public static function getTokenKindNameFromValue ( $kind )
230230
```
231231
### Token::jsonSerialize
232232
> TODO: add doc comment

‎src/Parser.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2098,7 +2098,23 @@ private function parseYieldExpression($parentNode) {
20982098
TokenKind::YieldFromKeyword,
20992099
TokenKind::YieldKeyword
21002100
);
2101-
$yieldExpression->arrayElement = $this->parseArrayElement($yieldExpression);
2101+
if ($yieldExpression->yieldOrYieldFromKeyword->kind === TokenKind::YieldFromKeyword) {
2102+
// Don't use parseArrayElement. E.g. `yield from &$varName` or `yield from $key => $varName` are both syntax errors
2103+
$arrayElement = new ArrayElement();
2104+
$arrayElement->parent = $yieldExpression;
2105+
$arrayElement->elementValue = $this->parseExpression($arrayElement);
2106+
$yieldExpression->arrayElement = $arrayElement;
2107+
} else {
2108+
// This is always an ArrayElement for backwards compatibilitiy.
2109+
// TODO: Can this be changed to a non-ArrayElement in a future release?
2110+
if ($this->isExpressionStart($this->getCurrentToken())) {
2111+
// Both `yield expr;` and `yield;` are possible.
2112+
$yieldExpression->arrayElement = $this->parseArrayElement($yieldExpression);
2113+
} else {
2114+
$yieldExpression->arrayElement = null;
2115+
}
2116+
}
2117+
21022118
return $yieldExpression;
21032119
}
21042120

‎tests/cases/parser/yieldExpression10.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

3-
// TODO `return yield` should fail
3+
// `return yield;` is valid code in PHP 7 (invalid in PHP 5),
4+
// since generators can also return values. It is parsed as `return (yield);`
45
function gen() {
56
return yield;
67
}

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1 @@
1-
[
2-
{
3-
"kind": 0,
4-
"message": "'Expression' expected.",
5-
"start": 75,
6-
"length": 0
7-
}
8-
]
1+
[]

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,7 @@
5353
"kind": "YieldKeyword",
5454
"textLength": 5
5555
},
56-
"arrayElement": {
57-
"ArrayElement": {
58-
"elementKey": null,
59-
"arrowToken": null,
60-
"byRef": null,
61-
"elementValue": {
62-
"error": "MissingToken",
63-
"kind": "Expression",
64-
"textLength": 0
65-
}
66-
}
67-
}
56+
"arrayElement": null
6857
}
6958
},
7059
"semicolon": {

‎tests/cases/parser/yieldExpression10_from.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
// TODO should fail
3+
// This fails with "'Expression' expected."
44
function gen() {
55
return yield from;
66
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"kind": 0,
44
"message": "'Expression' expected.",
5-
"start": 65,
5+
"start": 89,
66
"length": 0
77
}
88
]

‎tests/cases/parser/yieldExpression11.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

3-
// should fail
3+
// should not fail.
4+
// This is parsed as the binary bitwise and operator (yield) & ($a);
45
function gen() {
56
yield &$a;
67
}

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

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,27 +44,26 @@
4444
{
4545
"ExpressionStatement": {
4646
"expression": {
47-
"YieldExpression": {
48-
"yieldOrYieldFromKeyword": {
49-
"kind": "YieldKeyword",
50-
"textLength": 5
51-
},
52-
"arrayElement": {
53-
"ArrayElement": {
54-
"elementKey": null,
55-
"arrowToken": null,
56-
"byRef": {
57-
"kind": "AmpersandToken",
58-
"textLength": 1
47+
"BinaryExpression": {
48+
"leftOperand": {
49+
"YieldExpression": {
50+
"yieldOrYieldFromKeyword": {
51+
"kind": "YieldKeyword",
52+
"textLength": 5
5953
},
60-
"elementValue": {
61-
"Variable": {
62-
"dollar": null,
63-
"name": {
64-
"kind": "VariableName",
65-
"textLength": 2
66-
}
67-
}
54+
"arrayElement": null
55+
}
56+
},
57+
"operator": {
58+
"kind": "AmpersandToken",
59+
"textLength": 1
60+
},
61+
"rightOperand": {
62+
"Variable": {
63+
"dollar": null,
64+
"name": {
65+
"kind": "VariableName",
66+
"textLength": 2
6867
}
6968
}
7069
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,8 @@
1-
[]
1+
[
2+
{
3+
"kind": 0,
4+
"message": "'Expression' expected.",
5+
"start": 53,
6+
"length": 0
7+
}
8+
]

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

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,26 @@
5353
"ArrayElement": {
5454
"elementKey": null,
5555
"arrowToken": null,
56-
"byRef": {
57-
"kind": "AmpersandToken",
58-
"textLength": 1
59-
},
56+
"byRef": null,
6057
"elementValue": {
61-
"Variable": {
62-
"dollar": null,
63-
"name": {
64-
"kind": "VariableName",
65-
"textLength": 2
58+
"BinaryExpression": {
59+
"leftOperand": {
60+
"error": "MissingToken",
61+
"kind": "Expression",
62+
"textLength": 0
63+
},
64+
"operator": {
65+
"kind": "AmpersandToken",
66+
"textLength": 1
67+
},
68+
"rightOperand": {
69+
"Variable": {
70+
"dollar": null,
71+
"name": {
72+
"kind": "VariableName",
73+
"textLength": 2
74+
}
75+
}
6676
}
6777
}
6878
}

‎tests/cases/parser/yieldExpression18.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
3+
// This is invalid. But (yield) & ($x) would be valid in PHP 7.
4+
function example($x) {
5+
yield & &$x;
6+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[
2+
{
3+
"kind": 0,
4+
"message": "'Expression' expected.",
5+
"start": 105,
6+
"length": 0
7+
}
8+
]

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

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
{
2+
"SourceFileNode": {
3+
"statementList": [
4+
{
5+
"InlineHtml": {
6+
"scriptSectionEndTag": null,
7+
"text": null,
8+
"scriptSectionStartTag": {
9+
"kind": "ScriptSectionStartTag",
10+
"textLength": 6
11+
}
12+
}
13+
},
14+
{
15+
"FunctionDeclaration": {
16+
"functionKeyword": {
17+
"kind": "FunctionKeyword",
18+
"textLength": 8
19+
},
20+
"byRefToken": null,
21+
"name": {
22+
"kind": "Name",
23+
"textLength": 7
24+
},
25+
"openParen": {
26+
"kind": "OpenParenToken",
27+
"textLength": 1
28+
},
29+
"parameters": {
30+
"ParameterDeclarationList": {
31+
"children": [
32+
{
33+
"Parameter": {
34+
"questionToken": null,
35+
"typeDeclaration": null,
36+
"byRefToken": null,
37+
"dotDotDotToken": null,
38+
"variableName": {
39+
"kind": "VariableName",
40+
"textLength": 2
41+
},
42+
"equalsToken": null,
43+
"default": null
44+
}
45+
}
46+
]
47+
}
48+
},
49+
"closeParen": {
50+
"kind": "CloseParenToken",
51+
"textLength": 1
52+
},
53+
"colonToken": null,
54+
"questionToken": null,
55+
"returnType": null,
56+
"compoundStatementOrSemicolon": {
57+
"CompoundStatementNode": {
58+
"openBrace": {
59+
"kind": "OpenBraceToken",
60+
"textLength": 1
61+
},
62+
"statements": [
63+
{
64+
"ExpressionStatement": {
65+
"expression": {
66+
"BinaryExpression": {
67+
"leftOperand": {
68+
"BinaryExpression": {
69+
"leftOperand": {
70+
"YieldExpression": {
71+
"yieldOrYieldFromKeyword": {
72+
"kind": "YieldKeyword",
73+
"textLength": 5
74+
},
75+
"arrayElement": null
76+
}
77+
},
78+
"operator": {
79+
"kind": "AmpersandToken",
80+
"textLength": 1
81+
},
82+
"rightOperand": {
83+
"error": "MissingToken",
84+
"kind": "Expression",
85+
"textLength": 0
86+
}
87+
}
88+
},
89+
"operator": {
90+
"kind": "AmpersandToken",
91+
"textLength": 1
92+
},
93+
"rightOperand": {
94+
"Variable": {
95+
"dollar": null,
96+
"name": {
97+
"kind": "VariableName",
98+
"textLength": 2
99+
}
100+
}
101+
}
102+
}
103+
},
104+
"semicolon": {
105+
"kind": "SemicolonToken",
106+
"textLength": 1
107+
}
108+
}
109+
}
110+
],
111+
"closeBrace": {
112+
"kind": "CloseBraceToken",
113+
"textLength": 1
114+
}
115+
}
116+
}
117+
}
118+
}
119+
],
120+
"endOfFileToken": {
121+
"kind": "EndOfFileToken",
122+
"textLength": 0
123+
}
124+
}
125+
}

‎tests/cases/parser/yieldExpression19.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
3+
// This is parsed as (yield) && ($x).
4+
function example($x) {
5+
yield &&$x;
6+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[]

0 commit comments

Comments
 (0)