Skip to content

Commit

Permalink
fix #3111: incorrect ts parsing of x < y >= z
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed May 12, 2023
1 parent a3fcf70 commit 7cf5257
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 13 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Expand Up @@ -39,6 +39,14 @@
}
```

* Fix bug with TypeScript parsing of instantiation expressions followed by `=` ([#3111](https://github.com/evanw/esbuild/issues/3111))

This release fixes esbuild's TypeScript-to-JavaScript conversion code in the case where a potential instantiation expression is followed immediately by a `=` token (such that the trailing `>` becomes a `>=` token). Previously esbuild considered that to still be an instantiation expression, but the official TypeScript compiler considered it to be a `>=` operator instead. This release changes esbuild's interpretation to match TypeScript. This edge case currently [appears to be problematic](https://sucrase.io/#transforms=typescript&compareWithTypeScript=true&code=x%3Cy%3E%3Da%3Cb%3Cc%3E%3E()) for other TypeScript-to-JavaScript converters as well:

| Original code | TypeScript | esbuild 0.17.18 | esbuild 0.17.19 | Sucrase | Babel |
|---|---|---|---|---|---|
| `x<y>=a<b<c>>()` | `x<y>=a();` | `x=a();` | `x<y>=a();` | `x=a()` | Invalid left-hand side in assignment expression |

* Avoid removing unrecognized directives from the directive prologue when minifying ([#3115](https://github.com/evanw/esbuild/issues/3115))

The [directive prologue](https://262.ecma-international.org/6.0/#sec-directive-prologues-and-the-use-strict-directive) in JavaScript is a sequence of top-level string expressions that come before your code. The only directives that JavaScript engines currently recognize are `use strict` and sometimes `use asm`. However, the people behind React have made up their own directive for their own custom dialect of JavaScript. Previously esbuild only preserved the `use strict` directive when minifying, although you could still write React JavaScript with esbuild using something like `--banner:js="'your directive here';"`. With this release, you can now put arbitrary directives in the entry point and esbuild will preserve them in its minified output:
Expand Down
10 changes: 5 additions & 5 deletions internal/js_parser/js_parser.go
Expand Up @@ -4014,7 +4014,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE
if !p.options.ts.Parse {
p.lexer.Expected(js_lexer.TIdentifier)
}
p.skipTypeScriptTypeArguments(false /* isInsideJSXElement */)
p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{})
if p.lexer.Token != js_lexer.TOpenParen {
p.lexer.Expected(js_lexer.TOpenParen)
}
Expand Down Expand Up @@ -4326,7 +4326,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE
// TypeScript allows type arguments to be specified with angle brackets
// inside an expression. Unlike in other languages, this unfortunately
// appears to require backtracking to parse.
if p.options.ts.Parse && p.trySkipTypeScriptTypeArgumentsWithBacktracking() {
if p.options.ts.Parse && p.trySkipTypeArgumentsInExpressionWithBacktracking() {
optionalChain = oldOptionalChain
continue
}
Expand Down Expand Up @@ -4362,7 +4362,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE
// TypeScript allows type arguments to be specified with angle brackets
// inside an expression. Unlike in other languages, this unfortunately
// appears to require backtracking to parse.
if p.options.ts.Parse && p.trySkipTypeScriptTypeArgumentsWithBacktracking() {
if p.options.ts.Parse && p.trySkipTypeArgumentsInExpressionWithBacktracking() {
optionalChain = oldOptionalChain
continue
}
Expand Down Expand Up @@ -4777,7 +4777,7 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
// js_lexer.NextInsideJSXElement() after we hit the closing ">". The next
// token after the ">" might be an attribute name with a dash in it
// like this: "<Foo<T> data-disabled/>"
p.skipTypeScriptTypeArguments(true /* isInsideJSXElement */)
p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{isInsideJSXElement: true})
}

// Parse attributes
Expand Down Expand Up @@ -5949,7 +5949,7 @@ func (p *parser) parseClass(classKeyword logger.Range, name *js_ast.LocRef, clas
// does and it probably doesn't have that high of a performance overhead
// because "extends" clauses aren't that frequent, so it should be ok.
if p.options.ts.Parse {
p.skipTypeScriptTypeArguments(false /* isInsideJSXElement */)
p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{})
}
}

Expand Down
39 changes: 32 additions & 7 deletions internal/js_parser/ts_parser.go
Expand Up @@ -382,7 +382,7 @@ loop:

// "let foo: any \n <number>foo" must not become a single type
if checkTypeParameters && !p.lexer.HasNewlineBefore {
p.skipTypeScriptTypeArguments(false /* isInsideJSXElement */)
p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{})
}

case js_lexer.TTypeof:
Expand Down Expand Up @@ -414,7 +414,7 @@ loop:
}

if !p.lexer.HasNewlineBefore {
p.skipTypeScriptTypeArguments(false /* isInsideJSXElement */)
p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{})
}
}

Expand Down Expand Up @@ -510,7 +510,7 @@ loop:

// "{ <A extends B>(): c.d \n <E extends F>(): g.h }" must not become a single type
if !p.lexer.HasNewlineBefore {
p.skipTypeScriptTypeArguments(false /* isInsideJSXElement */)
p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{})
}

case js_lexer.TOpenBracket:
Expand Down Expand Up @@ -774,7 +774,12 @@ func (p *parser) skipTypeScriptTypeParameters(flags typeParameterFlags) skipType
return result
}

func (p *parser) skipTypeScriptTypeArguments(isInsideJSXElement bool) bool {
type skipTypeScriptTypeArgumentsOpts struct {
isInsideJSXElement bool
isParseTypeArgumentsInExpression bool
}

func (p *parser) skipTypeScriptTypeArguments(opts skipTypeScriptTypeArgumentsOpts) bool {
switch p.lexer.Token {
case js_lexer.TLessThan, js_lexer.TLessThanEquals,
js_lexer.TLessThanLessThan, js_lexer.TLessThanLessThanEquals:
Expand All @@ -793,11 +798,31 @@ func (p *parser) skipTypeScriptTypeArguments(isInsideJSXElement bool) bool {
}

// This type argument list must end with a ">"
p.lexer.ExpectGreaterThan(isInsideJSXElement)
if !opts.isParseTypeArgumentsInExpression {
// Normally TypeScript allows any token starting with ">". For example,
// "Array<Array<number>>()" is a type argument list even though there's a
// ">>" token, because ">>" starts with ">".
p.lexer.ExpectGreaterThan(opts.isInsideJSXElement)
} else {
// However, if we're emulating the TypeScript compiler's function called
// "parseTypeArgumentsInExpression" function, then we must only allow the
// ">" token itself. For example, "x < y >= z" is not a type argument list.
//
// This doesn't detect ">>" in "Array<Array<number>>()" because the inner
// type argument list isn't a call to "parseTypeArgumentsInExpression"
// because it's within a type context, not an expression context. So the
// token that we see here is ">" in that case because the first ">" has
// already been stripped off of the ">>" by the inner call.
if opts.isInsideJSXElement {
p.lexer.ExpectInsideJSXElement(js_lexer.TGreaterThan)
} else {
p.lexer.Expect(js_lexer.TGreaterThan)
}
}
return true
}

func (p *parser) trySkipTypeScriptTypeArgumentsWithBacktracking() bool {
func (p *parser) trySkipTypeArgumentsInExpressionWithBacktracking() bool {
oldLexer := p.lexer
p.lexer.IsLogDisabled = true

Expand All @@ -811,7 +836,7 @@ func (p *parser) trySkipTypeScriptTypeArgumentsWithBacktracking() bool {
}
}()

if p.skipTypeScriptTypeArguments(false /* isInsideJSXElement */) {
if p.skipTypeScriptTypeArguments(skipTypeScriptTypeArgumentsOpts{isParseTypeArgumentsInExpression: true}) {
// Check the token after the type argument list and backtrack if it's invalid
if !p.tsCanFollowTypeArgumentsInExpression() {
p.lexer.Unexpected()
Expand Down
17 changes: 16 additions & 1 deletion internal/js_parser/ts_parser_test.go
Expand Up @@ -2061,18 +2061,31 @@ func TestTSInstantiationExpression(t *testing.T) {
expectPrintedTS(t, "f.x<<T>() => T>;", "f.x;\n")
expectPrintedTS(t, "f['x']<<T>() => T>;", "f[\"x\"];\n")
expectPrintedTS(t, "f<x>g<y>;", "f < x > g;\n")
expectPrintedTS(t, "f<x>=g<y>;", "f = g;\n")
expectPrintedTS(t, "f<x>=g<y>;", "f < x >= g;\n")
expectPrintedTS(t, "f<x>>g<y>;", "f < x >> g;\n")
expectPrintedTS(t, "f<x>>>g<y>;", "f < x >>> g;\n")
expectParseErrorTS(t, "f<x>>=g<y>;", "<stdin>: ERROR: Invalid assignment target\n")
expectParseErrorTS(t, "f<x>>>=g<y>;", "<stdin>: ERROR: Invalid assignment target\n")
expectPrintedTS(t, "f<x,y>g<y>;", "f < x, y > g;\n")
expectPrintedTS(t, "f<x,y>=g<y>;", "f < x, y >= g;\n")
expectPrintedTS(t, "f<x,y>>g<y>;", "f < x, y >> g;\n")
expectPrintedTS(t, "f<x,y>>>g<y>;", "f < x, y >>> g;\n")
expectPrintedTS(t, "f<x,y>>=g<y>;", "f < x, y >>= g;\n")
expectPrintedTS(t, "f<x,y>>>=g<y>;", "f < x, y >>>= g;\n")
expectPrintedTS(t, "f<x> = g<y>;", "f = g;\n")
expectParseErrorTS(t, "f<x> > g<y>;", "<stdin>: ERROR: Unexpected \">\"\n")
expectParseErrorTS(t, "f<x> >> g<y>;", "<stdin>: ERROR: Unexpected \">>\"\n")
expectParseErrorTS(t, "f<x> >>> g<y>;", "<stdin>: ERROR: Unexpected \">>>\"\n")
expectParseErrorTS(t, "f<x> >= g<y>;", "<stdin>: ERROR: Unexpected \">=\"\n")
expectParseErrorTS(t, "f<x> >>= g<y>;", "<stdin>: ERROR: Unexpected \">>=\"\n")
expectParseErrorTS(t, "f<x> >>>= g<y>;", "<stdin>: ERROR: Unexpected \">>>=\"\n")
expectPrintedTS(t, "f<x,y> = g<y>;", "f = g;\n")
expectParseErrorTS(t, "f<x,y> > g<y>;", "<stdin>: ERROR: Unexpected \">\"\n")
expectParseErrorTS(t, "f<x,y> >> g<y>;", "<stdin>: ERROR: Unexpected \">>\"\n")
expectParseErrorTS(t, "f<x,y> >>> g<y>;", "<stdin>: ERROR: Unexpected \">>>\"\n")
expectParseErrorTS(t, "f<x,y> >= g<y>;", "<stdin>: ERROR: Unexpected \">=\"\n")
expectParseErrorTS(t, "f<x,y> >>= g<y>;", "<stdin>: ERROR: Unexpected \">>=\"\n")
expectParseErrorTS(t, "f<x,y> >>>= g<y>;", "<stdin>: ERROR: Unexpected \">>>=\"\n")
expectPrintedTS(t, "[f<x>];", "[f];\n")
expectPrintedTS(t, "f<x> ? g<y> : h<z>;", "f ? g : h;\n")
expectPrintedTS(t, "{ f<x> }", "{\n f;\n}\n")
Expand All @@ -2086,6 +2099,8 @@ func TestTSInstantiationExpression(t *testing.T) {
expectPrintedTS(t, "f<x> instanceof g<y>;", "f instanceof g;\n")
expectPrintedTS(t, "f<x> as g<y>;", "f;\n")
expectPrintedTS(t, "f<x> satisfies g<y>;", "f;\n")
expectPrintedTS(t, "class A extends B { f() { super.f<x>=y } }", "class A extends B {\n f() {\n super.f < x >= y;\n }\n}\n")
expectPrintedTS(t, "class A extends B { f() { super.f<x,y>=z } }", "class A extends B {\n f() {\n super.f < x, y >= z;\n }\n}\n")

expectParseErrorTS(t, "const a8 = f<number><number>;", "<stdin>: ERROR: Unexpected \";\"\n")
expectParseErrorTS(t, "const b1 = f?.<number>;", "<stdin>: ERROR: Expected \"(\" but found \";\"\n")
Expand Down

0 comments on commit 7cf5257

Please sign in to comment.