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.

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

fix(parser): offset internal index locations by startIndex #16936

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/babel-parser/src/parser/comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,11 @@ export default class CommentsParser extends BaseParser {
} else {
/*:: invariant(commentWS.containingNode !== null) */
const { containingNode: node, start: commentStart } = commentWS;
if (this.input.charCodeAt(commentStart - 1) === charCodes.comma) {
if (
this.input.charCodeAt(
this.state.offsetToSourcePos(commentStart) - 1,
) === charCodes.comma
) {
// If a commentWhitespace follows a comma and the containingNode allows
// list structures with trailing comma, merge it to the trailingComment
// of the last non-null list element
Expand Down
23 changes: 19 additions & 4 deletions packages/babel-parser/src/parser/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ export default abstract class ExpressionParser extends LValParser {
potentialArrowAt: number,
): expr is N.ArrowFunctionExpression {
return (
expr.type === "ArrowFunctionExpression" && expr.start === potentialArrowAt
expr.type === "ArrowFunctionExpression" &&
this.state.offsetToSourcePos(expr.start) === potentialArrowAt
);
}

Expand Down Expand Up @@ -949,7 +950,7 @@ export default abstract class ExpressionParser extends LValParser {
!this.canInsertSemicolon() &&
// check there are no escape sequences, such as \u{61}sync
base.end - base.start === 5 &&
base.start === this.state.potentialArrowAt
this.state.offsetToSourcePos(base.start) === this.state.potentialArrowAt
);
}

Expand Down Expand Up @@ -1662,7 +1663,14 @@ export default abstract class ExpressionParser extends LValParser {
node: any,
): T {
this.addExtra(node, "rawValue", value);
this.addExtra(node, "raw", this.input.slice(node.start, this.state.end));
this.addExtra(
node,
"raw",
this.input.slice(
this.state.offsetToSourcePos(node.start),
this.state.end,
),
);
node.value = value;
this.next();
return this.finishNode<T>(node, type);
Expand Down Expand Up @@ -1696,7 +1704,14 @@ export default abstract class ExpressionParser extends LValParser {
flags: N.RegExpLiteral["flags"];
}) {
const node = this.startNode<N.RegExpLiteral>();
this.addExtra(node, "raw", this.input.slice(node.start, this.state.end));
this.addExtra(
node,
"raw",
this.input.slice(
this.state.offsetToSourcePos(node.start),
this.state.end,
),
);
node.pattern = value.pattern;
node.flags = value.flags;
this.next();
Expand Down
9 changes: 6 additions & 3 deletions packages/babel-parser/src/parser/statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,10 @@ export default abstract class StatementParser extends ExpressionParser {

const directiveLiteral = directive.value;
const expressionValue = directiveLiteral.value;
const raw = this.input.slice(directiveLiteral.start, directiveLiteral.end);
const raw = this.input.slice(
this.state.offsetToSourcePos(directiveLiteral.start),
this.state.offsetToSourcePos(directiveLiteral.end),
);
const val = (directiveLiteral.value = raw.slice(1, -1)); // remove quotes

this.addExtra(directiveLiteral, "raw", raw);
Expand Down Expand Up @@ -1282,7 +1285,7 @@ export default abstract class StatementParser extends ExpressionParser {
for (let i = this.state.labels.length - 1; i >= 0; i--) {
const label = this.state.labels[i];
if (label.statementStart === node.start) {
label.statementStart = this.state.start;
label.statementStart = this.state.sourceToOffsetPos(this.state.start);
label.kind = kind;
} else {
break;
Expand All @@ -1292,7 +1295,7 @@ export default abstract class StatementParser extends ExpressionParser {
this.state.labels.push({
name: maybeName,
kind: kind,
statementStart: this.state.start,
statementStart: this.state.sourceToOffsetPos(this.state.start),
});
// https://tc39.es/ecma262/#prod-LabelledItem
node.body =
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-parser/src/parser/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export default abstract class UtilParser extends Tokenizer {
hasPrecedingLineBreak(): boolean {
return hasNewLine(
this.input,
this.state.lastTokEndLoc.index,
this.state.offsetToSourcePos(this.state.lastTokEndLoc.index),
this.state.start,
);
}
Expand Down
21 changes: 17 additions & 4 deletions packages/babel-parser/src/plugins/flow/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2180,7 +2180,11 @@ export default (superClass: typeof Parser) =>
parse: () => T,
): T {
let result: T;
if (this.state.noArrowParamsConversionAt.includes(node.start)) {
if (
this.state.noArrowParamsConversionAt.includes(
this.state.offsetToSourcePos(node.start),
)
) {
this.state.noArrowParamsConversionAt.push(this.state.start);
result = parse();
this.state.noArrowParamsConversionAt.pop();
Expand Down Expand Up @@ -3097,7 +3101,11 @@ export default (superClass: typeof Parser) =>
| Array<N.Expression | N.SpreadElement>
| Array<N.Expression | N.RestElement>,
): void {
if (this.state.noArrowParamsConversionAt.includes(node.start)) {
if (
this.state.noArrowParamsConversionAt.includes(
this.state.offsetToSourcePos(node.start),
)
) {
node.params = params as N.ArrowFunctionExpression["params"];
} else {
super.setArrowFunctionParameters(node, params);
Expand All @@ -3112,7 +3120,9 @@ export default (superClass: typeof Parser) =>
): void {
if (
isArrowFunction &&
this.state.noArrowParamsConversionAt.includes(node.start)
this.state.noArrowParamsConversionAt.includes(
this.state.offsetToSourcePos(node.start),
)
) {
return;
}
Expand All @@ -3134,7 +3144,10 @@ export default (superClass: typeof Parser) =>

parseParenAndDistinguishExpression(canBeArrow: boolean): N.Expression {
return super.parseParenAndDistinguishExpression(
canBeArrow && !this.state.noArrowAt.includes(this.state.start),
canBeArrow &&
!this.state.noArrowAt.includes(
this.state.sourceToOffsetPos(this.state.start),
),
);
}

Expand Down
5 changes: 4 additions & 1 deletion packages/babel-parser/src/plugins/placeholders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,10 @@ export default (superClass: typeof Parser) =>

// Throws if the current token and the prev one are separated by a space.
assertNoSpace(): void {
if (this.state.start > this.state.lastTokEndLoc.index) {
if (
this.state.start >
this.state.offsetToSourcePos(this.state.lastTokEndLoc.index)
) {
this.raise(PlaceholderErrors.UnexpectedSpace, this.state.lastTokEndLoc);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/babel-parser/src/plugins/typescript/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3237,7 +3237,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
propertyName:
key.type === "Identifier" && !node.computed
? key.name
: `[${this.input.slice(key.start, key.end)}]`,
: `[${this.input.slice(this.state.offsetToSourcePos(key.start), this.state.offsetToSourcePos(key.end))}]`,
},
);
}
Expand Down Expand Up @@ -4002,7 +4002,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
methodName:
key.type === "Identifier" && !method.computed
? key.name
: `[${this.input.slice(key.start, key.end)}]`,
: `[${this.input.slice(this.state.offsetToSourcePos(key.start), this.state.offsetToSourcePos(key.end))}]`,
});
}
}
Expand Down
11 changes: 6 additions & 5 deletions packages/babel-parser/src/tokenizer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ export default abstract class Tokenizer extends CommentsParser {
curLine: state.curLine,
lineStart: state.lineStart,
curPosition: state.curPosition,
sourceToOffsetPos: state.sourceToOffsetPos,
offsetToSourcePos: state.offsetToSourcePos,
};
}

Expand Down Expand Up @@ -446,8 +448,8 @@ export default abstract class Tokenizer extends CommentsParser {
if (comments.length > 0) {
const end = this.state.pos;
const commentWhitespace: CommentWhitespace = {
start: spaceStart,
end,
start: this.state.sourceToOffsetPos(spaceStart),
end: this.state.sourceToOffsetPos(end),
comments,
leadingNode: null,
trailingNode: null,
Expand Down Expand Up @@ -1172,6 +1174,7 @@ export default abstract class Tokenizer extends CommentsParser {
}

readRadixNumber(radix: number): void {
const start = this.state.pos;
const startLoc = this.state.curPosition();
let isBigInt = false;

Expand Down Expand Up @@ -1201,9 +1204,7 @@ export default abstract class Tokenizer extends CommentsParser {
}

if (isBigInt) {
const str = this.input
.slice(startLoc.index, this.state.pos)
.replace(/[_n]/g, "");
const str = this.input.slice(start, this.state.pos).replace(/[_n]/g, "");
this.finishToken(tt.bigint, str);
return;
}
Expand Down
14 changes: 12 additions & 2 deletions packages/babel-parser/src/tokenizer/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export default class State {
*/

curPosition(): Position {
const index = this.pos + this.startIndex;
const index = this.sourceToOffsetPos(this.pos);
return new Position(this.curLine, index - this.lineStart, index);
}

Expand Down Expand Up @@ -208,6 +208,14 @@ export default class State {

return state;
}

sourceToOffsetPos(sourcePos: number) {
return sourcePos + this.startIndex;
}

offsetToSourcePos(offsetPos: number) {
return offsetPos - this.startIndex;
}
}

export type LookaheadState = {
Expand All @@ -221,7 +229,9 @@ export type LookaheadState = {
lastTokEndLoc: Position;
curLine: number;
lineStart: number;
curPosition: () => Position;
curPosition: State["curPosition"];
sourceToOffsetPos: State["sourceToOffsetPos"];
offsetToSourcePos: State["offsetToSourcePos"];
Copy link
Contributor

Choose a reason for hiding this comment

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

The lookaheadState should be kept minimal for performance. I guess we didn't call sourceToOffsetPos or offsetToSourcePos on a lookahead state, if that is true, then we don't have to include them in the lookahead state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JLHwung fyi without this the tests are failing and so it seems necessary with the changes in the PR

Copy link
Member

Choose a reason for hiding this comment

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

Can we move those functions from State to the parser itself maybe?

Copy link
Member

Choose a reason for hiding this comment

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

If we inline sourceToOffsetPos in curPosition, the test passes.

Copy link
Member

Choose a reason for hiding this comment

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

I pushed a commit to move them.

/* Used only in readToken_mult_modulo */
inType: boolean;
// These boolean properties are not initialized in createLookaheadState()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"start":8,"end":9,"loc":{"start":{"line":1,"column":11,"index":8},"end":{"line":1,"column":12,"index":9}},
"extra": {
"rawValue": 1,
"raw": ""
"raw": "1"
},
"value": 1
}
Expand All @@ -48,7 +48,7 @@
"start":16,"end":17,"loc":{"start":{"line":2,"column":7,"index":16},"end":{"line":2,"column":8,"index":17}},
"extra": {
"rawValue": 2,
"raw": ""
"raw": "2"
},
"value": 2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"start":8,"end":9,"loc":{"start":{"line":1,"column":11,"index":8},"end":{"line":1,"column":12,"index":9}},
"extra": {
"rawValue": 1,
"raw": ""
"raw": "1"
},
"value": 1
}
Expand All @@ -48,7 +48,7 @@
"start":16,"end":17,"loc":{"start":{"line":2,"column":7,"index":16},"end":{"line":2,"column":8,"index":17}},
"extra": {
"rawValue": 2,
"raw": ""
"raw": "2"
},
"value": 2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"start":8,"end":9,"loc":{"start":{"line":3,"column":11,"index":8},"end":{"line":3,"column":12,"index":9}},
"extra": {
"rawValue": 1,
"raw": ""
"raw": "1"
},
"value": 1
}
Expand All @@ -48,7 +48,7 @@
"start":16,"end":17,"loc":{"start":{"line":4,"column":7,"index":16},"end":{"line":4,"column":8,"index":17}},
"extra": {
"rawValue": 2,
"raw": ""
"raw": "2"
},
"value": 2
}
Expand Down
Loading
Loading