Skip to content

Commit bbff459

Browse files
aminesbdevaminesbdev
committed
fix(compiler): parse a division after a postfix increment or decrement
The lexer treated a `/` that follows any operator as the start of a regular expression, including the postfix `++` and `--` operators. Valid expressions like `total = count++ / 2` failed with "Unterminated regular expression". `++` and `--` now follow the same rule as `!`: a slash after a postfix operator is a division, while a slash after a prefix one can still start a regex (e.g. `++/a/.lastIndex`). Fixes angular#71509
1 parent 5244ea7 commit bbff459

3 files changed

Lines changed: 181 additions & 21 deletions

File tree

‎packages/compiler/src/expression_parser/lexer.ts‎

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -670,37 +670,50 @@ class _Scanner {
670670
}
671671

672672
private isStartOfRegex(): boolean {
673-
if (this.tokens.length === 0) {
674-
return true;
673+
let i = this.tokens.length - 1;
674+
675+
// Skip `!`, `++`, and `--`, which can be either prefix or postfix. Whether `/` starts a regex
676+
// after them depends on whether an expression could start immediately before the sequence
677+
// (making them prefix operators) or an operand ended there (making them postfix).
678+
while (
679+
i >= 0 &&
680+
(this.tokens[i].isOperator('!') ||
681+
this.tokens[i].isOperator('++') ||
682+
this.tokens[i].isOperator('--'))
683+
) {
684+
i--;
675685
}
676686

677-
const prevToken = this.tokens[this.tokens.length - 1];
687+
if (i < 0) {
688+
return true;
689+
}
678690

679-
// If a slash is preceded by a `!` operator, we need to distinguish whether it's a
680-
// negation or a non-null assertion. Regexes can only be precded by negations.
681-
if (prevToken.isOperator('!')) {
682-
const beforePrevToken = this.tokens.length > 1 ? this.tokens[this.tokens.length - 2] : null;
683-
const isNegation =
684-
beforePrevToken === null ||
685-
(beforePrevToken.type !== TokenType.Identifier &&
686-
!beforePrevToken.isCharacter(chars.$RPAREN) &&
687-
!beforePrevToken.isCharacter(chars.$RBRACKET));
691+
const prevToken = this.tokens[i];
688692

689-
return isNegation;
693+
if (prevToken.type === TokenType.Keyword) {
694+
const beforeKeyword = i > 0 ? this.tokens[i - 1] : null;
695+
const isPropertyRead =
696+
beforeKeyword !== null &&
697+
(beforeKeyword.isCharacter(chars.$PERIOD) || beforeKeyword.isOperator('?.'));
698+
if (isPropertyRead) {
699+
return false;
700+
}
701+
return (
702+
!prevToken.isKeywordThis() &&
703+
!prevToken.isKeywordNull() &&
704+
!prevToken.isKeywordUndefined() &&
705+
!prevToken.isKeywordTrue() &&
706+
!prevToken.isKeywordFalse()
707+
);
690708
}
691709

692-
// Only consider the slash a regex if it's preceded either by:
693-
// - Any operator, aside from `!` which is special-cased above.
694-
// - Opening paren (e.g. `(/a/)`).
695-
// - Opening bracket (e.g. `[/a/]`).
696-
// - A comma (e.g. `[1, /a/]`).
697-
// - A colon (e.g. `{foo: /a/}`).
698710
return (
699-
prevToken.type === TokenType.Operator ||
711+
(prevToken.type === TokenType.Operator && !prevToken.isOperator('?.')) ||
700712
prevToken.isCharacter(chars.$LPAREN) ||
701713
prevToken.isCharacter(chars.$LBRACKET) ||
702714
prevToken.isCharacter(chars.$COMMA) ||
703-
prevToken.isCharacter(chars.$COLON)
715+
prevToken.isCharacter(chars.$COLON) ||
716+
prevToken.isCharacter(chars.$SEMICOLON)
704717
);
705718
}
706719

‎packages/compiler/test/expression_parser/lexer_spec.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,122 @@ describe('lexer', () => {
10281028
expectNumberToken(tokens[5], 7, 8, 2);
10291029
});
10301030

1031+
it('should not tokenize a regex preceded by a postfix increment', () => {
1032+
const tokens: Token[] = lex('a++ / 2');
1033+
expect(tokens.length).toBe(4);
1034+
expectIdentifierToken(tokens[0], 0, 1, 'a');
1035+
expectOperatorToken(tokens[1], 1, 3, '++');
1036+
expectOperatorToken(tokens[2], 4, 5, '/');
1037+
expectNumberToken(tokens[3], 6, 7, 2);
1038+
});
1039+
1040+
it('should not tokenize a regex preceded by a postfix decrement', () => {
1041+
const tokens: Token[] = lex('a-- / b / c');
1042+
expect(tokens.length).toBe(6);
1043+
expectIdentifierToken(tokens[0], 0, 1, 'a');
1044+
expectOperatorToken(tokens[1], 1, 3, '--');
1045+
expectOperatorToken(tokens[2], 4, 5, '/');
1046+
expectIdentifierToken(tokens[3], 6, 7, 'b');
1047+
expectOperatorToken(tokens[4], 8, 9, '/');
1048+
expectIdentifierToken(tokens[5], 10, 11, 'c');
1049+
});
1050+
1051+
it('should not tokenize a regex preceded by a postfix increment after a non-null assertion', () => {
1052+
const tokens: Token[] = lex('a!++ / 2');
1053+
expect(tokens.length).toBe(5);
1054+
expectIdentifierToken(tokens[0], 0, 1, 'a');
1055+
expectOperatorToken(tokens[1], 1, 2, '!');
1056+
expectOperatorToken(tokens[2], 2, 4, '++');
1057+
expectOperatorToken(tokens[3], 5, 6, '/');
1058+
expectNumberToken(tokens[4], 7, 8, 2);
1059+
});
1060+
1061+
it('should not tokenize a regex preceded by a postfix increment on a keyword property', () => {
1062+
const tokens: Token[] = lex('o.in++ / 2');
1063+
expect(tokens.length).toBe(6);
1064+
expectIdentifierToken(tokens[0], 0, 1, 'o');
1065+
expectCharacterToken(tokens[1], 1, 2, '.');
1066+
expectKeywordToken(tokens[2], 2, 4, 'in');
1067+
expectOperatorToken(tokens[3], 4, 6, '++');
1068+
expectOperatorToken(tokens[4], 7, 8, '/');
1069+
expectNumberToken(tokens[5], 9, 10, 2);
1070+
});
1071+
1072+
it('should not tokenize a regex preceded by a non-null assertion on this or null', () => {
1073+
const tokens: Token[] = lex('this! / 2');
1074+
expect(tokens.length).toBe(4);
1075+
expectKeywordToken(tokens[0], 0, 4, 'this');
1076+
expectOperatorToken(tokens[1], 4, 5, '!');
1077+
expectOperatorToken(tokens[2], 6, 7, '/');
1078+
expectNumberToken(tokens[3], 8, 9, 2);
1079+
});
1080+
1081+
it('should not tokenize a regex preceded by stacked non-null assertions', () => {
1082+
const tokens: Token[] = lex('a!! / 2');
1083+
expect(tokens.length).toBe(5);
1084+
expectIdentifierToken(tokens[0], 0, 1, 'a');
1085+
expectOperatorToken(tokens[1], 1, 2, '!');
1086+
expectOperatorToken(tokens[2], 2, 3, '!');
1087+
expectOperatorToken(tokens[3], 4, 5, '/');
1088+
expectNumberToken(tokens[4], 6, 7, 2);
1089+
});
1090+
1091+
it('should not tokenize a regex preceded by a postfix increment on a safe keyword property', () => {
1092+
const tokens: Token[] = lex('o?.in++ / 2');
1093+
expect(tokens.length).toBe(6);
1094+
expectIdentifierToken(tokens[0], 0, 1, 'o');
1095+
expectOperatorToken(tokens[1], 1, 3, '?.');
1096+
expectKeywordToken(tokens[2], 3, 5, 'in');
1097+
expectOperatorToken(tokens[3], 5, 7, '++');
1098+
expectOperatorToken(tokens[4], 8, 9, '/');
1099+
expectNumberToken(tokens[5], 10, 11, 2);
1100+
});
1101+
1102+
it('should not tokenize a regex preceded by a non-null assertion on a tagged template literal', () => {
1103+
const tokens: Token[] = lex('tag`42`! / 2');
1104+
expect(tokens.length).toBe(5);
1105+
expectIdentifierToken(tokens[0], 0, 3, 'tag');
1106+
expectStringToken(tokens[1], 3, 7, '42', StringTokenKind.TemplateLiteralEnd);
1107+
expectOperatorToken(tokens[2], 7, 8, '!');
1108+
expectOperatorToken(tokens[3], 9, 10, '/');
1109+
expectNumberToken(tokens[4], 11, 12, 2);
1110+
});
1111+
1112+
it('should tokenize a regex preceded by a negation after typeof', () => {
1113+
const tokens: Token[] = lex('typeof !/a/.test(b)');
1114+
expect(tokens.length).toBe(8);
1115+
expectKeywordToken(tokens[0], 0, 6, 'typeof');
1116+
expectOperatorToken(tokens[1], 7, 8, '!');
1117+
expectRegExpBodyToken(tokens[2], 8, 11, 'a');
1118+
expectCharacterToken(tokens[3], 11, 12, '.');
1119+
expectIdentifierToken(tokens[4], 12, 16, 'test');
1120+
expectCharacterToken(tokens[5], 16, 17, '(');
1121+
expectIdentifierToken(tokens[6], 17, 18, 'b');
1122+
expectCharacterToken(tokens[7], 18, 19, ')');
1123+
});
1124+
1125+
it('should tokenize a regex preceded by a negation after void', () => {
1126+
const tokens: Token[] = lex('void !/a/.test(b)');
1127+
expect(tokens.length).toBe(8);
1128+
expectKeywordToken(tokens[0], 0, 4, 'void');
1129+
expectOperatorToken(tokens[1], 5, 6, '!');
1130+
expectRegExpBodyToken(tokens[2], 6, 9, 'a');
1131+
expectCharacterToken(tokens[3], 9, 10, '.');
1132+
expectIdentifierToken(tokens[4], 10, 14, 'test');
1133+
expectCharacterToken(tokens[5], 14, 15, '(');
1134+
expectIdentifierToken(tokens[6], 15, 16, 'b');
1135+
expectCharacterToken(tokens[7], 16, 17, ')');
1136+
});
1137+
1138+
it('should tokenize a regex preceded by a prefix increment', () => {
1139+
const tokens: Token[] = lex('++/a/.lastIndex');
1140+
expect(tokens.length).toBe(4);
1141+
expectOperatorToken(tokens[0], 0, 2, '++');
1142+
expectRegExpBodyToken(tokens[1], 2, 5, 'a');
1143+
expectCharacterToken(tokens[2], 5, 6, '.');
1144+
expectIdentifierToken(tokens[3], 6, 15, 'lastIndex');
1145+
});
1146+
10311147
it('should not tokenize consecutive regexes', () => {
10321148
const tokens: Token[] = lex('/ 1 / 2 / 3 / 4');
10331149
expect(tokens.length).toBe(6);

‎packages/compiler/test/expression_parser/parser_spec.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,8 @@ describe('parser', () => {
215215
checkAction('--a!');
216216
checkAction('++(a)');
217217
checkAction('--(a)');
218+
checkAction('++/a/.lastIndex');
219+
checkAction('--/a/g.lastIndex');
218220
});
219221

220222
it('should parse postfix ++ and --', () => {
@@ -261,6 +263,10 @@ describe('parser', () => {
261263
checkAction('a++ in o');
262264
checkAction('a++ < 3');
263265
checkAction('a++ * 2');
266+
checkAction('a++ / 2');
267+
checkAction('a!++ / 2');
268+
checkAction('a-- / b / c');
269+
checkAction('c = a++ / 2');
264270
checkAction('a++ ?? b');
265271
checkAction('a ? b++ : c--');
266272
checkAction('o[a++]');
@@ -273,6 +279,22 @@ describe('parser', () => {
273279
checkAction('a++ + ++a');
274280
});
275281

282+
describe('division after postfix operators with keywords', () => {
283+
it('should parse division after postfix update on keyword properties', () => {
284+
checkAction('o.in++ / 2');
285+
checkAction('o.as++ / 2');
286+
checkAction('o.null++ / 2');
287+
checkAction('o.this++ / 2');
288+
checkAction('o.let++ / 2');
289+
});
290+
291+
it('should parse division after non-null assertion on keywords', () => {
292+
checkBinding('this! / 2', '! / 2');
293+
checkBinding('null! / 2');
294+
checkBinding('undefined! / 2');
295+
});
296+
});
297+
276298
describe('literals', () => {
277299
it('should parse array', () => {
278300
checkAction('[1][0]');
@@ -775,6 +797,10 @@ describe('parser', () => {
775797
checkBinding('typeof `hello!`');
776798
checkBinding('typeof `hello ${name}!`');
777799
});
800+
801+
it('should parse division after non-null assertion on a tagged template literal', () => {
802+
checkBinding('tag`42`! / 2');
803+
});
778804
});
779805

780806
describe('regular expression literals', () => {
@@ -803,6 +829,11 @@ describe('parser', () => {
803829
checkBinding('url.replace(/^\\//, "") //comment', 'url.replace(/^\\//, "")');
804830
});
805831

832+
it('should parse a regular expression after a negation that follows typeof or void', () => {
833+
checkBinding('typeof !/a/.test(b)');
834+
checkBinding('void !/a/.test(b)');
835+
});
836+
806837
it('should report invalid regular expression flag', () => {
807838
expectBindingError('"foo".match(/abc/O)', 'Unsupported regular expression flag "O"');
808839
});

0 commit comments

Comments
 (0)