From b3f39185b2f785b107c04b9b68beb7c4d796faf5 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Wed, 31 Oct 2018 16:36:45 -0700 Subject: [PATCH] fix: Do not allow TypeCastExpressions w/o parens SequenceExpressions This enables an already existing check again which was skipped because of some other changes. Also adds another check for SequenceExpressions. --- .../babel-parser/src/parser/expression.js | 14 +++- packages/babel-parser/src/plugins/flow.js | 81 ++++++++++++------- .../babel-parser/src/plugins/typescript.js | 33 +++++++- .../flow/typecasts/fail-in-calls/input.js | 1 + .../flow/typecasts/fail-in-calls/options.json | 3 + .../typecasts/fail-without-parens/input.js | 1 + .../fail-without-parens/options.json | 3 + scripts/tests/flow/flow_tests_whitelist.txt | 1 - 8 files changed, 102 insertions(+), 35 deletions(-) create mode 100644 packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/input.js create mode 100644 packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/options.json create mode 100644 packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/input.js create mode 100644 packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/options.json diff --git a/packages/babel-parser/src/parser/expression.js b/packages/babel-parser/src/parser/expression.js index dae74bf3d614..d409752905d9 100644 --- a/packages/babel-parser/src/parser/expression.js +++ b/packages/babel-parser/src/parser/expression.js @@ -804,7 +804,7 @@ export default class ExpressionParser extends LValParser { this.state.yieldInPossibleArrowParameters = null; const params = [this.parseIdentifier()]; this.expect(tt.arrow); - // let foo = bar => {}; + // let foo = async bar => {}; this.parseArrowExpression(node, params, true); this.state.yieldInPossibleArrowParameters = oldYield; return node; @@ -1165,6 +1165,14 @@ export default class ExpressionParser extends LValParser { } } + parseExpressionParenItem( + node: N.Expression, + startPos: number, // eslint-disable-line no-unused-vars + startLoc: Position, // eslint-disable-line no-unused-vars + ): N.Expression { + return node; + } + parseParenItem( node: N.Expression, startPos: number, // eslint-disable-line no-unused-vars @@ -1812,7 +1820,7 @@ export default class ExpressionParser extends LValParser { } else if (this.match(tt.ellipsis)) { const spreadNodeStartPos = this.state.start; const spreadNodeStartLoc = this.state.startLoc; - elt = this.parseParenItem( + elt = this.parseExpressionParenItem( this.parseSpread(refShorthandDefaultPos, refNeedsArrowPos), spreadNodeStartPos, spreadNodeStartLoc, @@ -1825,7 +1833,7 @@ export default class ExpressionParser extends LValParser { elt = this.parseMaybeAssign( false, refShorthandDefaultPos, - this.parseParenItem, + this.parseExpressionParenItem, refNeedsArrowPos, ); } diff --git a/packages/babel-parser/src/plugins/flow.js b/packages/babel-parser/src/plugins/flow.js index 6382c1f6f3c2..14b0a6172d74 100644 --- a/packages/babel-parser/src/plugins/flow.js +++ b/packages/babel-parser/src/plugins/flow.js @@ -1756,12 +1756,11 @@ export default (superClass: Class): Class => return result; } - parseParenItem( + flowTryParseOptionalTypeCastExpression( node: N.Expression, startPos: number, startLoc: Position, ): N.Expression { - node = super.parseParenItem(node, startPos, startLoc); if (this.eat(tt.question)) { node.optional = true; } @@ -1777,6 +1776,36 @@ export default (superClass: Class): Class => return node; } + parseExpressionParenItem( + node: N.Expression, + startPos: number, + startLoc: Position, + ): N.Expression { + node = this.flowTryParseOptionalTypeCastExpression( + super.parseParenItem(node, startPos, startLoc), + startPos, + startLoc, + ); + + if (node.type === "TypeCastExpression") { + node._exprListItem = true; + } + + return node; + } + + parseParenItem( + node: N.Expression, + startPos: number, + startLoc: Position, + ): N.Expression { + return this.flowTryParseOptionalTypeCastExpression( + super.parseParenItem(node, startPos, startLoc), + startPos, + startLoc, + ); + } + assertModuleNodeAllowed(node: N.Node) { if ( (node.type === "ImportDeclaration" && @@ -1932,36 +1961,16 @@ export default (superClass: Class): Class => for (let i = 0; i < exprList.length; i++) { const expr = exprList[i]; if (expr && expr._exprListItem && expr.type === "TypeCastExpression") { - this.raise(expr.start, "Unexpected type cast"); + this.raise( + expr.start, + "The type cast expression is expected to be wrapped with parenthesis", + ); } } return exprList; } - // parse an item inside a expression list eg. `(NODE, NODE)` where NODE represents - // the position where this function is called - parseExprListItem( - allowEmpty: ?boolean, - refShorthandDefaultPos: ?Pos, - refNeedsArrowPos: ?Pos, - ): ?N.Expression { - const container = this.startNode(); - const node = super.parseExprListItem( - allowEmpty, - refShorthandDefaultPos, - refNeedsArrowPos, - ); - if (this.match(tt.colon)) { - container._exprListItem = true; - container.expression = node; - container.typeAnnotation = this.flowParseTypeAnnotation(); - return this.finishNode(container, "TypeCastExpression"); - } else { - return node; - } - } - checkLVal( expr: N.Expression, isBinding: ?boolean, @@ -2494,9 +2503,27 @@ export default (superClass: Class): Class => } parseParenAndDistinguishExpression(canBeArrow: boolean): N.Expression { - return super.parseParenAndDistinguishExpression( + const list = super.parseParenAndDistinguishExpression( canBeArrow && this.state.noArrowAt.indexOf(this.state.start) === -1, ); + + // Ensure that TypeCastExpressions without parens are not allowed inside SequenceExpressions + // e.g. (A, B: T) + if (list.type === "SequenceExpression") { + const firstTypeCast = list.expressions.find( + ({ type, extra }) => + type === "TypeCastExpression" && + (!extra || extra.parenthesized !== true), + ); + if (firstTypeCast) { + this.unexpected( + firstTypeCast.typeAnnotation.start, + "The type cast expression is expected to be wrapped with parenthesis", + ); + } + } + + return list; } parseSubscripts( diff --git a/packages/babel-parser/src/plugins/typescript.js b/packages/babel-parser/src/plugins/typescript.js index bd3d88d9ac1f..727bc1dd59e5 100644 --- a/packages/babel-parser/src/plugins/typescript.js +++ b/packages/babel-parser/src/plugins/typescript.js @@ -1716,14 +1716,11 @@ export default (superClass: Class): Class => } } - // Note: These "type casts" are *not* valid TS expressions. - // But we parse them here and change them when completing the arrow function. - parseParenItem( + tsTryParseParamType( node: N.Expression, startPos: number, startLoc: Position, ): N.Expression { - node = super.parseParenItem(node, startPos, startLoc); if (this.eat(tt.question)) { node.optional = true; } @@ -1742,6 +1739,34 @@ export default (superClass: Class): Class => return node; } + // Note: These "type casts" are *not* valid TS expressions. + // But we parse them here and change them when completing the arrow function. + parseExpressionParenItem( + node: N.Expression, + startPos: number, + startLoc: Position, + ): N.Expression { + return this.tsTryParseParamType( + super.parseParenItem(node, startPos, startLoc), + startPos, + startLoc, + ); + } + + // Note: These "type casts" are *not* valid TS expressions. + // But we parse them here and change them when completing the arrow function. + parseParenItem( + node: N.Expression, + startPos: number, + startLoc: Position, + ): N.Expression { + return this.tsTryParseParamType( + super.parseParenItem(node, startPos, startLoc), + startPos, + startLoc, + ); + } + parseExportDeclaration(node: N.ExportNamedDeclaration): ?N.Declaration { // "export declare" is equivalent to just "export". const isDeclare = this.eatContextual("declare"); diff --git a/packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/input.js b/packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/input.js new file mode 100644 index 000000000000..56208a86b2b8 --- /dev/null +++ b/packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/input.js @@ -0,0 +1 @@ +funccall(a, b: string); diff --git a/packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/options.json b/packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/options.json new file mode 100644 index 000000000000..7dd0478e8ee5 --- /dev/null +++ b/packages/babel-parser/test/fixtures/flow/typecasts/fail-in-calls/options.json @@ -0,0 +1,3 @@ +{ + "throws": "The type cast expression is expected to be wrapped with parenthesis (1:12)" +} diff --git a/packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/input.js b/packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/input.js new file mode 100644 index 000000000000..767b0cba53b8 --- /dev/null +++ b/packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/input.js @@ -0,0 +1 @@ +(A, B: T) diff --git a/packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/options.json b/packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/options.json new file mode 100644 index 000000000000..847e987ef0af --- /dev/null +++ b/packages/babel-parser/test/fixtures/flow/typecasts/fail-without-parens/options.json @@ -0,0 +1,3 @@ +{ + "throws": "The type cast expression is expected to be wrapped with parenthesis (1:5)" +} diff --git a/scripts/tests/flow/flow_tests_whitelist.txt b/scripts/tests/flow/flow_tests_whitelist.txt index e9f6d0bc4262..83bd8efbb415 100644 --- a/scripts/tests/flow/flow_tests_whitelist.txt +++ b/scripts/tests/flow/flow_tests_whitelist.txt @@ -31,7 +31,6 @@ types/annotations_in_comments_invalid/migrated_0003.js types/annotations/void_is_reserved_param.js types/member/reserved_words.js types/parameter_defaults/migrated_0032.js -types/typecasts_invalid/migrated_0001.js class_method_kinds/polymorphic_getter.js numbers/underscored_bin.js numbers/underscored_float.js