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

TypeScript: x < y >= z is incorrectly interpreted as a type argument list #797

Closed
evanw opened this issue May 13, 2023 · 1 comment · Fixed by #798
Closed

TypeScript: x < y >= z is incorrectly interpreted as a type argument list #797

evanw opened this issue May 13, 2023 · 1 comment · Fixed by #798

Comments

@evanw
Copy link

evanw commented May 13, 2023

Here's a full example (link):

// Input
fn(x < y, x >= y)

// Sucrase
fn(x = y)

// TypeScript
fn(x < y, x >= y);

Specifically TypeScript's parseTypeArgumentsInExpression function backtracks unless the trailing > comes from a > real token. This is subtle because TypeScript's type argument parser normally strips off the leading > from a token, which is why >> works in a<b<c>>(d). But it turns out that should only be done when you're in a type context. When you're in an expression context, the trailing > must not come from a token that has anything else after it. So for example a<b<c>>=(d) should not be considered a type argument list because parseTypeArgumentsInExpression encounters a >= token.

I just fixed this bug in esbuild: evanw/esbuild#3111. I'm reporting it here because Sucrase appears to also have this issue. The bug does not introduce a syntax error, which means the problematic output might be missed before releasing it, so I considered this bug to be higher severity.

@alangpierce
Copy link
Owner

Thank you letting me know! Agreed that it's higher severity.

alangpierce added a commit that referenced this issue Jun 28, 2023
Instructions: https://github.com/alangpierce/sucrase/wiki/Porting-changes-from-Babel's-parser

032203ea18 chore: Update TS 5.0.2 (#15502)
🚫 Babel-internal change.

656403b49f refactor: introduce `lookaheadInLineCharCode` (#15510)
🚫 I considered porting this refactor for `await using`, but decided against it.

202e4fa60b v7.21.4
🚫 Release only.

35c09db4f3 perf: Improve the code quality of `@babe/parser` for ts (#15540)
🚫 Code changes are minor and generally don't affect Sucrase.

bc7f795024 fix: Remove `mixins` and `implements` for `DeclareInterface` and `InterfaceDeclaration` (#15539)
🚫 AST-only.

0a1cfc0ed6 Babel 8 misc changes (#15068)
🚫 Comment-only change not affecting Sucrase.

86df74531d v7.21.5
🚫 Release only.

29426297d6 Remove `using await` restriction in explicitResourceManagement (#15602)
🚫 Only affects error handling, not relevant to Sucrase.

8fd99c8f48 v7.21.8
🚫 Release only.

2b70257372 chore: Enable rule `no-import-type-side-effects` (#15627)
🚫 Babel-internal change.

c5f68eb62e rescan gt token at the end of type args parsing (#15631)
✅ Added test and implemented fix similarly. Fixes #797.

0ad567f913 v7.21.9
🚫 Release only.

920b78c0c2 Enable regexp unicode sets parsing by default (#15638)
🚫 Doesn't affect Sucrase.

56a6c85aca Parse `await using` declarations (#15520)
✅ Implemented using a simpler backtracking method for now.

4b5ebdc9b3 Add support for the updated import attributes proposal (#15536)
✅ Implemented with a small tweak to Sucrase implementation.

7e5e7bf2f8 Unify parsing of import/export modifiers (type/typeof/module) (#15630)
🚫 Skipped for now, since it's prep work for later import syntax changes and I want to wait for that to settle a little more.

560b8ac523 [ts] Support `import ... =` and `export =` in scripts (#15497)
🚫 Sucrase already supported this behavior.

389ecb08ed v7.22.0
🚫 Release only.

325fe683d8 v7.22.3
🚫 Release only.

35116224d2 Mark `assert` attributes with `extra.deprecatedAssertSyntax` (#15667)
🚫 AST only.

eb4aa876b2 v7.22.4
🚫 Release only.

ecc819bf75 [babel 8] Require Node.js `^16.20.0 || ^18.16.0 || >=20.0.0` (#15585)
🚫 Babel-internal change.

72006cb657 Use Prettier v3.0.0-alpha.12 (#15617)
🚫 Babel-internal change.

08564ea230 v7.22.5
🚫 Release only.

be8fccd6f3 chore: Run `readmes.js` in CI (#15690)
🚫 Babel-internal change.

980b63c829 Use more optional chaining (#15703)
🚫 Babel-internal.
alangpierce added a commit that referenced this issue Jun 28, 2023
Instructions: https://github.com/alangpierce/sucrase/wiki/Porting-changes-from-Babel's-parser

032203ea18 chore: Update TS 5.0.2 (#15502)
🚫 Babel-internal change.

656403b49f refactor: introduce `lookaheadInLineCharCode` (#15510)
🚫 I considered porting this refactor for `await using`, but decided that it wasn't necessary for my implementation.

202e4fa60b v7.21.4
🚫 Release only.

35c09db4f3 perf: Improve the code quality of `@babe/parser` for ts (#15540)
🚫 Code changes are minor and generally don't affect Sucrase.

bc7f795024 fix: Remove `mixins` and `implements` for `DeclareInterface` and `InterfaceDeclaration` (#15539)
🚫 AST-only.

0a1cfc0ed6 Babel 8 misc changes (#15068)
🚫 Comment-only change not affecting Sucrase.

86df74531d v7.21.5
🚫 Release only.

29426297d6 Remove `using await` restriction in explicitResourceManagement (#15602)
🚫 Only affects error handling, not relevant to Sucrase.

8fd99c8f48 v7.21.8
🚫 Release only.

2b70257372 chore: Enable rule `no-import-type-side-effects` (#15627)
🚫 Babel-internal change.

c5f68eb62e rescan gt token at the end of type args parsing (#15631)
✅ Added test and implemented fix similarly. Fixes #797.

0ad567f913 v7.21.9
🚫 Release only.

920b78c0c2 Enable regexp unicode sets parsing by default (#15638)
🚫 Doesn't affect Sucrase.

56a6c85aca Parse `await using` declarations (#15520)
✅ Implemented using a simpler backtracking method for now.

4b5ebdc9b3 Add support for the updated import attributes proposal (#15536)
✅ Implemented with a small tweak to Sucrase implementation.

7e5e7bf2f8 Unify parsing of import/export modifiers (type/typeof/module) (#15630)
🚫 Skipped for now, since it's prep work for later import syntax changes and I want to wait for that to settle a little more.

560b8ac523 [ts] Support `import ... =` and `export =` in scripts (#15497)
🚫 Sucrase already supported this behavior.

389ecb08ed v7.22.0
🚫 Release only.

325fe683d8 v7.22.3
🚫 Release only.

35116224d2 Mark `assert` attributes with `extra.deprecatedAssertSyntax` (#15667)
🚫 AST only.

eb4aa876b2 v7.22.4
🚫 Release only.

ecc819bf75 [babel 8] Require Node.js `^16.20.0 || ^18.16.0 || >=20.0.0` (#15585)
🚫 Babel-internal change.

72006cb657 Use Prettier v3.0.0-alpha.12 (#15617)
🚫 Babel-internal change.

08564ea230 v7.22.5
🚫 Release only.

be8fccd6f3 chore: Run `readmes.js` in CI (#15690)
🚫 Babel-internal change.

980b63c829 Use more optional chaining (#15703)
🚫 Babel-internal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants