Permalink
Browse files

finally put #2333 to rest by resorting to using indirect eval

related: #1772, #1776; we're relying on the underlying engine having the
string escaping behaviour we want instead of implementing it manually.
  • Loading branch information...
1 parent 6838bae commit 81f780f1fbf25985709e51191c8f5506b79cc278 @michaelficarra michaelficarra committed May 21, 2012
Showing with 39 additions and 98 deletions.
  1. +10 −55 lib/coffee-script/nodes.js
  2. +4 −22 src/nodes.coffee
  3. +25 −21 test/strict.coffee
@@ -1172,61 +1172,15 @@
Obj.prototype.children = ['properties'];
Obj.prototype.compileNode = function(o) {
- var i, idt, indent, isDuplicate, join, lastNoncom, node, normaliseString, obj, prop, propName, propNames, props, _i, _j, _len, _len1, _ref2;
+ var i, idt, indent, join, lastNoncom, node, normalise, normalisedPropName, obj, prop, propName, propNames, props, _i, _j, _len, _len1, _ref2;
props = this.properties;
propNames = [];
- normaliseString = function(s) {
- var escapeNormalised, quoteNormalised;
- quoteNormalised = (function() {
- switch (s[0]) {
- case '"':
- return s.slice(1, -1).replace(/\\"/g, '"');
- case "'":
- return s.slice(1, -1).replace(/\\'/g, "'");
- }
- })();
- escapeNormalised = quoteNormalised.replace(/\\x([0-9a-f]{2})|\\u([0-9a-f]{4})/ig, function(match, h, u) {
- return String.fromCharCode(parseInt(h != null ? h : u, 16));
- });
- return escapeNormalised.replace(/\\([\s\S])/g, function(match, c) {
- var _ref2;
- return (_ref2 = {
- 0: "\0",
- b: "\b",
- t: "\t",
- n: "\n",
- v: "\v",
- f: "\f",
- r: "\r",
- "\\": "\\",
- "\n": ""
- }[c]) != null ? _ref2 : c;
- });
- };
- isDuplicate = function(x) {
- var mx;
- mx = x.match(/^['"]/);
- return function(y) {
- var my;
- if (y === x || +y === +x) {
- return true;
- }
- my = y.match(/^['"]/);
- if (mx && my) {
- if (normaliseString(x) === normaliseString(y)) {
- return true;
- }
- } else if (mx) {
- if (y === normaliseString(x)) {
- return true;
- }
- } else if (my) {
- if (x === normaliseString(y)) {
- return true;
- }
- }
- return false;
- };
+ normalise = function(v) {
+ if (IDENTIFIER.test(v)) {
+ return v;
+ } else {
+ return "" + Function("return " + v)();
+ }
};
_ref2 = this.properties;
for (_i = 0, _len = _ref2.length; _i < _len; _i++) {
@@ -1236,10 +1190,11 @@
}
if (prop != null) {
propName = prop.unwrapAll().value.toString();
- if (some.call(propNames, isDuplicate(propName))) {
+ normalisedPropName = normalise(propName);
+ if (__indexOf.call(propNames, normalisedPropName) >= 0) {
throw SyntaxError("multiple object literal properties named \"" + propName + "\"");
}
- propNames.push(propName);
+ propNames.push(normalisedPropName);
}
}
if (!props.length) {
View
@@ -802,33 +802,15 @@ exports.Obj = class Obj extends Base
compileNode: (o) ->
props = @properties
propNames = []
- normaliseString = (s) ->
- quoteNormalised = switch s[0]
- when '"' then s[1...-1].replace /\\"/g, '"'
- when "'" then s[1...-1].replace /\\'/g, "'"
- escapeNormalised = quoteNormalised.replace /\\x([0-9a-f]{2})|\\u([0-9a-f]{4})/ig, (match, h, u) ->
- String.fromCharCode parseInt (h ? u), 16
- escapeNormalised.replace /\\([\s\S])/g, (match, c) ->
- {0:"\0", b:"\b", t:"\t", n:"\n", v:"\v", f:"\f", r:"\r", "\\":"\\", "\n":""}[c] ? c
- isDuplicate = (x) ->
- mx = x.match /^['"]/
- (y) ->
- return true if y is x or +y is +x
- my = y.match /^['"]/
- if mx and my
- return true if normaliseString(x) is normaliseString y
- else if mx
- return true if y is normaliseString x
- else if my
- return true if x is normaliseString y
- false
+ normalise = (v) -> if IDENTIFIER.test v then v else "" + do Function "return #{v}"
for prop in @properties
prop = prop.variable if prop.isComplex()
if prop?
propName = prop.unwrapAll().value.toString()
- if some.call propNames, isDuplicate propName
+ normalisedPropName = normalise propName
+ if normalisedPropName in propNames
throw SyntaxError "multiple object literal properties named \"#{propName}\""
- propNames.push propName
+ propNames.push normalisedPropName
return (if @front then '({})' else '{}') unless props.length
if @generated
for node in props when node instanceof Value
View
@@ -61,27 +61,31 @@ test "duplicate property definitions in object literals are prohibited", ->
strict 'x = 1; o = {x, x: 2}'
test "#2333: more duplicate property prohibitions", ->
- strict '{a:0, "a":0}'
- strict "{'a':0, a:0}"
- strict '{\'a\':0, "a":0}'
- strict '{0:0, 0x0:0}'
- strict '{0:0, "\\x30":0}'
- strict '{.1:0, 0.1:0}'
- strict '{.1:0, 1e-1:0}'
- strict '{100:0, 1e2:0}'
- strict '{"\\0":0, "\\x00":0}'
- strict '{"\\t":0, "\\x09":0}'
- strict '{"c":0, "\\c":0}'
- strict '{"\\\\":0, "\\x5c":0}'
- strict "{'\n0':0, 0:0}"
- strict '{"\n0":0, 0:0}'
- strict "{'\\\n0':0, 0:0}"
- strict '{"\\\n0":0, "\\x00":0}'
- strict 'a = 0; {a, "a":0}'
- strict "{'\\'a':0, \"'a\":0}"
- strict "{'\\\\a':0, '\\\\a':0}"
- strictOk '{0:0, "0x0":0}'
- strictOk '{"a":0, "\'a\'":0}'
+ usingKeys = (a, b) -> "{#{a}:0, #{b}:0}"
+ strict '{a, "a":0}'
+ strict usingKeys "a", '"a"'
+ strict usingKeys "'a'", "a"
+ strict usingKeys "'a'", '"a"'
+ strict usingKeys "'a'", "'a'"
+ strict usingKeys "0", "0x0"
+ strict usingKeys "0", "'\\x30'"
+ strict usingKeys ".1", "0.1"
+ strict usingKeys ".1", "1e-1"
+ strict usingKeys "100", "1e2"
+ strict usingKeys "'\\0'", "'\\x00'"
+ strict usingKeys "'\\t'", "'\\x09'"
+ strict usingKeys "'\\\\x00'", "'\\\\\\x7800'"
+ strict usingKeys "'c'", "'\\c'"
+ strict usingKeys "'\\\\'", "'\\x5c'"
+ strict usingKeys "'\\\n0'", "0"
+ strict usingKeys "'\\\n0'", "'\\x00'"
+ strict usingKeys "{'\\'a'", "\"'a\""
+ strict usingKeys "{'\\\\a'", "'\\\\a'"
+ strictOk usingKeys "a", "b"
+ strictOk usingKeys "'\"a\"'", "'a'"
+ strictOk usingKeys "'\"a\"'", '"\'a\'"'
+ strictOk usingKeys "0", '"0x0"'
+ strictOk usingKeys "0", '"\\\\x30"'
test "duplicate formal parameters are prohibited", ->
nonce = {}

3 comments on commit 81f780f

@jashkenas
Owner

"normalize" is with a "z" ... and can we put all this on a branch for the time being, instead of having it on master? We shouldn't be doing backflips or indirect evals here.

@michaelficarra
Collaborator

"normalize" is with a "z"

That's more of an opinion. I can change it, though, if you'd like.

can we put all this on a branch for the time being

Sure, but we do want to disallow duplicate keys in object initialisers, right? If so, why wouldn't this belong on master?

@jashkenas
Owner

Yep, let's follow American English style.

We do want to disallow duplicate keys in object literals, but let's leave it on a pull request until we have an implementation we're comfortable with.

Please sign in to comment.