Skip to content

Commit

Permalink
Merge pull request #4640 from GeoffreyBooth/generated-variables-in-fu…
Browse files Browse the repository at this point in the history
…nction-parameters

[CS2] Fix #4413: Generated variables in function parameters
  • Loading branch information
connec authored Aug 15, 2017
2 parents ae7f97b + 911c21f commit eff160e
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 19 deletions.
37 changes: 23 additions & 14 deletions lib/coffeescript/nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 14 additions & 5 deletions src/nodes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ exports.Base = class Base
@compileCommentFragments o, node, fragments
fragments

compileToFragmentsWithoutComments: (o, lvl) ->
@compileWithoutComments o, lvl, 'compileToFragments'

# Statements converted into expressions via closure-wrapping share a scope
# object with their parent closure, to preserve the expected lexical scope.
compileClosure: (o) ->
Expand Down Expand Up @@ -2626,11 +2629,7 @@ exports.Code = class Code extends Base
# compilation, so that they get output the “real” time this param
# is compiled.
paramToAddToScope = if param.value? then param else ref
if paramToAddToScope.name?.comments
salvagedComments = paramToAddToScope.name.comments
delete paramToAddToScope.name.comments
o.scope.parameter fragmentsToText paramToAddToScope.compileToFragments o
paramToAddToScope.name.comments = salvagedComments if salvagedComments
o.scope.parameter fragmentsToText paramToAddToScope.compileToFragmentsWithoutComments o
params.push ref
else
paramsAfterSplat.push param
Expand Down Expand Up @@ -2675,7 +2674,14 @@ exports.Code = class Code extends Base
for param, i in params
signature.push @makeCode ', ' if i isnt 0
signature.push @makeCode '...' if haveSplatParam and i is params.length - 1
# Compile this parameter, but if any generated variables get created
# (e.g. `ref`), shift those into the parent scope since we can’t put a
# `var` line inside a function parameter list.
scopeVariablesCount = o.scope.variables.length
signature.push param.compileToFragments(o)...
if scopeVariablesCount isnt o.scope.variables.length
generatedVariables = o.scope.variables.splice scopeVariablesCount
o.scope.parent.variables.push generatedVariables...
signature.push @makeCode ')'
# Block comments between `)` and `->`/`=>` get output between `)` and `{`.
if @funcGlyph?.comments?
Expand Down Expand Up @@ -2774,6 +2780,9 @@ exports.Param = class Param extends Base
compileToFragments: (o) ->
@name.compileToFragments o, LEVEL_LIST

compileToFragmentsWithoutComments: (o) ->
@name.compileToFragmentsWithoutComments o, LEVEL_LIST

asReference: (o) ->
return @reference if @reference
node = @name
Expand Down
11 changes: 11 additions & 0 deletions test/functions.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,14 @@ test "#3845/#3446: chain after function glyph", ->
doThing()
.then (@result) =>
.catch handleError

test "#4413: expressions in function parameters that create generated variables have those variables declared correctly", ->
'use strict'
# We’re in strict mode because we want an error to be thrown if the generated
# variable (`ref`) is assigned before being declared.
foo = -> null
bar = -> 33
f = (a = foo() ? bar()) -> a
g = (a = foo() ? bar()) -> a + 1
eq f(), 33
eq g(), 34

0 comments on commit eff160e

Please sign in to comment.