Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(parser): remove refNeedsArrowPos #13419

Merged
merged 13 commits into from Jun 19, 2021
Merged

refactor(parser): remove refNeedsArrowPos #13419

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9bb8274
fix(parser:expression): remove refNeedsArrowPos in parseExprListItem
tony-go Jun 3, 2021
6b29e13
feat: use refExpressionErrors to catch optional parameters
tony-go Jun 11, 2021
b443d89
feat: delete useless refNeedArrowPos
tony-go Jun 11, 2021
3fa08d2
fix: forgotten deletion
tony-go Jun 11, 2021
2ad5050
fix: lint and factorize
tony-go Jun 11, 2021
1950d7d
fix: delete useless comment
tony-go Jun 11, 2021
0a35f8a
fix: review corrections
tony-go Jun 11, 2021
cbbc10a
fix: review corrections pt2
tony-go Jun 11, 2021
9666004
fix: review correction pt2
tony-go Jun 16, 2021
da283bf
fix: nit correction
tony-go Jun 18, 2021
63be4e1
fix: update types
tony-go Jun 18, 2021
fe20d8c
Update packages/babel-parser/src/parser/util.js
nicolo-ribaudo Jun 18, 2021
6d9321e
fix: add invariant comment in flow and ts file
tony-go Jun 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 25 additions & 38 deletions packages/babel-parser/src/parser/expression.js
View file
Open in desktop
Expand Up @@ -30,7 +30,6 @@ import {
isIdentifierStart,
canBeReservedWord,
} from "../util/identifier";
import type { Pos } from "../util/location";
import { Position } from "../util/location";
import * as charCodes from "charcodes";
import {
Expand All @@ -57,6 +56,7 @@ import {
newExpressionScope,
} from "../util/expression-scope";
import { Errors, SourceTypeModuleErrors } from "./error";
import type { ParsingError } from "./error";

/*::
import type { SourceType } from "../options";
Expand Down Expand Up @@ -222,40 +222,38 @@ export default class ExpressionParser extends LValParser {
parseMaybeAssignDisallowIn(
refExpressionErrors?: ?ExpressionErrors,
afterLeftParse?: Function,
refNeedsArrowPos?: ?Pos,
) {
return this.disallowInAnd(() =>
this.parseMaybeAssign(
refExpressionErrors,
afterLeftParse,
refNeedsArrowPos,
),
this.parseMaybeAssign(refExpressionErrors, afterLeftParse),
);
}

// Set [+In] parameter for assignment expression
parseMaybeAssignAllowIn(
refExpressionErrors?: ?ExpressionErrors,
afterLeftParse?: Function,
refNeedsArrowPos?: ?Pos,
) {
return this.allowInAnd(() =>
this.parseMaybeAssign(
refExpressionErrors,
afterLeftParse,
refNeedsArrowPos,
),
this.parseMaybeAssign(refExpressionErrors, afterLeftParse),
);
}

// This method is only used by
// the typescript and flow plugins.
setOptionalParametersError(
tony-go marked this conversation as resolved.
Show resolved Hide resolved
refExpressionErrors: ExpressionErrors,
resultError?: ?ParsingError,
tony-go marked this conversation as resolved.
Show resolved Hide resolved
) {
refExpressionErrors.optionalParameters =
resultError?.pos || this.state.start;
tony-go marked this conversation as resolved.
Show resolved Hide resolved
}

// Parse an assignment expression. This includes applications of
// operators like `+=`.

// https://tc39.es/ecma262/#prod-AssignmentExpression
parseMaybeAssign(
refExpressionErrors?: ?ExpressionErrors,
afterLeftParse?: Function,
refNeedsArrowPos?: ?Pos,
): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
Expand Down Expand Up @@ -285,10 +283,7 @@ export default class ExpressionParser extends LValParser {
this.state.potentialArrowAt = this.state.start;
}

let left = this.parseMaybeConditional(
refExpressionErrors,
refNeedsArrowPos,
);
let left = this.parseMaybeConditional(refExpressionErrors);
if (afterLeftParse) {
left = afterLeftParse.call(this, left, startPos, startLoc);
}
Expand Down Expand Up @@ -323,10 +318,7 @@ export default class ExpressionParser extends LValParser {
// Parse a ternary conditional (`?:`) operator.
// https://tc39.es/ecma262/#prod-ConditionalExpression

parseMaybeConditional(
refExpressionErrors: ExpressionErrors,
refNeedsArrowPos?: ?Pos,
): N.Expression {
parseMaybeConditional(refExpressionErrors: ExpressionErrors): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
const potentialArrowAt = this.state.potentialArrowAt;
Expand All @@ -336,16 +328,15 @@ export default class ExpressionParser extends LValParser {
return expr;
}

return this.parseConditional(expr, startPos, startLoc, refNeedsArrowPos);
return this.parseConditional(expr, startPos, startLoc, refExpressionErrors);
}

parseConditional(
expr: N.Expression,
startPos: number,
startLoc: Position,
// FIXME: Disabling this for now since can't seem to get it to play nicely
// eslint-disable-next-line no-unused-vars
refNeedsArrowPos?: ?Pos,
refExpressionErrors?: ?ExpressionErrors,
): N.Expression {
if (this.eat(tt.question)) {
const node = this.startNodeAt(startPos, startLoc);
Expand Down Expand Up @@ -935,12 +926,7 @@ export default class ExpressionParser extends LValParser {
}

elts.push(
this.parseExprListItem(
false,
refExpressionErrors,
{ start: 0 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: refNeedsArrowPos is reset here. Do we need to reset refExpressionErrors.optionalParameters to -1? Maybe worth a test covering this.

allowPlaceholder,
),
this.parseExprListItem(false, refExpressionErrors, allowPlaceholder),
);
}

Expand Down Expand Up @@ -1466,7 +1452,6 @@ export default class ExpressionParser extends LValParser {
const innerStartLoc = this.state.startLoc;
const exprList = [];
const refExpressionErrors = new ExpressionErrors();
const refNeedsArrowPos = { start: 0 };
let first = true;
let spreadStart;
let optionalCommaStart;
Expand All @@ -1475,7 +1460,12 @@ export default class ExpressionParser extends LValParser {
if (first) {
first = false;
} else {
this.expect(tt.comma, refNeedsArrowPos.start || null);
this.expect(
tt.comma,
refExpressionErrors.optionalParameters === -1
? null
: refExpressionErrors.optionalParameters,
);
if (this.match(tt.parenR)) {
optionalCommaStart = this.state.start;
break;
Expand All @@ -1502,7 +1492,6 @@ export default class ExpressionParser extends LValParser {
this.parseMaybeAssignAllowIn(
refExpressionErrors,
this.parseParenItem,
refNeedsArrowPos,
),
);
}
Expand Down Expand Up @@ -1534,7 +1523,6 @@ export default class ExpressionParser extends LValParser {
if (optionalCommaStart) this.unexpected(optionalCommaStart);
if (spreadStart) this.unexpected(spreadStart);
this.checkExpressionErrors(refExpressionErrors, true);
if (refNeedsArrowPos.start) this.unexpected(refNeedsArrowPos.start);

this.toReferencedListDeep(exprList, /* isParenthesizedExpr */ true);
if (exprList.length > 1) {
Expand Down Expand Up @@ -2296,7 +2284,6 @@ export default class ExpressionParser extends LValParser {
parseExprListItem(
allowEmpty: ?boolean,
refExpressionErrors?: ?ExpressionErrors,
refNeedsArrowPos: ?Pos,
allowPlaceholder: ?boolean,
): ?N.Expression {
let elt;
Expand All @@ -2308,6 +2295,7 @@ export default class ExpressionParser extends LValParser {
} else if (this.match(tt.ellipsis)) {
const spreadNodeStartPos = this.state.start;
const spreadNodeStartLoc = this.state.startLoc;
const refNeedsArrowPos = { start: 0 };
tony-go marked this conversation as resolved.
Show resolved Hide resolved
elt = this.parseParenItem(
this.parseSpread(refExpressionErrors, refNeedsArrowPos),
spreadNodeStartPos,
Expand All @@ -2325,7 +2313,6 @@ export default class ExpressionParser extends LValParser {
elt = this.parseMaybeAssignAllowIn(
refExpressionErrors,
this.parseParenItem,
refNeedsArrowPos,
);
}
return elt;
Expand Down
39 changes: 25 additions & 14 deletions packages/babel-parser/src/parser/util.js
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@ import ProductionParameterHandler, {
PARAM,
} from "../util/production-parameter";
import { Errors, type ErrorTemplate, ErrorCodes } from "./error";
import type { ParsingError } from "./error";
/*::
import type ScopeHandler from "../util/scope";
*/
Expand Down Expand Up @@ -175,9 +176,9 @@ export default class UtilParser extends Tokenizer {
template: `Unexpected token, expected "${messageOrType.label}"`,
};
}

/* eslint-disable @babel/development-internal/dry-error-messages */
throw this.raise(pos != null ? pos : this.state.start, messageOrType);
/* eslint-enable @babel/development-internal/dry-error-messages */
tony-go marked this conversation as resolved.
Show resolved Hide resolved
}

expectPlugin(name: string, pos?: ?number): true {
Expand Down Expand Up @@ -211,7 +212,7 @@ export default class UtilParser extends Tokenizer {
oldState: State = this.state.clone(),
):
| TryParse<T, null, false, false, null>
| TryParse<T | null, SyntaxError, boolean, false, State>
tony-go marked this conversation as resolved.
Show resolved Hide resolved
| TryParse<T | null, ParsingError, boolean, false, State>
| TryParse<T | null, null, false, true, State> {
const abortSignal: { node: T | null } = { node: null };
try {
Expand All @@ -228,7 +229,7 @@ export default class UtilParser extends Tokenizer {
this.state.tokensLength = failState.tokensLength;
return {
node,
error: (failState.errors[oldState.errors.length]: SyntaxError),
error: (failState.errors[oldState.errors.length]: ParsingError),
thrown: false,
aborted: false,
failState,
Expand Down Expand Up @@ -267,14 +268,22 @@ export default class UtilParser extends Tokenizer {
andThrow: boolean,
) {
if (!refExpressionErrors) return false;
const { shorthandAssign, doubleProto } = refExpressionErrors;
if (!andThrow) return shorthandAssign >= 0 || doubleProto >= 0;
const { shorthandAssign, doubleProto, optionalParameters } =
refExpressionErrors;
if (!andThrow) {
return (
shorthandAssign >= 0 || doubleProto >= 0 || optionalParameters >= 0
);
}
if (shorthandAssign >= 0) {
this.unexpected(shorthandAssign);
}
if (doubleProto >= 0) {
this.raise(doubleProto, Errors.DuplicateProto);
}
if (optionalParameters >= 0) {
this.unexpected(optionalParameters);
}
}

/**
Expand Down Expand Up @@ -394,17 +403,19 @@ export default class UtilParser extends Tokenizer {
}

/**
* The ExpressionErrors is a context struct used to track
* - **shorthandAssign**: track initializer `=` position when parsing ambiguous
* patterns. When we are sure the parsed pattern is a RHS, which means it is
* not a pattern, we will throw on this position on invalid assign syntax,
* otherwise it will be reset to -1
* - **doubleProto**: track the duplicate `__proto__` key position when parsing
* ambiguous object patterns. When we are sure the parsed pattern is a RHS,
* which means it is an object literal, we will throw on this position for
* __proto__ redefinition, otherwise it will be reset to -1
* The ExpressionErrors is a context struct used to track ambiguous patterns
* When we are sure the parsed pattern is a RHS, which means it is not a pattern,
* we will throw on this position on invalid assign syntax, otherwise it will be reset to -1
*
* Types of ExpressionErrors:
*
* - **shorthandAssign**: track initializer `=` position
* - **doubleProto**: track the duplicate `__proto__` key position
* - **optionalParameters**: track the optional paramter (`?`).
* It's only used by typescript and flow plugins
*/
export class ExpressionErrors {
shorthandAssign = -1;
doubleProto = -1;
optionalParameters = -1;
tony-go marked this conversation as resolved.
Show resolved Hide resolved
}
33 changes: 12 additions & 21 deletions packages/babel-parser/src/plugins/flow/index.js
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@
import type Parser from "../../parser";
import { types as tt, type TokenType } from "../../tokenizer/types";
import * as N from "../../types";
import type { Pos, Position } from "../../util/location";
import type { Position } from "../../util/location";
import { types as tc } from "../../tokenizer/context";
import * as charCodes from "charcodes";
import { isIteratorStart, isKeyword } from "../../util/identifier";
Expand Down Expand Up @@ -1910,20 +1910,22 @@ export default (superClass: Class<Parser>): Class<Parser> =>
expr: N.Expression,
startPos: number,
startLoc: Position,
refNeedsArrowPos?: ?Pos,
refExpressionErrors?: ?ExpressionErrors,
): N.Expression {
if (!this.match(tt.question)) return expr;

// only use the expensive "tryParse" method if there is a question mark
// and if we come from inside parens
if (refNeedsArrowPos) {
if (this.state.maybeInArrowParameters) {
const result = this.tryParse(() =>
super.parseConditional(expr, startPos, startLoc),
);

if (!result.node) {
// $FlowIgnore
refNeedsArrowPos.start = result.error.pos || this.state.start;
if (refExpressionErrors) {
tony-go marked this conversation as resolved.
Show resolved Hide resolved
super.setOptionalParametersError(refExpressionErrors, result.error);
}

return expr;
}

Expand Down Expand Up @@ -1977,7 +1979,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
node.test = expr;
node.consequent = consequent;
node.alternate = this.forwardNoArrowParamsConversionAt(node, () =>
this.parseMaybeAssign(undefined, undefined, undefined),
this.parseMaybeAssign(undefined, undefined),
);

return this.finishNode(node, "ConditionalExpression");
Expand Down Expand Up @@ -2824,7 +2826,6 @@ export default (superClass: Class<Parser>): Class<Parser> =>
parseMaybeAssign(
refExpressionErrors?: ?ExpressionErrors,
afterLeftParse?: Function,
refNeedsArrowPos?: ?Pos,
): N.Expression {
let state = null;

Expand All @@ -2837,17 +2838,12 @@ export default (superClass: Class<Parser>): Class<Parser> =>
state = this.state.clone();

jsx = this.tryParse(
() =>
super.parseMaybeAssign(
refExpressionErrors,
afterLeftParse,
refNeedsArrowPos,
),
() => super.parseMaybeAssign(refExpressionErrors, afterLeftParse),
state,
);
/*:: invariant(!jsx.aborted) */

if (!jsx.error) return jsx.node;
if (!jsx.error && jsx.node) return jsx.node;
tony-go marked this conversation as resolved.
Show resolved Hide resolved

// Remove `tc.j_expr` and `tc.j_oTag` from context added
// by parsing `jsxTagStart` to stop the JSX plugin from
Expand All @@ -2874,7 +2870,6 @@ export default (superClass: Class<Parser>): Class<Parser> =>
const result = super.parseMaybeAssign(
refExpressionErrors,
afterLeftParse,
refNeedsArrowPos,
);

this.resetStartLocationFromNode(result, typeParameters);
Expand Down Expand Up @@ -2954,11 +2949,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
);
}

return super.parseMaybeAssign(
refExpressionErrors,
afterLeftParse,
refNeedsArrowPos,
);
return super.parseMaybeAssign(refExpressionErrors, afterLeftParse);
}

// handle return types for arrow functions
Expand Down Expand Up @@ -3072,7 +3063,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
state,
);

if (!arrow.error && !arrow.aborted) return arrow.node;
if (!arrow.error && !arrow.aborted && arrow.node) return arrow.node;
tony-go marked this conversation as resolved.
Show resolved Hide resolved

const result = this.tryParse(
() => super.parseSubscripts(base, startPos, startLoc, noCalls),
Expand Down