Skip to content

Commit

Permalink
fix #857: more hoisting edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 21, 2021
1 parent 39ac908 commit c202f4c
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 8 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@
I am deliberately not attempting to preserve the validity of asm.js code because it's a complicated legacy format and it's obsolete now that WebAssembly exists. By removing all `"use asm"` directives, the code will just become normal JavaScript and work fine without generating a warning.
* Fix a variable hoisting edge case ([#857](https://github.com/evanw/esbuild/issues/857))
It is allowed to use a nested `var` hoisted declaration with the same name as a top-level function declaration. In that case the two symbols should merge and be treated as the same symbol:
```js
async function x() {}
{
var x;
}
```
The parser previously allowed this for regular functions but not for async or generator functions. Now with this release, this behavior is also allowed for these special kinds of functions too.
## 0.8.49
* Work around a problem with `pnpm` and `NODE_PATH` ([#816](https://github.com/evanw/esbuild/issues/816))
Expand Down
4 changes: 4 additions & 0 deletions internal/js_ast/js_ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,10 @@ func (kind SymbolKind) IsHoistedOrFunction() bool {
return kind.IsHoisted() || kind == SymbolGeneratorOrAsyncFunction
}

func (kind SymbolKind) IsFunction() bool {
return kind == SymbolHoistedFunction || kind == SymbolGeneratorOrAsyncFunction
}

var InvalidRef Ref = Ref{^uint32(0), ^uint32(0)}

// Files are parsed in parallel for speed. We want to allow each parser to
Expand Down
27 changes: 19 additions & 8 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1345,19 +1345,25 @@ func (p *parser) hoistSymbols(scope *js_ast.Scope) {

if existingMember, ok := s.Members[symbol.OriginalName]; ok {
existingSymbol := &p.symbols[existingMember.Ref.InnerIndex]
switch existingSymbol.Kind {
case js_ast.SymbolUnbound, js_ast.SymbolHoisted, js_ast.SymbolHoistedFunction:

// We can hoist the symbol from the child scope into the symbol in
// this scope if:
//
// - The symbol is unbound (i.e. a global variable access)
// - The symbol is also another hoisted variable
// - The symbol is a function of any kind and we're in a function or module scope
//
// Is this unbound (i.e. a global access) or also hoisted?
if existingSymbol.Kind == js_ast.SymbolUnbound || existingSymbol.Kind == js_ast.SymbolHoisted ||
(existingSymbol.Kind.IsFunction() && (s.Kind == js_ast.ScopeEntry || s.Kind == js_ast.ScopeFunctionBody)) {
// Silently merge this symbol into the existing symbol
symbol.Link = existingMember.Ref
s.Members[symbol.OriginalName] = existingMember
continue nextMember
}

case js_ast.SymbolCatchIdentifier:
// Silently merge the existing symbol into this symbol
existingSymbol.Link = member.Ref
s.Members[symbol.OriginalName] = member

default:
// Otherwise if this isn't a catch identifier, it's a collision
if existingSymbol.Kind != js_ast.SymbolCatchIdentifier {
// An identifier binding from a catch statement and a function
// declaration can both silently shadow another hoisted symbol
if symbol.Kind != js_ast.SymbolCatchIdentifier && symbol.Kind != js_ast.SymbolHoistedFunction {
Expand All @@ -1373,6 +1379,11 @@ func (p *parser) hoistSymbols(scope *js_ast.Scope) {
}
continue nextMember
}

// If this is a catch identifier, silently merge the existing symbol
// into this symbol but continue hoisting past this catch scope
existingSymbol.Link = member.Ref
s.Members[symbol.OriginalName] = member
}

if s.Kind.StopsHoisting() {
Expand Down
40 changes: 40 additions & 0 deletions internal/js_parser/js_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,46 @@ func TestScope(t *testing.T) {
expectParseError(t, "{let x} {function x() {}}", "")
expectParseError(t, "{function x() {}} {let x}", "")

expectParseError(t, "function x() {} {var x}", "")
expectParseError(t, "function *x() {} {var x}", "")
expectParseError(t, "async function x() {} {var x}", "")
expectParseError(t, "async function *x() {} {var x}", "")

expectParseError(t, "{var x} function x() {}", "")
expectParseError(t, "{var x} function *x() {}", "")
expectParseError(t, "{var x} async function x() {}", "")
expectParseError(t, "{var x} async function *x() {}", "")

expectParseError(t, "{ function x() {} {var x} }", errorText)
expectParseError(t, "{ function *x() {} {var x} }", errorText)
expectParseError(t, "{ async function x() {} {var x} }", errorText)
expectParseError(t, "{ async function *x() {} {var x} }", errorText)

expectParseError(t, "{ {var x} function x() {} }", errorText)
expectParseError(t, "{ {var x} function *x() {} }", errorText)
expectParseError(t, "{ {var x} async function x() {} }", errorText)
expectParseError(t, "{ {var x} async function *x() {} }", errorText)

expectParseError(t, "function f() { function x() {} {var x} }", "")
expectParseError(t, "function f() { function *x() {} {var x} }", "")
expectParseError(t, "function f() { async function x() {} {var x} }", "")
expectParseError(t, "function f() { async function *x() {} {var x} }", "")

expectParseError(t, "function f() { {var x} function x() {} }", "")
expectParseError(t, "function f() { {var x} function *x() {} }", "")
expectParseError(t, "function f() { {var x} async function x() {} }", "")
expectParseError(t, "function f() { {var x} async function *x() {} }", "")

expectParseError(t, "function f() { { function x() {} {var x} } }", errorText)
expectParseError(t, "function f() { { function *x() {} {var x} } }", errorText)
expectParseError(t, "function f() { { async function x() {} {var x} } }", errorText)
expectParseError(t, "function f() { { async function *x() {} {var x} } }", errorText)

expectParseError(t, "function f() { { {var x} function x() {} } }", errorText)
expectParseError(t, "function f() { { {var x} function *x() {} } }", errorText)
expectParseError(t, "function f() { { {var x} async function x() {} } }", errorText)
expectParseError(t, "function f() { { {var x} async function *x() {} } }", errorText)

expectParseError(t, "var x=1, x=2", "")
expectParseError(t, "let x=1, x=2", errorText)
expectParseError(t, "const x=1, x=2", errorText)
Expand Down
56 changes: 56 additions & 0 deletions scripts/js-api-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2959,6 +2959,62 @@ let transformTests = {
asyncGenClassExprFn: ({ service }) => futureSyntax(service, '(class { async* foo() {} })', 'es2017', 'es2018'),
}

let functionScopeCases = [
'function x() {} { var x }',
'function* x() {} { var x }',
'async function x() {} { var x }',
'async function* x() {} { var x }',
'{ var x } function x() {}',
'{ var x } function* x() {}',
'{ var x } async function x() {}',
'{ var x } async function* x() {}',

'{ function x() {} { var x } }',
'{ function* x() {} { var x } }',
'{ async function x() {} { var x } }',
'{ async function* x() {} { var x } }',
'{ { var x } function x() {} }',
'{ { var x } function* x() {} }',
'{ { var x } async function x() {} }',
'{ { var x } async function* x() {} }',

'function f() { function x() {} { var x } }',
'function f() { function* x() {} { var x } }',
'function f() { async function x() {} { var x } }',
'function f() { async function* x() {} { var x } }',
'function f() { { var x } function x() {} }',
'function f() { { var x } function* x() {} }',
'function f() { { var x } async function x() {} }',
'function f() { { var x } async function* x() {} }',

'function f() { { function x() {} { var x } }}',
'function f() { { function* x() {} { var x } }}',
'function f() { { async function x() {} { var x } }}',
'function f() { { async function* x() {} { var x } }}',
'function f() { { { var x } function x() {} }}',
'function f() { { { var x } function* x() {} }}',
'function f() { { { var x } async function x() {} }}',
'function f() { { { var x } async function* x() {} }}',
];

for (let kind of ['var', 'let', 'const']) {
for (let code of functionScopeCases) {
code = code.replace('var', kind)
transformTests['functionScope: ' + code] = async ({ service }) => {
let esbuildError
let nodeError
try { await service.transform(code) } catch (e) { esbuildError = e }
try { new Function(code)() } catch (e) { nodeError = e }
if (!esbuildError !== !nodeError) {
throw new Error(`
esbuild: ${esbuildError}
node: ${nodeError}
`)
}
}
}
}

let syncTests = {
async buildSync({ esbuild, testDir }) {
const input = path.join(testDir, 'in.js')
Expand Down

0 comments on commit c202f4c

Please sign in to comment.