From 2786ffddcca5544eee4c0721f28c350b612d7be0 Mon Sep 17 00:00:00 2001 From: Alan Pierce Date: Wed, 13 Jul 2022 00:10:48 -0700 Subject: [PATCH 1/4] Fix parse failures when using ?? just after the end of a type This was detected when running Sucrase on the Babel test suite. --- src/parser/plugins/typescript.ts | 6 ++++++ src/parser/tokenizer/index.ts | 13 ++++++++++--- src/parser/traverser/expression.ts | 4 ++-- test/typescript-test.ts | 19 +++++++++++++++++-- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/parser/plugins/typescript.ts b/src/parser/plugins/typescript.ts index 10950613..9d48e3e3 100644 --- a/src/parser/plugins/typescript.ts +++ b/src/parser/plugins/typescript.ts @@ -9,6 +9,7 @@ import { nextTemplateToken, popTypeContext, pushTypeContext, + rescanAfterTypeEnd, } from "../tokenizer/index"; import {ContextualKeyword} from "../tokenizer/keywords"; import {TokenType, TokenType as tt} from "../tokenizer/types"; @@ -1252,6 +1253,11 @@ export function tsParseSubscript( // Bail out. We have something like ac, which is not an expression with // type arguments but an (a < b) > c comparison. unexpected(); + } else { + // This is an instantiation expression, e.g. Array, so we are + // leaving a type context, and operators like ?? need to be re-scanned to + // pick up the second question mark. + rescanAfterTypeEnd(); } if (state.error) { diff --git a/src/parser/tokenizer/index.ts b/src/parser/tokenizer/index.ts index 3349a601..9d122e7e 100644 --- a/src/parser/tokenizer/index.ts +++ b/src/parser/tokenizer/index.ts @@ -535,13 +535,20 @@ function readToken_gt(): void { } /** - * Called after `as` expressions in TS; we're switching from a type to a - * non-type context, so a > token may actually be >= + * Called when switching from a type to non-type context, e.g. after an `as` + * expression or after an instantiation expression like `Array`. Since + * we always tokenize one token ahead of the current state and tokenization is + * sometimes different in type and non-type contexts, we need to re-tokenize + * some specific cases to make sure we're picking up the right operator, + * particularly recognizing > as >= and recognizing ? as ??. */ -export function rescan_gt(): void { +export function rescanAfterTypeEnd(): void { if (state.type === tt.greaterThan) { state.pos -= 1; readToken_gt(); + } else if (state.type === tt.question) { + state.pos -= 1; + readToken_question(); } } diff --git a/src/parser/traverser/expression.ts b/src/parser/traverser/expression.ts index 3981489d..778f6ed0 100644 --- a/src/parser/traverser/expression.ts +++ b/src/parser/traverser/expression.ts @@ -51,7 +51,7 @@ import { nextTemplateToken, popTypeContext, pushTypeContext, - rescan_gt, + rescanAfterTypeEnd, retokenizeSlashAsRegex, } from "../tokenizer/index"; import {ContextualKeyword} from "../tokenizer/keywords"; @@ -206,7 +206,7 @@ function parseExprOp(startTokenIndex: number, minPrec: number, noIn: boolean): v const oldIsType = pushTypeContext(1); tsParseType(); popTypeContext(oldIsType); - rescan_gt(); + rescanAfterTypeEnd(); parseExprOp(startTokenIndex, minPrec, noIn); return; } diff --git a/test/typescript-test.ts b/test/typescript-test.ts index a657a9ad..b220202e 100644 --- a/test/typescript-test.ts +++ b/test/typescript-test.ts @@ -5,6 +5,7 @@ import { IMPORT_DEFAULT_PREFIX, IMPORT_WILDCARD_PREFIX, JSX_PREFIX, + NULLISH_COALESCE_PREFIX, OPTIONAL_CHAIN_PREFIX, } from "./prefixes"; import {assertResult, devProps} from "./util"; @@ -2388,15 +2389,17 @@ describe("typescript transform", () => { ); }); - it("properly handles a >= symbol after an `as` cast", () => { + it("properly handles >= and ?? after `as`", () => { assertTypeScriptResult( ` const x: string | number = 1; if (x as number >= 5) {} + if (y as unknown ?? false) {} `, - `"use strict"; + `"use strict";${NULLISH_COALESCE_PREFIX} const x = 1; if (x >= 5) {} + if (_nullishCoalesce(y , () => ( false))) {} `, ); }); @@ -3214,6 +3217,18 @@ describe("typescript transform", () => { ); }); + it("allows instantiation expressions followed by ??", () => { + assertResult( + ` + const foo = a ?? c; + `, + `${NULLISH_COALESCE_PREFIX} + const foo = _nullishCoalesce(a, () => ( c)); + `, + {transforms: ["typescript"]}, + ); + }); + it("allows extends constraints on infer type variables", () => { assertResult( ` From 71cffaed52c408b248eb7ef4a1e123b7139e0681 Mon Sep 17 00:00:00 2001 From: Alan Pierce Date: Wed, 13 Jul 2022 12:46:42 -0700 Subject: [PATCH 2/4] Remove expected failures from check-babel-tests --- spec-compliance-tests/babel-tests/check-babel-tests.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec-compliance-tests/babel-tests/check-babel-tests.ts b/spec-compliance-tests/babel-tests/check-babel-tests.ts index 01f78d14..6ecae671 100644 --- a/spec-compliance-tests/babel-tests/check-babel-tests.ts +++ b/spec-compliance-tests/babel-tests/check-babel-tests.ts @@ -67,7 +67,6 @@ flow/scope/declare-module flow/this-annotation/function-type flow/typecasts/yield jsx/basic/3 -typescript/cast/as typescript/export/as-namespace typescript/import/export-import typescript/import/export-import-require @@ -76,7 +75,6 @@ typescript/import/export-import-type-require typescript/import/import-default-id-type typescript/import/type-asi typescript/import/type-equals-require -typescript/type-arguments/instantiation-expression-binary-operator ` .split("\n") .filter((s) => s); From 99683a2d94e245fe6623273ceff3d8a02126bde2 Mon Sep 17 00:00:00 2001 From: Alan Pierce Date: Wed, 13 Jul 2022 15:31:49 -0700 Subject: [PATCH 3/4] Switch to an implementation closer to Babel's approach The conditional tokenization is only necessary for Flow and only causes issuse in TypeScript, so separating the cases out fixes this issue in a more fundamental way. --- src/parser/plugins/typescript.ts | 9 +++------ src/parser/tokenizer/index.ts | 20 +++++++++----------- src/parser/traverser/expression.ts | 4 ++-- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/parser/plugins/typescript.ts b/src/parser/plugins/typescript.ts index 9d48e3e3..cf57085b 100644 --- a/src/parser/plugins/typescript.ts +++ b/src/parser/plugins/typescript.ts @@ -9,7 +9,6 @@ import { nextTemplateToken, popTypeContext, pushTypeContext, - rescanAfterTypeEnd, } from "../tokenizer/index"; import {ContextualKeyword} from "../tokenizer/keywords"; import {TokenType, TokenType as tt} from "../tokenizer/types"; @@ -1243,6 +1242,9 @@ export function tsParseSubscript( // Tagged template with a type argument. parseTemplate(); } else if ( + // The remaining possible case is an instantiation expression, e.g. + // Array . Check for a few cases that would disqualify it and + // cause us to bail out. // a>c is not (a)>c, but a<(b>>c) state.type === tt.greaterThan || // ac is (ac @@ -1253,11 +1255,6 @@ export function tsParseSubscript( // Bail out. We have something like ac, which is not an expression with // type arguments but an (a < b) > c comparison. unexpected(); - } else { - // This is an instantiation expression, e.g. Array, so we are - // leaving a type context, and operators like ?? need to be re-scanned to - // pick up the second question mark. - rescanAfterTypeEnd(); } if (state.error) { diff --git a/src/parser/tokenizer/index.ts b/src/parser/tokenizer/index.ts index 9d122e7e..b02acb3d 100644 --- a/src/parser/tokenizer/index.ts +++ b/src/parser/tokenizer/index.ts @@ -535,20 +535,13 @@ function readToken_gt(): void { } /** - * Called when switching from a type to non-type context, e.g. after an `as` - * expression or after an instantiation expression like `Array`. Since - * we always tokenize one token ahead of the current state and tokenization is - * sometimes different in type and non-type contexts, we need to re-tokenize - * some specific cases to make sure we're picking up the right operator, - * particularly recognizing > as >= and recognizing ? as ??. + * Called after `as` expressions in TS; we're switching from a type to a + * non-type context, so a > token may actually be >= . */ -export function rescanAfterTypeEnd(): void { +export function rescan_gt(): void { if (state.type === tt.greaterThan) { state.pos -= 1; readToken_gt(); - } else if (state.type === tt.question) { - state.pos -= 1; - readToken_question(); } } @@ -572,7 +565,12 @@ function readToken_question(): void { // '?' const nextChar = input.charCodeAt(state.pos + 1); const nextChar2 = input.charCodeAt(state.pos + 2); - if (nextChar === charCodes.questionMark && !state.isType) { + if ( + nextChar === charCodes.questionMark && + // In Flow (but not TypeScript), ??string is a valid type that should be + // tokenized as two individual ? tokens. + !(isFlowEnabled && state.isType) + ) { if (nextChar2 === charCodes.equalsTo) { // '??=' finishOp(tt.assign, 3); diff --git a/src/parser/traverser/expression.ts b/src/parser/traverser/expression.ts index 778f6ed0..3981489d 100644 --- a/src/parser/traverser/expression.ts +++ b/src/parser/traverser/expression.ts @@ -51,7 +51,7 @@ import { nextTemplateToken, popTypeContext, pushTypeContext, - rescanAfterTypeEnd, + rescan_gt, retokenizeSlashAsRegex, } from "../tokenizer/index"; import {ContextualKeyword} from "../tokenizer/keywords"; @@ -206,7 +206,7 @@ function parseExprOp(startTokenIndex: number, minPrec: number, noIn: boolean): v const oldIsType = pushTypeContext(1); tsParseType(); popTypeContext(oldIsType); - rescanAfterTypeEnd(); + rescan_gt(); parseExprOp(startTokenIndex, minPrec, noIn); return; } From eedf1808e6045b0709436b236112ad3134d399d5 Mon Sep 17 00:00:00 2001 From: Alan Pierce Date: Wed, 13 Jul 2022 15:37:04 -0700 Subject: [PATCH 4/4] Add test for flow case --- test/flow-test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/flow-test.ts b/test/flow-test.ts index cdad9228..b919f2d8 100644 --- a/test/flow-test.ts +++ b/test/flow-test.ts @@ -688,4 +688,15 @@ describe("transform flow", () => { `, ); }); + + it("handles two ? operators in a row", () => { + assertFlowResult( + ` + type T = ??number; + `, + `"use strict"; + + `, + ); + }); });