From 23a691ae8751bf1f22ecfe58bb13fa254e862457 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 7 Dec 2014 15:34:30 -0800 Subject: [PATCH 1/3] Add test for reserved keywords as parameters --- test/functions.coffee | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/functions.coffee b/test/functions.coffee index 931d79a8..d9cd5830 100644 --- a/test/functions.coffee +++ b/test/functions.coffee @@ -201,6 +201,18 @@ test "arguments vs parameters", -> f = (g) -> g() eq 5, f (x) -> 5 +test "reserved keyword as parameters", -> + f = (_case, @case) -> [_case, @case] + [a, b] = f(1, 2) + eq 1, a + eq 2, b + + f = (@case, _case...) -> [@case, _case...] + [a, b, c] = f(1, 2, 3) + eq 1, a + eq 2, b + eq 3, c + test "#1844: bound functions in nested comprehensions causing empty var statements", -> a = ((=>) for a in [0] for b in [0]) eq 1, a.length From 8ab15d737225acf3d0c2e9d575225df6a470971a Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 10 Jan 2015 23:04:30 +0100 Subject: [PATCH 2/3] Fix #1500, #1574, #3318: Name generated vars uniquely Any variables generated by CoffeeScript are now made sure to be named to something not present in the source code being compiled. This way you can no longer interfere with them, either on purpose or by mistake. (#1500, #1574) For example, `({a}, _arg) ->` now compiles correctly. (#1574) As opposed to the somewhat complex implementations discussed in #1500, this commit takes a very simple approach by saving all used variables names using a single pass over the token stream. Any generated variables are then made sure not to exist in that list. `(@a) -> a` used to be equivalent to `(@a) -> @a`, but now throws a runtime `ReferenceError` instead (unless `a` exists in an upper scope of course). (#3318) `(@a) ->` used to compile to `(function(a) { this.a = a; })`. Now it compiles to `(function(_at_a) { this.a = _at_a; })`. (But you cannot access `_at_a` either, of course.) Because of the above, `(@a, a) ->` is now valid; `@a` and `a` are not duplicate parameters. Duplicate this-parameters with a reserved word, such as `(@case, @case) ->`, used to compile but now throws, just like regular duplicate parameters. --- lib/coffee-script/coffee-script.js | 20 +++- lib/coffee-script/lexer.js | 1 + lib/coffee-script/nodes.js | 150 ++++++++++++++--------------- lib/coffee-script/optparse.js | 4 +- lib/coffee-script/rewriter.js | 4 +- lib/coffee-script/scope.js | 18 ++-- lib/coffee-script/sourcemap.js | 4 +- src/coffee-script.coffee | 12 ++- src/lexer.coffee | 1 + src/nodes.coffee | 14 +-- src/scope.litcoffee | 12 ++- test/assignment.coffee | 32 ++++++ test/error_messages.coffee | 16 +++ test/functions.coffee | 19 +++- test/strict.coffee | 14 +-- 15 files changed, 202 insertions(+), 119 deletions(-) diff --git a/lib/coffee-script/coffee-script.js b/lib/coffee-script/coffee-script.js index f5928ec4..61df6733 100644 --- a/lib/coffee-script/coffee-script.js +++ b/lib/coffee-script/coffee-script.js @@ -40,13 +40,25 @@ }; exports.compile = compile = withPrettyErrors(function(code, options) { - var answer, currentColumn, currentLine, extend, fragment, fragments, header, js, map, merge, newLines, _i, _len; + var answer, currentColumn, currentLine, extend, fragment, fragments, header, js, map, merge, newLines, token, tokens, _i, _len; merge = helpers.merge, extend = helpers.extend; options = extend({}, options); if (options.sourceMap) { map = new SourceMap; } - fragments = parser.parse(lexer.tokenize(code, options)).compileToFragments(options); + tokens = lexer.tokenize(code, options); + options.referencedVars = (function() { + var _i, _len, _results; + _results = []; + for (_i = 0, _len = tokens.length; _i < _len; _i++) { + token = tokens[_i]; + if (token.variable && token[1].charAt(0) === '_') { + _results.push(token[1]); + } + } + return _results; + })(); + fragments = parser.parse(tokens).compileToFragments(options); currentLine = 0; if (options.header) { currentLine += 1; @@ -236,8 +248,8 @@ } return tag; }, - setInput: function(tokens) { - this.tokens = tokens; + setInput: function(_at_tokens) { + this.tokens = _at_tokens; return this.pos = 0; }, upcomingInput: function() { diff --git a/lib/coffee-script/lexer.js b/lib/coffee-script/lexer.js index e1f7fcc7..5a078c83 100644 --- a/lib/coffee-script/lexer.js +++ b/lib/coffee-script/lexer.js @@ -139,6 +139,7 @@ })(); } tagToken = this.token(tag, id, 0, idLength); + tagToken.variable = !forcedIdentifier; if (poppedToken) { _ref4 = [poppedToken[2].first_line, poppedToken[2].first_column], tagToken[2].first_line = _ref4[0], tagToken[2].first_column = _ref4[1]; } diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index aea906d2..36e59ea9 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -417,7 +417,7 @@ o.indent = o.bare ? '' : TAB; o.level = LEVEL_TOP; this.spaced = true; - o.scope = new Scope(null, this, null); + o.scope = new Scope(null, this, null, o.referencedVars); _ref2 = o.locals || []; for (_i = 0, _len = _ref2.length; _i < _len; _i++) { name = _ref2[_i]; @@ -517,8 +517,8 @@ exports.Literal = Literal = (function(_super) { __extends(Literal, _super); - function Literal(value) { - this.value = value; + function Literal(_at_value) { + this.value = _at_value; } Literal.prototype.makeReturn = function() { @@ -617,8 +617,8 @@ return [this.makeCode(this.val)]; }; - function Bool(val) { - this.val = val; + function Bool(_at_val) { + this.val = _at_val; } return Bool; @@ -628,8 +628,8 @@ exports.Return = Return = (function(_super) { __extends(Return, _super); - function Return(expression) { - this.expression = expression; + function Return(_at_expression) { + this.expression = _at_expression; } Return.prototype.children = ['expression']; @@ -850,8 +850,8 @@ exports.Comment = Comment = (function(_super) { __extends(Comment, _super); - function Comment(comment) { - this.comment = comment; + function Comment(_at_comment) { + this.comment = _at_comment; } Comment.prototype.isStatement = YES; @@ -875,9 +875,9 @@ exports.Call = Call = (function(_super) { __extends(Call, _super); - function Call(variable, args, soak) { - this.args = args != null ? args : []; - this.soak = soak; + function Call(variable, _at_args, _at_soak) { + this.args = _at_args != null ? _at_args : []; + this.soak = _at_soak; this.isNew = false; this.isSuper = variable === 'super'; this.variable = this.isSuper ? null : variable; @@ -1046,9 +1046,9 @@ exports.Extends = Extends = (function(_super) { __extends(Extends, _super); - function Extends(child, parent) { - this.child = child; - this.parent = parent; + function Extends(_at_child, _at_parent) { + this.child = _at_child; + this.parent = _at_parent; } Extends.prototype.children = ['child', 'parent']; @@ -1064,8 +1064,8 @@ exports.Access = Access = (function(_super) { __extends(Access, _super); - function Access(name, tag) { - this.name = name; + function Access(_at_name, tag) { + this.name = _at_name; this.name.asKey = true; this.soak = tag === 'soak'; } @@ -1093,8 +1093,8 @@ exports.Index = Index = (function(_super) { __extends(Index, _super); - function Index(index) { - this.index = index; + function Index(_at_index) { + this.index = _at_index; } Index.prototype.children = ['index']; @@ -1116,9 +1116,9 @@ Range.prototype.children = ['from', 'to']; - function Range(from, to, tag) { - this.from = from; - this.to = to; + function Range(_at_from, _at_to, tag) { + this.from = _at_from; + this.to = _at_to; this.exclusive = tag === 'exclusive'; this.equals = this.exclusive ? '' : '='; } @@ -1214,8 +1214,8 @@ Slice.prototype.children = ['range']; - function Slice(range) { - this.range = range; + function Slice(_at_range) { + this.range = _at_range; Slice.__super__.constructor.call(this); } @@ -1240,8 +1240,8 @@ exports.Obj = Obj = (function(_super) { __extends(Obj, _super); - function Obj(props, generated) { - this.generated = generated != null ? generated : false; + function Obj(props, _at_generated) { + this.generated = _at_generated != null ? _at_generated : false; this.objects = this.properties = props || []; } @@ -1379,10 +1379,10 @@ exports.Class = Class = (function(_super) { __extends(Class, _super); - function Class(variable, parent, body) { - this.variable = variable; - this.parent = parent; - this.body = body != null ? body : new Block; + function Class(_at_variable, _at_parent, _at_body) { + this.variable = _at_variable; + this.parent = _at_parent; + this.body = _at_body != null ? _at_body : new Block; this.boundFuncs = []; this.body.classBody = true; } @@ -1566,11 +1566,11 @@ exports.Assign = Assign = (function(_super) { __extends(Assign, _super); - function Assign(variable, value, context, options) { + function Assign(_at_variable, _at_value, _at_context, options) { var forbidden, name, _ref2; - this.variable = variable; - this.value = value; - this.context = context; + this.variable = _at_variable; + this.value = _at_value; + this.context = _at_context; this.param = options && options.param; this.subpattern = options && options.subpattern; forbidden = (_ref2 = (name = this.variable.unwrapAll().value), __indexOf.call(STRICT_PROSCRIBED, _ref2) >= 0); @@ -1875,14 +1875,10 @@ _ref5 = this.params; for (_k = 0, _len2 = _ref5.length; _k < _len2; _k++) { p = _ref5[_k].name; - if (!(!(param instanceof Expansion))) { - continue; - } - if (p["this"]) { - p = p.properties[0].name; - } - if (p.value) { - o.scope.add(p.value, 'var', true); + if (!(param instanceof Expansion)) { + if (p.value) { + o.scope.add(p.value, 'var', true); + } } } splats = new Assign(new Value(new Arr((function() { @@ -1935,7 +1931,7 @@ uniqs = []; this.eachParamName(function(name, node) { if (__indexOf.call(uniqs, name) >= 0) { - node.error("multiple parameters named '" + name + "'"); + node.error("multiple parameters named " + name); } return uniqs.push(name); }); @@ -1997,11 +1993,11 @@ exports.Param = Param = (function(_super) { __extends(Param, _super); - function Param(name, value, splat) { - var _ref2; - this.name = name; - this.value = value; - this.splat = splat; + function Param(_at_name, _at_value, _at_splat) { + var name, _ref2; + this.name = _at_name; + this.value = _at_value; + this.splat = _at_splat; if (_ref2 = (name = this.name.unwrapAll().value), __indexOf.call(STRICT_PROSCRIBED, _ref2) >= 0) { this.name.error("parameter name \"" + name + "\" is not allowed"); } @@ -2014,16 +2010,14 @@ }; Param.prototype.asReference = function(o) { - var node; + var name, node; if (this.reference) { return this.reference; } node = this.name; if (node["this"]) { - node = node.properties[0].name; - if (node.value.reserved) { - node = new Literal(o.scope.freeVariable(node.value)); - } + name = "at_" + node.properties[0].name.value; + node = new Literal(o.scope.freeVariable(name)); } else if (node.isComplex()) { node = new Literal(o.scope.freeVariable('arg')); } @@ -2045,11 +2039,7 @@ name = this.name; } atParam = function(obj) { - var node; - node = obj.properties[0].name; - if (!node.value.reserved) { - return iterator(node.value, node); - } + return iterator("@" + obj.properties[0].name.value, obj); }; if (name instanceof Literal) { return iterator(name.value, name); @@ -2199,8 +2189,8 @@ } }; - While.prototype.addBody = function(body) { - this.body = body; + While.prototype.addBody = function(_at_body) { + this.body = _at_body; return this; }; @@ -2514,9 +2504,9 @@ exports.In = In = (function(_super) { __extends(In, _super); - function In(object, array) { - this.object = object; - this.array = array; + function In(_at_object, _at_array) { + this.object = _at_object; + this.array = _at_array; } In.prototype.children = ['object', 'array']; @@ -2588,11 +2578,11 @@ exports.Try = Try = (function(_super) { __extends(Try, _super); - function Try(attempt, errorVariable, recovery, ensure) { - this.attempt = attempt; - this.errorVariable = errorVariable; - this.recovery = recovery; - this.ensure = ensure; + function Try(_at_attempt, _at_errorVariable, _at_recovery, _at_ensure) { + this.attempt = _at_attempt; + this.errorVariable = _at_errorVariable; + this.recovery = _at_recovery; + this.ensure = _at_ensure; } Try.prototype.children = ['attempt', 'recovery', 'ensure']; @@ -2630,8 +2620,8 @@ exports.Throw = Throw = (function(_super) { __extends(Throw, _super); - function Throw(expression) { - this.expression = expression; + function Throw(_at_expression) { + this.expression = _at_expression; } Throw.prototype.children = ['expression']; @@ -2653,8 +2643,8 @@ exports.Existence = Existence = (function(_super) { __extends(Existence, _super); - function Existence(expression) { - this.expression = expression; + function Existence(_at_expression) { + this.expression = _at_expression; } Existence.prototype.children = ['expression']; @@ -2681,8 +2671,8 @@ exports.Parens = Parens = (function(_super) { __extends(Parens, _super); - function Parens(body) { - this.body = body; + function Parens(_at_body) { + this.body = _at_body; } Parens.prototype.children = ['body']; @@ -2894,10 +2884,10 @@ exports.Switch = Switch = (function(_super) { __extends(Switch, _super); - function Switch(subject, cases, otherwise) { - this.subject = subject; - this.cases = cases; - this.otherwise = otherwise; + function Switch(_at_subject, _at_cases, _at_otherwise) { + this.subject = _at_subject; + this.cases = _at_cases; + this.otherwise = _at_otherwise; } Switch.prototype.children = ['subject', 'cases', 'otherwise']; @@ -2979,8 +2969,8 @@ exports.If = If = (function(_super) { __extends(If, _super); - function If(condition, body, options) { - this.body = body; + function If(condition, _at_body, options) { + this.body = _at_body; if (options == null) { options = {}; } diff --git a/lib/coffee-script/optparse.js b/lib/coffee-script/optparse.js index a90f0e62..ca63970a 100644 --- a/lib/coffee-script/optparse.js +++ b/lib/coffee-script/optparse.js @@ -5,8 +5,8 @@ repeat = require('./helpers').repeat; exports.OptionParser = OptionParser = (function() { - function OptionParser(rules, banner) { - this.banner = banner; + function OptionParser(rules, _at_banner) { + this.banner = _at_banner; this.rules = buildRules(rules); } diff --git a/lib/coffee-script/rewriter.js b/lib/coffee-script/rewriter.js index ebe668ba..9f14335d 100644 --- a/lib/coffee-script/rewriter.js +++ b/lib/coffee-script/rewriter.js @@ -17,8 +17,8 @@ exports.Rewriter = (function() { function Rewriter() {} - Rewriter.prototype.rewrite = function(tokens) { - this.tokens = tokens; + Rewriter.prototype.rewrite = function(_at_tokens) { + this.tokens = _at_tokens; this.removeLeadingNewlines(); this.closeOpenCalls(); this.closeOpenIndexes(); diff --git a/lib/coffee-script/scope.js b/lib/coffee-script/scope.js index 60289050..75fc7eb4 100644 --- a/lib/coffee-script/scope.js +++ b/lib/coffee-script/scope.js @@ -1,16 +1,18 @@ // Generated by CoffeeScript 1.8.0 (function() { - var Scope, extend, last, _ref; + var Scope, extend, last, _ref, + __indexOf = [].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; }; _ref = require('./helpers'), extend = _ref.extend, last = _ref.last; exports.Scope = Scope = (function() { Scope.root = null; - function Scope(parent, expressions, method) { - this.parent = parent; - this.expressions = expressions; - this.method = method; + function Scope(_at_parent, _at_expressions, _at_method, _at_referencedVars) { + this.parent = _at_parent; + this.expressions = _at_expressions; + this.method = _at_method; + this.referencedVars = _at_referencedVars; this.variables = [ { name: 'arguments', @@ -91,7 +93,11 @@ reserve = true; } index = 0; - while (this.check((temp = this.temporary(name, index)))) { + while (true) { + temp = this.temporary(name, index); + if (!(this.check(temp) || __indexOf.call(Scope.root.referencedVars, temp) >= 0)) { + break; + } index++; } if (reserve) { diff --git a/lib/coffee-script/sourcemap.js b/lib/coffee-script/sourcemap.js index c94ebd17..b9927e1b 100644 --- a/lib/coffee-script/sourcemap.js +++ b/lib/coffee-script/sourcemap.js @@ -3,8 +3,8 @@ var LineMap, SourceMap; LineMap = (function() { - function LineMap(line) { - this.line = line; + function LineMap(_at_line) { + this.line = _at_line; this.columns = []; } diff --git a/src/coffee-script.coffee b/src/coffee-script.coffee index ef711238..b86f4d7c 100644 --- a/src/coffee-script.coffee +++ b/src/coffee-script.coffee @@ -44,7 +44,17 @@ exports.compile = compile = withPrettyErrors (code, options) -> if options.sourceMap map = new SourceMap - fragments = parser.parse(lexer.tokenize code, options).compileToFragments options + tokens = lexer.tokenize code, options + + # Pass a list of referenced variables, so that generated variables won't get + # the same name. Since all generated variables start with an underscore only + # referenced variables also starting with an underscore are passed, as an + # optimization. + options.referencedVars = ( + token[1] for token in tokens when token.variable and token[1].charAt(0) is '_' + ) + + fragments = parser.parse(tokens).compileToFragments options currentLine = 0 currentLine += 1 if options.header diff --git a/src/lexer.coffee b/src/lexer.coffee index e712e67a..5a462e47 100644 --- a/src/lexer.coffee +++ b/src/lexer.coffee @@ -156,6 +156,7 @@ exports.Lexer = class Lexer else tag tagToken = @token tag, id, 0, idLength + tagToken.variable = not forcedIdentifier if poppedToken [tagToken[2].first_line, tagToken[2].first_column] = [poppedToken[2].first_line, poppedToken[2].first_column] diff --git a/src/nodes.coffee b/src/nodes.coffee index 48062709..20ff9e90 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -320,7 +320,7 @@ exports.Block = class Block extends Base o.indent = if o.bare then '' else TAB o.level = LEVEL_TOP @spaced = yes - o.scope = new Scope null, this, null + o.scope = new Scope null, this, null, o.referencedVars # Mark given local variables in the root scope as parameters so they don't # end up being declared on this block. o.scope.parameter name for name in o.locals or [] @@ -1361,7 +1361,6 @@ exports.Code = class Code extends Base o.scope.parameter param.asReference o for param in @params when param.splat or param instanceof Expansion for {name: p} in @params when param not instanceof Expansion - if p.this then p = p.properties[0].name if p.value then o.scope.add p.value, 'var', yes splats = new Assign new Value(new Arr(p.asReference o for p in @params)), new Value new Literal 'arguments' @@ -1386,7 +1385,7 @@ exports.Code = class Code extends Base o.scope.parameter fragmentsToText params[i] uniqs = [] @eachParamName (name, node) -> - node.error "multiple parameters named '#{name}'" if name in uniqs + node.error "multiple parameters named #{name}" if name in uniqs uniqs.push name @body.makeReturn() unless wasEmpty or @noReturn code = 'function' @@ -1431,9 +1430,8 @@ exports.Param = class Param extends Base return @reference if @reference node = @name if node.this - node = node.properties[0].name - if node.value.reserved - node = new Literal o.scope.freeVariable node.value + name = "at_#{node.properties[0].name.value}" + node = new Literal o.scope.freeVariable name else if node.isComplex() node = new Literal o.scope.freeVariable 'arg' node = new Value node @@ -1451,9 +1449,7 @@ exports.Param = class Param extends Base # `name` is the name of the parameter and `node` is the AST node corresponding # to that name. eachName: (iterator, name = @name)-> - atParam = (obj) -> - node = obj.properties[0].name - iterator node.value, node unless node.value.reserved + atParam = (obj) -> iterator "@#{obj.properties[0].name.value}", obj # * simple literals `foo` return iterator name.value, name if name instanceof Literal # * at-params `@foo` diff --git a/src/scope.litcoffee b/src/scope.litcoffee index 23128cfa..132dc078 100644 --- a/src/scope.litcoffee +++ b/src/scope.litcoffee @@ -17,10 +17,11 @@ The `root` is the top-level **Scope** object for a given file. Initialize a scope with its parent, for lookups up the chain, as well as a reference to the **Block** node it belongs to, which is -where it should declare its variables, and a reference to the function that -it belongs to. +where it should declare its variables, a reference to the function that +it belongs to, and a list of variables referenced in the source code +and therefore should be avoided when generating variables. - constructor: (@parent, @expressions, @method) -> + constructor: (@parent, @expressions, @method, @referencedVars) -> @variables = [{name: 'arguments', type: 'arguments'}] @positions = {} Scope.root = this unless @parent @@ -84,7 +85,10 @@ compiler-generated variable. `_var`, `_var2`, and so on... freeVariable: (name, reserve=true) -> index = 0 - index++ while @check((temp = @temporary name, index)) + loop + temp = @temporary name, index + break unless @check(temp) or temp in Scope.root.referencedVars + index++ @add temp, 'var', yes if reserve temp diff --git a/test/assignment.coffee b/test/assignment.coffee index 7bdde64c..9753d3da 100644 --- a/test/assignment.coffee +++ b/test/assignment.coffee @@ -6,6 +6,7 @@ # * Destructuring Assignment # * Context Property (@) Assignment # * Existential Assignment (?=) +# * Assignment to variables similar to generated variables test "context property assignment (using @)", -> nonce = {} @@ -405,3 +406,34 @@ test "#2181: conditional assignment as a subexpression", -> false && a or= true eq false, a eq false, not a or= true + +test "#1500: Assignment to variables similar to generated variables", -> + _len = 0 + x = ((_results = null; i) for i in [1, 2, 3]) + arrayEq [1, 2, 3], x + eq 0, _len + + for x in [1, 2, 3] + f = -> + _i = 0 + f() + eq 'undefined', typeof _i + + _ref = 2 + x = _ref * 2 ? 1 + eq x, 4 + eq 'undefined', typeof _ref1 + + x = {} + _base = -> x + _name = -1 + _base()[-_name] ?= 2 + eq x[1], 2 + eq _base(), x + eq _name, -1 + + f = (@a, _at_a, a) -> [@a, _at_a, a] + arrayEq [1, 2, 3], f.call scope = {}, 1, 2, 3 + eq 1, scope.a + + doesNotThrow -> CoffeeScript.compile '(@_slice...) ->' diff --git a/test/error_messages.coffee b/test/error_messages.coffee index 2aa9e7f1..e95386be 100644 --- a/test/error_messages.coffee +++ b/test/error_messages.coffee @@ -431,3 +431,19 @@ test "unclosed regexes", -> a #{""" ""#{if /[/].test "|" then 1 else 0}"" """} ^ ''' + +test "duplicate function arguments", -> + assertErrorFormat ''' + (foo, bar, foo) -> + ''', ''' + [stdin]:1:12: error: multiple parameters named foo + (foo, bar, foo) -> + ^^^ + ''' + assertErrorFormat ''' + (@foo, bar, @foo) -> + ''', ''' + [stdin]:1:13: error: multiple parameters named @foo + (@foo, bar, @foo) -> + ^^^^ + ''' diff --git a/test/functions.coffee b/test/functions.coffee index d9cd5830..e1721847 100644 --- a/test/functions.coffee +++ b/test/functions.coffee @@ -116,8 +116,13 @@ test "@-parameters: automatically assign an argument's value to a property of th ((@prop...) ->).call context = {}, 0, nonce, 0 eq nonce, context.prop[1] - # the argument should still be able to be referenced normally - eq nonce, (((@prop) -> prop).call {}, nonce) + # the argument should not be able to be referenced normally + code = '((@prop) -> prop).call {}' + doesNotThrow -> CoffeeScript.compile code + throws (-> CoffeeScript.run code), ReferenceError + code = '((@prop) -> _at_prop).call {}' + doesNotThrow -> CoffeeScript.compile code + throws (-> CoffeeScript.run code), ReferenceError test "@-parameters and splats with constructors", -> a = {} @@ -213,6 +218,16 @@ test "reserved keyword as parameters", -> eq 2, b eq 3, c +test "reserved keyword at-splat", -> + f = (@case...) -> @case + [a, b] = f(1, 2) + eq 1, a + eq 2, b + +test "#1574: Destructuring and a parameter named _arg", -> + f = ({a, b}, _arg, _arg1) -> [a, b, _arg, _arg1] + arrayEq [1, 2, 3, 4], f a: 1, b: 2, 3, 4 + test "#1844: bound functions in nested comprehensions causing empty var statements", -> a = ((=>) for a in [0] for b in [0]) eq 1, a.length diff --git a/test/strict.coffee b/test/strict.coffee index 16a5605a..03fb56ec 100644 --- a/test/strict.coffee +++ b/test/strict.coffee @@ -72,19 +72,14 @@ test "duplicate formal parameters are prohibited", -> # a Param can also be a splat (...) or an assignment (param=value) # the following function expressions should throw errors strict '(_,_)->', 'param, param' - strict '(_,@_)->', 'param, @param' strict '(_,_...)->', 'param, param...' - strict '(@_,_...)->', '@param, param...' strict '(_,_ = true)->', 'param, param=' strict '(@_,@_)->', 'two @params' - strict '(_,@_ = true)->', 'param, @param=' + strict '(@case,@case)->', 'two @reserved' strict '(_,{_})->', 'param, {param}' - strict '(@_,{_})->', '@param, {param}' strict '({_,_})->', '{param, param}' - strict '({_,@_})->', '{param, @param}' strict '(_,[_])->', 'param, [param]' strict '([_,_])->', '[param, param]' - strict '([_,@_])->', '[param, @param]' strict '(_,[_]=true)->', 'param, [param]=' strict '(_,[@_,{_}])->', 'param, [@param, {param}]' strict '(_,[_,{@_}])->', 'param, [param, {@param}]' @@ -95,6 +90,12 @@ test "duplicate formal parameters are prohibited", -> strict '(0:a,1:a)->', '0:param,1:param' strict '({0:a,1:a})->', '{0:param,1:param}' # the following function expressions should **not** throw errors + strictOk '(_,@_)->' + strictOk '(@_,_...)->' + strictOk '(_,@_ = true)->' + strictOk '(@_,{_})->' + strictOk '({_,@_})->' + strictOk '([_,@_])->' strictOk '({},_arg)->' strictOk '({},{})->' strictOk '([]...,_arg)->' @@ -113,7 +114,6 @@ test "`delete` operand restrictions", -> strict 'a = 1; delete a' strictOk 'delete a' #noop strict '(a) -> delete a' - strict '(@a) -> delete a' strict '(a...) -> delete a' strict '(a = 1) -> delete a' strict '([a]) -> delete a' From a46978640b731b53d1580b68a1c2d2781506c090 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sun, 11 Jan 2015 12:12:40 +0100 Subject: [PATCH 3/3] Allow variables named like helper functions --- lib/coffee-script/lexer.js | 2 +- lib/coffee-script/nodes.js | 12 ++++++++---- lib/coffee-script/scope.js | 1 + src/lexer.coffee | 5 ++--- src/nodes.coffee | 9 ++++++--- src/scope.litcoffee | 4 +++- test/assignment.coffee | 20 ++++++++++++++++++++ 7 files changed, 41 insertions(+), 12 deletions(-) diff --git a/lib/coffee-script/lexer.js b/lib/coffee-script/lexer.js index 5a078c83..fe5bd6f9 100644 --- a/lib/coffee-script/lexer.js +++ b/lib/coffee-script/lexer.js @@ -813,7 +813,7 @@ COFFEE_KEYWORDS = COFFEE_KEYWORDS.concat(COFFEE_ALIASES); - RESERVED = ['case', 'default', 'function', 'var', 'void', 'with', 'const', 'let', 'enum', 'export', 'import', 'native', '__hasProp', '__extends', '__slice', '__bind', '__indexOf', 'implements', 'interface', 'package', 'private', 'protected', 'public', 'static']; + RESERVED = ['case', 'default', 'function', 'var', 'void', 'with', 'const', 'let', 'enum', 'export', 'import', 'native', 'implements', 'interface', 'package', 'private', 'protected', 'public', 'static']; STRICT_PROSCRIBED = ['arguments', 'eval', 'yield*']; diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index 36e59ea9..f9eeb9a4 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -1,8 +1,8 @@ // Generated by CoffeeScript 1.8.0 (function() { var Access, Arr, Assign, Base, Block, Call, Class, Code, CodeFragment, Comment, Existence, Expansion, Extends, For, HEXNUM, IDENTIFIER, IDENTIFIER_STR, IS_REGEX, IS_STRING, If, In, Index, LEVEL_ACCESS, LEVEL_COND, LEVEL_LIST, LEVEL_OP, LEVEL_PAREN, LEVEL_TOP, Literal, METHOD_DEF, NEGATE, NO, NUMBER, Obj, Op, Param, Parens, RESERVED, Range, Return, SIMPLENUM, STRICT_PROSCRIBED, Scope, Slice, Splat, Switch, TAB, THIS, Throw, Try, UTILITIES, Value, While, YES, addLocationDataFn, compact, del, ends, extend, flatten, fragmentsToText, isLiteralArguments, isLiteralThis, last, locationDataToString, merge, multident, parseNum, some, starts, throwSyntaxError, unfoldSoak, utility, _ref, _ref1, - __hasProp = {}.hasOwnProperty, __extends = function(child, parent) { for (var key in parent) { if (__hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; }, + __hasProp = {}.hasOwnProperty, __indexOf = [].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; }, __slice = [].slice; @@ -3145,9 +3145,13 @@ utility = function(name) { var ref; - ref = "__" + name; - Scope.root.assign(ref, UTILITIES[name]()); - return ref; + if (name in Scope.root.utilities) { + return Scope.root.utilities[name]; + } else { + ref = Scope.root.freeVariable("_" + name); + Scope.root.assign(ref, UTILITIES[name]()); + return Scope.root.utilities[name] = ref; + } }; multident = function(code, tab) { diff --git a/lib/coffee-script/scope.js b/lib/coffee-script/scope.js index 75fc7eb4..2ffb641d 100644 --- a/lib/coffee-script/scope.js +++ b/lib/coffee-script/scope.js @@ -21,6 +21,7 @@ ]; this.positions = {}; if (!this.parent) { + this.utilities = {}; Scope.root = this; } } diff --git a/src/lexer.coffee b/src/lexer.coffee index 5a462e47..058fbcf7 100644 --- a/src/lexer.coffee +++ b/src/lexer.coffee @@ -718,9 +718,8 @@ COFFEE_KEYWORDS = COFFEE_KEYWORDS.concat COFFEE_ALIASES # to avoid having a JavaScript error at runtime. RESERVED = [ 'case', 'default', 'function', 'var', 'void', 'with', 'const', 'let', 'enum' - 'export', 'import', 'native', '__hasProp', '__extends', '__slice', '__bind' - '__indexOf', 'implements', 'interface', 'package', 'private', 'protected' - 'public', 'static' + 'export', 'import', 'native', 'implements', 'interface', 'package', 'private' + 'protected', 'public', 'static' ] STRICT_PROSCRIBED = ['arguments', 'eval', 'yield*'] diff --git a/src/nodes.coffee b/src/nodes.coffee index 20ff9e90..d624b700 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -2260,9 +2260,12 @@ IS_REGEX = /^\// # Helper for ensuring that utility functions are assigned at the top level. utility = (name) -> - ref = "__#{name}" - Scope.root.assign ref, UTILITIES[name]() - ref + if name of Scope.root.utilities + Scope.root.utilities[name] + else + ref = Scope.root.freeVariable "_#{name}" + Scope.root.assign ref, UTILITIES[name]() + Scope.root.utilities[name] = ref multident = (code, tab) -> code = code.replace /\n/g, '$&' + tab diff --git a/src/scope.litcoffee b/src/scope.litcoffee index 132dc078..ab813ece 100644 --- a/src/scope.litcoffee +++ b/src/scope.litcoffee @@ -24,7 +24,9 @@ and therefore should be avoided when generating variables. constructor: (@parent, @expressions, @method, @referencedVars) -> @variables = [{name: 'arguments', type: 'arguments'}] @positions = {} - Scope.root = this unless @parent + unless @parent + @utilities = {} + Scope.root = this Adds a new variable or overrides an existing one. diff --git a/test/assignment.coffee b/test/assignment.coffee index 9753d3da..0ede9867 100644 --- a/test/assignment.coffee +++ b/test/assignment.coffee @@ -437,3 +437,23 @@ test "#1500: Assignment to variables similar to generated variables", -> eq 1, scope.a doesNotThrow -> CoffeeScript.compile '(@_slice...) ->' + +test "Assignment to variables similar to helper functions", -> + f = (__slice...) -> __slice + arrayEq [1, 2, 3], f 1, 2, 3 + eq 'undefined', typeof __slice1 + + class A + class B extends A + __extends = 3 + __hasProp = 4 + value: 5 + method: (__bind, __bind1) => [__bind, __bind1, __extends, __hasProp, @value] + {method} = new B + arrayEq [1, 2, 3, 4, 5], method 1, 2 + + __modulo = -1 %% 3 + eq 2, __modulo + + __indexOf = [1, 2, 3] + ok 2 in __indexOf