diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index ad9c6e5b94..1d5609bd96 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -145,6 +145,10 @@ return fragments; } + compileToFragmentsWithoutComments(o, lvl) { + return this.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) { @@ -3671,7 +3675,7 @@ // parameters after the splat, they are declared via expressions in the // function body. compileNode(o) { - var answer, body, boundMethodCheck, comment, condition, exprs, haveBodyParam, haveSplatParam, i, ifTrue, j, k, l, len1, len2, len3, m, methodScope, modifiers, name, param, paramNames, paramToAddToScope, params, paramsAfterSplat, ref, ref1, ref2, ref3, ref4, ref5, ref6, ref7, ref8, salvagedComments, signature, splatParamName, thisAssignments, wasEmpty; + var answer, body, boundMethodCheck, comment, condition, exprs, generatedVariables, haveBodyParam, haveSplatParam, i, ifTrue, j, k, l, len1, len2, len3, m, methodScope, modifiers, name, param, paramNames, paramToAddToScope, params, paramsAfterSplat, ref, ref1, ref2, ref3, ref4, ref5, ref6, ref7, scopeVariablesCount, signature, splatParamName, thisAssignments, wasEmpty; if (this.ctor) { if (this.isAsync) { this.name.error('Class constructor may not be async'); @@ -3823,14 +3827,7 @@ // compilation, so that they get output the “real” time this param // is compiled. paramToAddToScope = param.value != null ? param : ref; - if ((ref5 = paramToAddToScope.name) != null ? ref5.comments : void 0) { - salvagedComments = paramToAddToScope.name.comments; - delete paramToAddToScope.name.comments; - } - o.scope.parameter(fragmentsToText(paramToAddToScope.compileToFragments(o))); - if (salvagedComments) { - paramToAddToScope.name.comments = salvagedComments; - } + o.scope.parameter(fragmentsToText(paramToAddToScope.compileToFragmentsWithoutComments(o))); } params.push(ref); } else { @@ -3843,7 +3840,7 @@ ifTrue = new Assign(new Value(param.name), param.value); exprs.push(new If(condition, ifTrue)); } - if (((ref6 = param.name) != null ? ref6.value : void 0) != null) { + if (((ref5 = param.name) != null ? ref5.value : void 0) != null) { // Add this parameter to the scope, since it wouldn’t have been added // yet since it was skipped earlier. o.scope.add(param.name.value, 'var', true); @@ -3903,14 +3900,22 @@ if (haveSplatParam && i === params.length - 1) { signature.push(this.makeCode('...')); } + // 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 !== o.scope.variables.length) { + generatedVariables = o.scope.variables.splice(scopeVariablesCount); + o.scope.parent.variables.push(...generatedVariables); + } } signature.push(this.makeCode(')')); // Block comments between `)` and `->`/`=>` get output between `)` and `{`. - if (((ref7 = this.funcGlyph) != null ? ref7.comments : void 0) != null) { - ref8 = this.funcGlyph.comments; - for (l = 0, len3 = ref8.length; l < len3; l++) { - comment = ref8[l]; + if (((ref6 = this.funcGlyph) != null ? ref6.comments : void 0) != null) { + ref7 = this.funcGlyph.comments; + for (l = 0, len3 = ref7.length; l < len3; l++) { + comment = ref7[l]; comment.unshift = false; } this.compileCommentFragments(o, this.funcGlyph, signature); @@ -4068,6 +4073,10 @@ return this.name.compileToFragments(o, LEVEL_LIST); } + compileToFragmentsWithoutComments(o) { + return this.name.compileToFragmentsWithoutComments(o, LEVEL_LIST); + } + asReference(o) { var name, node; if (this.reference) { diff --git a/src/nodes.coffee b/src/nodes.coffee index 677cf27ab1..677d4df028 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -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) -> @@ -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 @@ -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? @@ -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 diff --git a/test/functions.coffee b/test/functions.coffee index 47fa9ae305..a94ed0529c 100644 --- a/test/functions.coffee +++ b/test/functions.coffee @@ -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