Skip to content

Commit 9a48566

Browse files
Julian RosseGeoffreyBooth
authored andcommitted
[CS2] Restore bound class methods via runtime check to avoid premature calling of bound method before binding (#4561)
* bound method runtime check * restore bound method tests * bound method tests * test bound method in prop-named class * run check against parent class * dummy commit * remove comment * rename to boundMethodCheck * fixes from code review * use ref to own class for check * fixes from code review * remove unneeded param
1 parent 76e70a6 commit 9a48566

File tree

6 files changed

+338
-14
lines changed

6 files changed

+338
-14
lines changed

lib/coffeescript/nodes.js

Lines changed: 45 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/nodes.coffee

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,6 +1308,10 @@ exports.Class = class Class extends Base
13081308
# Anonymous classes are only valid in expressions
13091309
node = new Parens node
13101310

1311+
if @boundMethods.length and @parent
1312+
@variable ?= new IdentifierLiteral o.scope.freeVariable '_class'
1313+
[@variable, @variableRef] = @variable.cache o unless @variableRef?
1314+
13111315
if @variable
13121316
node = new Assign @variable, node, null, { @moduleDeclaration }
13131317

@@ -1318,9 +1322,11 @@ exports.Class = class Class extends Base
13181322
delete @compileNode
13191323

13201324
compileClassDeclaration: (o) ->
1321-
@ctor ?= @makeDefaultConstructor() if @externalCtor
1325+
@ctor ?= @makeDefaultConstructor() if @externalCtor or @boundMethods.length
13221326
@ctor?.noReturn = true
13231327

1328+
@proxyBoundMethods() if @boundMethods.length
1329+
13241330
o.indent += TAB
13251331

13261332
result = []
@@ -1356,6 +1362,7 @@ exports.Class = class Class extends Base
13561362

13571363
walkBody: ->
13581364
@ctor = null
1365+
@boundMethods = []
13591366
executableBody = null
13601367

13611368
initializer = []
@@ -1401,6 +1408,8 @@ exports.Class = class Class extends Base
14011408
@ctor = method
14021409
else if method.isStatic and method.bound
14031410
method.context = @name
1411+
else if method.bound
1412+
@boundMethods.push method
14041413

14051414
if initializer.length isnt expressions.length
14061415
@body.expressions = (expression.hoist() for expression in initializer)
@@ -1438,7 +1447,7 @@ exports.Class = class Class extends Base
14381447
method.name = new (if methodName.shouldCache() then Index else Access) methodName
14391448
method.name.updateLocationDataIfMissing methodName.locationData
14401449
method.ctor = (if @parent then 'derived' else 'base') if methodName.value is 'constructor'
1441-
method.error 'Methods cannot be bound functions' if method.bound
1450+
method.error 'Cannot define a constructor as a bound (fat arrow) function' if method.bound and method.ctor
14421451

14431452
method
14441453

@@ -1457,6 +1466,15 @@ exports.Class = class Class extends Base
14571466

14581467
ctor
14591468

1469+
proxyBoundMethods: ->
1470+
@ctor.thisAssignments = for method in @boundMethods
1471+
method.classVariable = @variableRef if @parent
1472+
1473+
name = new Value(new ThisLiteral, [ method.name ])
1474+
new Assign name, new Call(new Value(name, [new Access new PropertyName 'bind']), [new ThisLiteral])
1475+
1476+
null
1477+
14601478
exports.ExecutableClassBody = class ExecutableClassBody extends Base
14611479
children: [ 'class', 'body' ]
14621480

@@ -1540,7 +1558,7 @@ exports.ExecutableClassBody = class ExecutableClassBody extends Base
15401558
@body.traverseChildren false, (node) =>
15411559
if node instanceof ThisLiteral
15421560
node.value = @name
1543-
else if node instanceof Code and node.bound
1561+
else if node instanceof Code and node.bound and node.isStatic
15441562
node.context = @name
15451563

15461564
# Make class/prototype assignments for invalid ES properties
@@ -2180,6 +2198,9 @@ exports.Code = class Code extends Base
21802198
wasEmpty = @body.isEmpty()
21812199
@body.expressions.unshift thisAssignments... unless @expandCtorSuper thisAssignments
21822200
@body.expressions.unshift exprs...
2201+
if @isMethod and @bound and not @isStatic and @classVariable
2202+
boundMethodCheck = new Value new Literal utility 'boundMethodCheck', o
2203+
@body.expressions.unshift new Call(boundMethodCheck, [new Value(new ThisLiteral), @classVariable])
21832204
@body.makeReturn() unless wasEmpty or @noReturn
21842205

21852206
# Assemble the output
@@ -3149,6 +3170,13 @@ exports.If = class If extends Base
31493170

31503171
UTILITIES =
31513172
modulo: -> 'function(a, b) { return (+a % (b = +b) + b) % b; }'
3173+
boundMethodCheck: -> "
3174+
function(instance, Constructor) {
3175+
if (!(instance instanceof Constructor)) {
3176+
throw new Error('Bound instance method accessed before binding');
3177+
}
3178+
}
3179+
"
31523180

31533181
# Shortcuts to speed up the lookup time for native functions.
31543182
hasProp: -> '{}.hasOwnProperty'

test/assignment.coffee

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,9 @@ test "Assignment to variables similar to helper functions", ->
562562
extend = 3
563563
hasProp = 4
564564
value: 5
565-
method: (bind, bind1) -> [bind, bind1, extend, hasProp, @value]
566-
arrayEq [1, 2, 3, 4, 5], new B().method 1, 2
565+
method: (bind, bind1) => [bind, bind1, extend, hasProp, @value]
566+
{method} = new B
567+
arrayEq [1, 2, 3, 4, 5], method 1, 2
567568

568569
modulo = -1 %% 3
569570
eq 2, modulo

0 commit comments

Comments
 (0)