fix handling of reserved keywords when parsing (sveltejs#4390)

jesseskinner · Feb 27, 2020 · 4152175 · 4152175
1 parent 526f350
commit 4152175
Show file tree

Hide file tree

Showing 35 changed files with 333 additions and 94 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@
 * Improve parsing error messages when there is a pending unclosed tag ([#4131](https://github.com/sveltejs/svelte/issues/4131))
 * Disallow attribute/prop names from matching two-way-bound names or `{shorthand}` attribute/prop names ([#4325](https://github.com/sveltejs/svelte/issues/4325))
 * Improve performance of `flush()` by not using `.shift()` ([#4356](https://github.com/sveltejs/svelte/pull/4356))
+* Permit reserved keywords as destructuring keys in `{#each}` ([#4372](https://github.com/sveltejs/svelte/issues/4372))
+* Disallow reserved keywords in `{expressions}` ([#4372](https://github.com/sveltejs/svelte/issues/4372))
 * Fix code generation error with precedence of arrow functions ([#4384](https://github.com/sveltejs/svelte/issues/4384))
 
 ## 3.18.1

diff --git a/src/compiler/parse/index.ts b/src/compiler/parse/index.ts
@@ -141,7 +141,7 @@ export class Parser {
 		return result;
 	}
 
-	read_identifier() {
+	read_identifier(allow_reserved = false) {
 		const start = this.index;
 
 		let i = this.index;
@@ -160,7 +160,7 @@ export class Parser {
 
 		const identifier = this.template.slice(this.index, this.index = i);
 
-		if (reserved.has(identifier)) {
+		if (!allow_reserved && reserved.has(identifier)) {
 			this.error({
 				code: `unexpected-reserved-word`,
 				message: `'${identifier}' is a reserved word in JavaScript and cannot be used here`

diff --git a/src/compiler/parse/read/context.ts b/src/compiler/parse/read/context.ts
@@ -1,4 +1,5 @@
 import { Parser } from '../index';
+import { reserved } from '../../utils/names';
 
 interface Identifier {
 	start: number;
@@ -116,8 +117,11 @@ export default function read_context(parser: Parser) {
 				break;
 			}
 
+			// TODO: DRY this out somehow
+			// We don't know whether we want to allow reserved words until we see whether there's a ':' after it
+			// Probably ideally we'd use Acorn to do all of this
 			const start = parser.index;
-			const name = parser.read_identifier();
+			const name = parser.read_identifier(true);
 			const key: Identifier = {
 				start,
 				end: parser.index,
@@ -126,9 +130,19 @@ export default function read_context(parser: Parser) {
 			};
 			parser.allow_whitespace();
 
-			const value = parser.eat(':')
-				? (parser.allow_whitespace(), read_context(parser))
-				: key;
+			let value: Context;
+			if (parser.eat(':')) {
+				parser.allow_whitespace();
+				value = read_context(parser);
+			} else {
+				if (reserved.has(name)) {
+					parser.error({
+						code: `unexpected-reserved-word`,
+						message: `'${name}' is a reserved word in JavaScript and cannot be used here`
+					}, start);
+				}
+				value = key;
+			}
 
 			const property: Property = {
 				start,

diff --git a/src/compiler/parse/read/expression.ts b/src/compiler/parse/read/expression.ts
@@ -1,37 +1,9 @@
 import { parse_expression_at } from '../acorn';
 import { Parser } from '../index';
-import { Identifier, Node, SimpleLiteral } from 'estree';
+import { Node } from 'estree';
 import { whitespace } from '../../utils/patterns';
 
-const literals = new Map([['true', true], ['false', false], ['null', null]]);
-
 export default function read_expression(parser: Parser): Node {
-	const start = parser.index;
-
-	const name = parser.read_until(/\s*}/);
-	if (name && /^[a-z]+$/.test(name)) {
-		const end = start + name.length;
-
-		if (literals.has(name)) {
-			return {
-				type: 'Literal',
-				start,
-				end,
-				value: literals.get(name),
-				raw: name,
-			} as SimpleLiteral;
-		}
-
-		return {
-			type: 'Identifier',
-			start,
-			end: start + name.length,
-			name,
-		} as Identifier;
-	}
-
-	parser.index = start;
-
 	try {
 		const node = parse_expression_at(parser.template, parser.index);
 

diff --git a/test/parser/samples/action-with-identifier/output.json b/test/parser/samples/action-with-identifier/output.json
@@ -20,6 +20,16 @@
 							"type": "Identifier",
 							"start": 20,
 							"end": 27,
+							"loc": {
+								"start": {
+									"line": 1,
+									"column": 20
+								},
+								"end": {
+									"line": 1,
+									"column": 27
+								}
+							},
 							"name": "message"
 						}
 					}

diff --git a/test/parser/samples/attribute-dynamic-boolean/output.json b/test/parser/samples/attribute-dynamic-boolean/output.json
@@ -24,6 +24,16 @@
 									"type": "Identifier",
 									"start": 20,
 									"end": 28,
+									"loc": {
+										"start": {
+											"line": 1,
+											"column": 20
+										},
+										"end": {
+											"line": 1,
+											"column": 28
+										}
+									},
 									"name": "readonly"
 								}
 							}

diff --git a/test/parser/samples/attribute-dynamic-reserved/input.svelte b/test/parser/samples/attribute-dynamic-reserved/input.svelte
diff --git a/test/parser/samples/attribute-dynamic-reserved/output.json b/test/parser/samples/attribute-dynamic-reserved/output.json
diff --git a/test/parser/samples/attribute-dynamic/output.json b/test/parser/samples/attribute-dynamic/output.json
@@ -31,6 +31,16 @@
 									"type": "Identifier",
 									"start": 20,
 									"end": 25,
+									"loc": {
+										"start": {
+											"line": 1,
+											"column": 20
+										},
+										"end": {
+											"line": 1,
+											"column": 25
+										}
+									},
 									"name": "color"
 								}
 							},
@@ -53,6 +63,16 @@
 							"type": "Identifier",
 							"start": 30,
 							"end": 35,
+							"loc": {
+								"start": {
+									"line": 1,
+									"column": 30
+								},
+								"end": {
+									"line": 1,
+									"column": 35
+								}
+							},
 							"name": "color"
 						}
 					}

diff --git a/test/parser/samples/attribute-with-whitespace/output.json b/test/parser/samples/attribute-with-whitespace/output.json
@@ -20,6 +20,16 @@
 							"type": "Identifier",
 							"start": 19,
 							"end": 22,
+							"loc": {
+								"start": {
+									"line": 1,
+									"column": 19
+								},
+								"end": {
+									"line": 1,
+									"column": 22
+								}
+							},
 							"name": "foo"
 						}
 					}

diff --git a/test/parser/samples/binding/output.json b/test/parser/samples/binding/output.json
@@ -27,6 +27,16 @@
 							"type": "Identifier",
 							"start": 50,
 							"end": 54,
+							"loc": {
+								"start": {
+									"line": 5,
+									"column": 19
+								},
+								"end": {
+									"line": 5,
+									"column": 23
+								}
+							},
 							"name": "name"
 						}
 					}

diff --git a/test/parser/samples/each-block-destructured/output.json b/test/parser/samples/each-block-destructured/output.json
@@ -40,6 +40,16 @@
 									"type": "Identifier",
 									"start": 46,
 									"end": 49,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 5
+										},
+										"end": {
+											"line": 2,
+											"column": 8
+										}
+									},
 									"name": "key"
 								}
 							},
@@ -58,6 +68,16 @@
 									"type": "Identifier",
 									"start": 53,
 									"end": 58,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 12
+										},
+										"end": {
+											"line": 2,
+											"column": 17
+										}
+									},
 									"name": "value"
 								}
 							}

diff --git a/test/parser/samples/each-block-else/output.json b/test/parser/samples/each-block-else/output.json
@@ -40,6 +40,16 @@
 									"type": "Identifier",
 									"start": 31,
 									"end": 37,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 5
+										},
+										"end": {
+											"line": 2,
+											"column": 11
+										}
+									},
 									"name": "animal"
 								}
 							}

diff --git a/test/parser/samples/each-block-indexed/output.json b/test/parser/samples/each-block-indexed/output.json
@@ -40,6 +40,16 @@
 									"type": "Identifier",
 									"start": 34,
 									"end": 35,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 5
+										},
+										"end": {
+											"line": 2,
+											"column": 6
+										}
+									},
 									"name": "i"
 								}
 							},
@@ -58,6 +68,16 @@
 									"type": "Identifier",
 									"start": 39,
 									"end": 45,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 10
+										},
+										"end": {
+											"line": 2,
+											"column": 16
+										}
+									},
 									"name": "animal"
 								}
 							}

diff --git a/test/parser/samples/each-block-keyed/output.json b/test/parser/samples/each-block-keyed/output.json
@@ -40,6 +40,16 @@
 									"type": "Identifier",
 									"start": 37,
 									"end": 41,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 5
+										},
+										"end": {
+											"line": 2,
+											"column": 9
+										}
+									},
 									"name": "todo"
 								}
 							}

diff --git a/test/parser/samples/each-block/output.json b/test/parser/samples/each-block/output.json
@@ -40,6 +40,16 @@
 									"type": "Identifier",
 									"start": 31,
 									"end": 37,
+									"loc": {
+										"start": {
+											"line": 2,
+											"column": 5
+										},
+										"end": {
+											"line": 2,
+											"column": 11
+										}
+									},
 									"name": "animal"
 								}
 							}

diff --git a/test/parser/samples/element-with-mustache/output.json b/test/parser/samples/element-with-mustache/output.json
@@ -26,6 +26,16 @@
 							"type": "Identifier",
 							"start": 11,
 							"end": 15,
+							"loc": {
+								"start": {
+									"line": 1,
+									"column": 11
+								},
+								"end": {
+									"line": 1,
+									"column": 15
+								}
+							},
 							"name": "name"
 						}
 					},