From 9c1ad0a9f7f8c57e02a20e9f151fce8076d80725 Mon Sep 17 00:00:00 2001 From: Guy Waldman Date: Wed, 18 Sep 2019 02:19:45 +0300 Subject: [PATCH] [parser] Disallow static fields named `constructor` (#10461) * Disallow static fields named `constructor` in a class - Disallowed static fields named `constructor` in a class (previously only non-static were disallowed) - Updated the text for the error for one consolidated warning, for both static and non-static fields - Added a test - Updated an existing test in the `flow` test suite of the parser to reflect the parse error. Strangely, the test used to pass and started failing when inlining the `isNonstaticConstructor` method, without any changes. In that test, `constructor` was a field, so in theory it should never have passed. Would appreciate some feedback on this, as I'm not 100% sure if this is somehow related to Flow * Update test262 whitelist * Add comment and fix && operator --- packages/babel-parser/src/parser/statement.js | 11 +- .../class-properties/no-ctor-2/options.json | 2 +- .../class-properties/no-ctor/options.json | 2 +- .../static-field-named-constructor/input.js | 3 + .../options.json | 4 + .../options.json | 1 + .../output.json | 169 ------------------ scripts/tests/test262/test262_whitelist.txt | 8 - 8 files changed, 18 insertions(+), 182 deletions(-) create mode 100644 packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/input.js create mode 100644 packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/options.json delete mode 100644 packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/output.json diff --git a/packages/babel-parser/src/parser/statement.js b/packages/babel-parser/src/parser/statement.js index 3e950d26578b..bb5702e7de34 100644 --- a/packages/babel-parser/src/parser/statement.js +++ b/packages/babel-parser/src/parser/statement.js @@ -1498,13 +1498,18 @@ export default class StatementParser extends ExpressionParser { } pushClassProperty(classBody: N.ClassBody, prop: N.ClassProperty) { - // This only affects properties, not methods. - if (this.isNonstaticConstructor(prop)) { + if ( + !prop.computed && + (prop.key.name === "constructor" || prop.key.value === "constructor") + ) { + // Non-computed field, which is either an identifier named "constructor" + // or a string literal named "constructor" this.raise( prop.key.start, - "Classes may not have a non-static field named 'constructor'", + "Classes may not have a field named 'constructor'", ); } + classBody.body.push(this.parseClassProperty(prop)); } diff --git a/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor-2/options.json b/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor-2/options.json index 9884529d32d7..5560d1052435 100644 --- a/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor-2/options.json +++ b/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor-2/options.json @@ -1,4 +1,4 @@ { - "throws": "Classes may not have a non-static field named 'constructor' (2:2)", + "throws": "Classes may not have a field named 'constructor' (2:2)", "plugins": ["classProperties"] } diff --git a/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor/options.json b/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor/options.json index 9884529d32d7..5560d1052435 100644 --- a/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor/options.json +++ b/packages/babel-parser/test/fixtures/experimental/class-properties/no-ctor/options.json @@ -1,4 +1,4 @@ { - "throws": "Classes may not have a non-static field named 'constructor' (2:2)", + "throws": "Classes may not have a field named 'constructor' (2:2)", "plugins": ["classProperties"] } diff --git a/packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/input.js b/packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/input.js new file mode 100644 index 000000000000..d9180dcec0cc --- /dev/null +++ b/packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/input.js @@ -0,0 +1,3 @@ +class Foo { + static constructor; +} diff --git a/packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/options.json b/packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/options.json new file mode 100644 index 000000000000..36858dc626ae --- /dev/null +++ b/packages/babel-parser/test/fixtures/experimental/class-properties/static-field-named-constructor/options.json @@ -0,0 +1,4 @@ +{ + "throws": "Classes may not have a field named 'constructor' (2:11)", + "plugins": ["classProperties"] +} diff --git a/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/options.json b/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/options.json index 3c8f72c90062..382ffd0c6970 100644 --- a/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/options.json +++ b/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/options.json @@ -1,3 +1,4 @@ { + "throws": "Classes may not have a field named 'constructor' (2:2)", "plugins": ["jsx", "flow", "classProperties"] } diff --git a/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/output.json b/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/output.json deleted file mode 100644 index 38c7d603748f..000000000000 --- a/packages/babel-parser/test/fixtures/flow/classes/constructor-override-with-class-prop-plugin/output.json +++ /dev/null @@ -1,169 +0,0 @@ -{ - "type": "File", - "start": 0, - "end": 40, - "loc": { - "start": { - "line": 1, - "column": 0 - }, - "end": { - "line": 3, - "column": 1 - } - }, - "program": { - "type": "Program", - "start": 0, - "end": 40, - "loc": { - "start": { - "line": 1, - "column": 0 - }, - "end": { - "line": 3, - "column": 1 - } - }, - "sourceType": "module", - "interpreter": null, - "body": [ - { - "type": "ClassDeclaration", - "start": 0, - "end": 40, - "loc": { - "start": { - "line": 1, - "column": 0 - }, - "end": { - "line": 3, - "column": 1 - } - }, - "id": { - "type": "Identifier", - "start": 6, - "end": 9, - "loc": { - "start": { - "line": 1, - "column": 6 - }, - "end": { - "line": 1, - "column": 9 - }, - "identifierName": "Foo" - }, - "name": "Foo" - }, - "superClass": null, - "body": { - "type": "ClassBody", - "start": 10, - "end": 40, - "loc": { - "start": { - "line": 1, - "column": 10 - }, - "end": { - "line": 3, - "column": 1 - } - }, - "body": [ - { - "type": "ClassProperty", - "start": 14, - "end": 38, - "loc": { - "start": { - "line": 2, - "column": 2 - }, - "end": { - "line": 2, - "column": 26 - } - }, - "static": false, - "key": { - "type": "Identifier", - "start": 14, - "end": 25, - "loc": { - "start": { - "line": 2, - "column": 2 - }, - "end": { - "line": 2, - "column": 13 - }, - "identifierName": "constructor" - }, - "name": "constructor" - }, - "computed": false, - "variance": null, - "typeAnnotation": { - "type": "TypeAnnotation", - "start": 25, - "end": 37, - "loc": { - "start": { - "line": 2, - "column": 13 - }, - "end": { - "line": 2, - "column": 25 - } - }, - "typeAnnotation": { - "type": "FunctionTypeAnnotation", - "start": 27, - "end": 37, - "loc": { - "start": { - "line": 2, - "column": 15 - }, - "end": { - "line": 2, - "column": 25 - } - }, - "params": [], - "rest": null, - "returnType": { - "type": "ThisTypeAnnotation", - "start": 33, - "end": 37, - "loc": { - "start": { - "line": 2, - "column": 21 - }, - "end": { - "line": 2, - "column": 25 - } - } - }, - "typeParameters": null - } - }, - "value": null - } - ] - } - } - ], - "directives": [] - } -} \ No newline at end of file diff --git a/scripts/tests/test262/test262_whitelist.txt b/scripts/tests/test262/test262_whitelist.txt index d3579a6eccef..01c45f11a59d 100644 --- a/scripts/tests/test262/test262_whitelist.txt +++ b/scripts/tests/test262/test262_whitelist.txt @@ -12,10 +12,6 @@ language/expressions/async-arrow-function/early-errors-arrow-await-in-formals.js language/expressions/async-arrow-function/early-errors-arrow-await-in-formals.js(strict mode) language/expressions/class/elements/fields-duplicate-privatenames.js(default) language/expressions/class/elements/fields-duplicate-privatenames.js(strict mode) -language/expressions/class/elements/fields-literal-name-static-propname-constructor.js(default) -language/expressions/class/elements/fields-literal-name-static-propname-constructor.js(strict mode) -language/expressions/class/elements/fields-string-name-static-propname-constructor.js(default) -language/expressions/class/elements/fields-string-name-static-propname-constructor.js(strict mode) language/expressions/class/elements/private-methods/prod-private-method-initialize-order.js(default) language/expressions/class/elements/private-methods/prod-private-method-initialize-order.js(strict mode) language/expressions/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-chained-usage.js(default) @@ -188,10 +184,6 @@ language/module-code/top-level-await/syntax/try-await-expr.js(default) language/module-code/top-level-await/syntax/try-await-expr.js(strict mode) language/statements/class/elements/fields-duplicate-privatenames.js(default) language/statements/class/elements/fields-duplicate-privatenames.js(strict mode) -language/statements/class/elements/fields-literal-name-static-propname-constructor.js(default) -language/statements/class/elements/fields-literal-name-static-propname-constructor.js(strict mode) -language/statements/class/elements/fields-string-name-static-propname-constructor.js(default) -language/statements/class/elements/fields-string-name-static-propname-constructor.js(strict mode) language/statements/class/elements/private-field-is-visible-in-computed-properties.js(default) language/statements/class/elements/private-field-is-visible-in-computed-properties.js(strict mode) language/statements/class/elements/private-field-with-initialized-id-is-visible-in-computed-properties.js(default)