Skip to content

Commit

Permalink
fix #835: allow "arguments" in computed class fields
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 18, 2021
1 parent 536b609 commit 27aca43
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 23 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

Previously esbuild did not permit this, which is incorrect. Doing this is now permitted.

* It is an error to use `arguments` in a class field initializer such as `class { x = arguments[0] }`, but it is not an error to use `arguments` in a computed class property name such as `class { [arguments[0]] = x }` or inside TypeScript decorators such as `class { @decorator(arguments[0]) x() {} }`. Previously all of these cases were an error in esbuild, which is incorrect. Using `arguments` inside computed class property names and TypeScript decorators is now allowed.

## 0.8.47

* Release native binaries for the Apple M1 chip ([#550](https://github.com/evanw/esbuild/issues/550))
Expand Down
13 changes: 12 additions & 1 deletion internal/bundler/bundler_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,8 @@ func TestTypeScriptDecorators(t *testing.T) {
import {i} from './i'
import {j} from './j'
import k from './k'
console.log(all, all_computed, a, b, c, d, e, f, g, h, i, j, k)
import {fn} from './arguments'
console.log(all, all_computed, a, b, c, d, e, f, g, h, i, j, k, fn)
`,
"/all.ts": `
@x.y()
Expand Down Expand Up @@ -625,6 +626,16 @@ func TestTypeScriptDecorators(t *testing.T) {
foo(@x(() => 0) @y(() => 1) x) {}
}
`,
"/arguments.ts": `
function dec(x: any): any {}
export function fn(x: string): any {
class Foo {
@dec(arguments[0])
[arguments[0]]() {}
}
return Foo;
}
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Expand Down
17 changes: 16 additions & 1 deletion internal/bundler/snapshots/snapshots_ts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -542,5 +542,20 @@ __decorate([
], k_default.prototype, "foo", 1);
var k_default2 = k_default;

// arguments.ts
function dec(x2) {
}
function fn(x2) {
var _a2;
class Foo3 {
[_a2 = arguments[0]]() {
}
}
__decorate([
dec(arguments[0])
], Foo3.prototype, _a2, 1);
return Foo3;
}

// entry.js
console.log(all_default, all_computed_default, a, b, c, d, e_default2, f_default, g_default2, h_default, i, j, k_default2);
console.log(all_default, all_computed_default, a, b, c, d, e_default2, f_default, g_default2, h_default, i, j, k_default2, fn);
6 changes: 3 additions & 3 deletions internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1280,9 +1280,6 @@ const (

// This annotates all other symbols that don't have special behavior.
SymbolOther

// This symbol causes a compile error when referenced
SymbolError
)

func (kind SymbolKind) IsPrivate() bool {
Expand Down Expand Up @@ -1503,6 +1500,9 @@ type Scope struct {
// evaluated code might reference anything that it has access to.
ContainsDirectEval bool

// This is to help forbid "arguments" inside class body scopes
ForbidArguments bool

StrictMode StrictModeKind
}

Expand Down
35 changes: 22 additions & 13 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -4711,26 +4711,24 @@ func (p *parser) parseClass(name *js_ast.LocRef, classOpts parseClassOpts) js_as
// A scope is needed for private identifiers
scopeIndex := p.pushScopeForParsePass(js_ast.ScopeClassBody, bodyLoc)

// Make it an error to use "arguments" in a class body
argumentsRef := p.declareSymbol(js_ast.SymbolError, bodyLoc, "arguments")
p.symbols[argumentsRef.InnerIndex].MustNotBeRenamed = true
opts := propertyOpts{
isClass: true,
allowTSDecorators: classOpts.allowTSDecorators,
classHasExtends: extends != nil,
}

for p.lexer.Token != js_lexer.TCloseBrace {
if p.lexer.Token == js_lexer.TSemicolon {
p.lexer.Next()
continue
}

opts := propertyOpts{
isClass: true,
allowTSDecorators: classOpts.allowTSDecorators,
classHasExtends: extends != nil,
}

// Parse decorators for this property
firstDecoratorLoc := p.lexer.Loc()
if opts.allowTSDecorators {
opts.tsDecorators = p.parseTypeScriptDecorators()
} else {
opts.tsDecorators = nil
}

// This property may turn out to be a type in TypeScript, which should be ignored
Expand Down Expand Up @@ -6170,6 +6168,7 @@ func (p *parser) findSymbol(loc logger.Loc, name string) findSymbolResult {
var ref js_ast.Ref
var declareLoc logger.Loc
isInsideWithScope := false
didForbidArguments := false
s := p.currentScope

for {
Expand All @@ -6178,14 +6177,17 @@ func (p *parser) findSymbol(loc logger.Loc, name string) findSymbolResult {
isInsideWithScope = true
}

// Forbid referencing "arguments" inside class bodies
if s.ForbidArguments && name == "arguments" && !didForbidArguments {
r := js_lexer.RangeOfIdentifier(p.source, loc)
p.log.AddRangeError(&p.source, r, fmt.Sprintf("Cannot access %q here", name))
didForbidArguments = true
}

// Is the symbol a member of this scope?
if member, ok := s.Members[name]; ok {
ref = member.Ref
declareLoc = member.Loc
if p.symbols[ref.InnerIndex].Kind == js_ast.SymbolError {
r := js_lexer.RangeOfIdentifier(p.source, loc)
p.log.AddRangeError(&p.source, r, fmt.Sprintf("Cannot access %q here", name))
}
break
}

Expand Down Expand Up @@ -8827,12 +8829,19 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast
}
}
}

// Make it an error to use "arguments" in a class body
p.currentScope.ForbidArguments = true

if property.Value != nil {
*property.Value = p.visitExpr(*property.Value)
}
if property.Initializer != nil {
*property.Initializer = p.visitExpr(*property.Initializer)
}

// Restore the ability to use "arguments" in decorators and computed properties
p.currentScope.ForbidArguments = false
}

p.fnOnlyDataVisit.isThisNested = oldIsThisCaptured
Expand Down
13 changes: 8 additions & 5 deletions internal/js_parser/js_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1035,15 +1035,18 @@ func TestClass(t *testing.T) {
expectParseError(t, "(class static {})", "<stdin>: error: Expected \"{\" but found \"static\"\n")
expectParseError(t, "(class implements {})", "<stdin>: error: Expected \"{\" but found \"implements\"\n")

// The name "arguments" is forbidden
expectParseError(t, "class Foo { arguments = 1 }", "")
expectParseError(t, "class Foo { x = function() { arguments } }", "")
expectParseError(t, "class Foo { [arguments] }", "<stdin>: error: Cannot access \"arguments\" here\n")
expectParseError(t, "class Foo { [arguments = 1] }", "<stdin>: error: Cannot access \"arguments\" here\n")
// The name "arguments" is forbidden in class bodies outside of computed properties
expectPrinted(t, "class Foo { [arguments] }", "class Foo {\n [arguments];\n}\n")
expectPrinted(t, "class Foo { [arguments] = 1 }", "class Foo {\n [arguments] = 1;\n}\n")
expectPrinted(t, "class Foo { arguments = 1 }", "class Foo {\n arguments = 1;\n}\n")
expectPrinted(t, "class Foo { x = class { arguments = 1 } }", "class Foo {\n x = class {\n arguments = 1;\n };\n}\n")
expectPrinted(t, "class Foo { x = function() { arguments } }", "class Foo {\n x = function() {\n arguments;\n };\n}\n")
expectParseError(t, "class Foo { x = arguments }", "<stdin>: error: Cannot access \"arguments\" here\n")
expectParseError(t, "class Foo { x = () => arguments }", "<stdin>: error: Cannot access \"arguments\" here\n")
expectParseError(t, "class Foo { x = typeof arguments }", "<stdin>: error: Cannot access \"arguments\" here\n")
expectParseError(t, "class Foo { x = 1 ? 2 : arguments }", "<stdin>: error: Cannot access \"arguments\" here\n")
expectParseError(t, "class Foo { x = class { [arguments] } }", "<stdin>: error: Cannot access \"arguments\" here\n")
expectParseError(t, "class Foo { x = class { [arguments] = 1 } }", "<stdin>: error: Cannot access \"arguments\" here\n")

// The name "constructor" is sometimes forbidden
expectPrinted(t, "class Foo { get ['constructor']() {} }", "class Foo {\n get [\"constructor\"]() {\n }\n}\n")
Expand Down

0 comments on commit 27aca43

Please sign in to comment.