From 16c3a9a082c7048633e996efd1500583e84da8a0 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sat, 4 Oct 2014 17:09:22 -0700 Subject: [PATCH] refactor($parse): separate tokenizing vs parsing more Fixes `a.b` and `a .b` generating different getterFns Fixes `{'': ...}` turning into `{"''": ...}` Fixes `{.: ...}` parsing as `{'.': ...}` instead of throwing Fixes #9131 --- src/ng/parse.js | 214 +++++++++++++------------------- test/ng/directive/ngBindSpec.js | 4 +- test/ng/parseSpec.js | 116 ++++++++++++----- 3 files changed, 174 insertions(+), 160 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index 846e11b77223..d905cdb975be 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -150,44 +150,31 @@ Lexer.prototype = { lex: function(text) { this.text = text; this.index = 0; - this.ch = undefined; this.tokens = []; while (this.index < this.text.length) { - this.ch = this.text.charAt(this.index); - if (this.is('"\'')) { - this.readString(this.ch); - } else if (this.isNumber(this.ch) || this.is('.') && this.isNumber(this.peek())) { + var ch = this.text.charAt(this.index); + if (ch === '"' || ch === "'") { + this.readString(ch); + } else if (this.isNumber(ch) || ch === '.' && this.isNumber(this.peek())) { this.readNumber(); - } else if (this.isIdent(this.ch)) { + } else if (this.isIdent(ch)) { this.readIdent(); - } else if (this.is('(){}[].,;:?')) { - this.tokens.push({ - index: this.index, - text: this.ch - }); + } else if (this.is(ch, '(){}[].,;:?')) { + this.tokens.push({index: this.index, text: ch}); this.index++; - } else if (this.isWhitespace(this.ch)) { + } else if (this.isWhitespace(ch)) { this.index++; } else { - var ch2 = this.ch + this.peek(); + var ch2 = ch + this.peek(); var ch3 = ch2 + this.peek(2); - var fn = OPERATORS[this.ch]; - var fn2 = OPERATORS[ch2]; - var fn3 = OPERATORS[ch3]; - if (fn3) { - this.tokens.push({index: this.index, text: ch3, fn: fn3}); - this.index += 3; - } else if (fn2) { - this.tokens.push({index: this.index, text: ch2, fn: fn2}); - this.index += 2; - } else if (fn) { - this.tokens.push({ - index: this.index, - text: this.ch, - fn: fn - }); - this.index += 1; + var op1 = OPERATORS[ch]; + var op2 = OPERATORS[ch2]; + var op3 = OPERATORS[ch3]; + if (op1 || op2 || op3) { + var token = op3 ? ch3 : (op2 ? ch2 : ch); + this.tokens.push({index: this.index, text: token, operator: true}); + this.index += token.length; } else { this.throwError('Unexpected next character ', this.index, this.index + 1); } @@ -196,8 +183,8 @@ Lexer.prototype = { return this.tokens; }, - is: function(chars) { - return chars.indexOf(this.ch) !== -1; + is: function(ch, chars) { + return chars.indexOf(ch) !== -1; }, peek: function(i) { @@ -206,7 +193,7 @@ Lexer.prototype = { }, isNumber: function(ch) { - return ('0' <= ch && ch <= '9'); + return ('0' <= ch && ch <= '9') && typeof ch === "string"; }, isWhitespace: function(ch) { @@ -259,79 +246,28 @@ Lexer.prototype = { } this.index++; } - number = 1 * number; this.tokens.push({ index: start, text: number, constant: true, - fn: function() { return number; } + value: Number(number) }); }, readIdent: function() { - var expression = this.text; - - var ident = ''; var start = this.index; - - var lastDot, peekIndex, methodName, ch; - while (this.index < this.text.length) { - ch = this.text.charAt(this.index); - if (ch === '.' || this.isIdent(ch) || this.isNumber(ch)) { - if (ch === '.') lastDot = this.index; - ident += ch; - } else { + var ch = this.text.charAt(this.index); + if (!(this.isIdent(ch) || this.isNumber(ch))) { break; } this.index++; } - - //check if the identifier ends with . and if so move back one char - if (lastDot && ident[ident.length - 1] === '.') { - this.index--; - ident = ident.slice(0, -1); - lastDot = ident.lastIndexOf('.'); - if (lastDot === -1) { - lastDot = undefined; - } - } - - //check if this is not a method invocation and if it is back out to last dot - if (lastDot) { - peekIndex = this.index; - while (peekIndex < this.text.length) { - ch = this.text.charAt(peekIndex); - if (ch === '(') { - methodName = ident.substr(lastDot - start + 1); - ident = ident.substr(0, lastDot - start); - this.index = peekIndex; - break; - } - if (this.isWhitespace(ch)) { - peekIndex++; - } else { - break; - } - } - } - this.tokens.push({ index: start, - text: ident, - fn: CONSTANTS[ident] || getterFn(ident, this.options, expression) + text: this.text.slice(start, this.index), + identifier: true }); - - if (methodName) { - this.tokens.push({ - index: lastDot, - text: '.' - }); - this.tokens.push({ - index: lastDot + 1, - text: methodName - }); - } }, readString: function(quote) { @@ -362,9 +298,8 @@ Lexer.prototype = { this.tokens.push({ index: start, text: rawString, - string: string, constant: true, - fn: function() { return string; } + value: string }); return; } else { @@ -425,16 +360,12 @@ Parser.prototype = { primary = this.arrayDeclaration(); } else if (this.expect('{')) { primary = this.object(); + } else if (this.peek().identifier) { + primary = this.identifier(); + } else if (this.peek().constant) { + primary = this.constant(); } else { - var token = this.expect(); - primary = token.fn; - if (!primary) { - this.throwError('not a primary expression', token); - } - if (token.constant) { - primary.constant = true; - primary.literal = true; - } + this.throwError('not a primary expression', this.peek()); } var next, context; @@ -468,8 +399,11 @@ Parser.prototype = { }, peek: function(e1, e2, e3, e4) { - if (this.tokens.length > 0) { - var token = this.tokens[0]; + return this.peekAhead(0, e1, e2, e3, e4); + }, + peekAhead: function(i, e1, e2, e3, e4) { + if (this.tokens.length > i) { + var token = this.tokens[i]; var t = token.text; if (t === e1 || t === e2 || t === e3 || t === e4 || (!e1 && !e2 && !e3 && !e4)) { @@ -489,12 +423,19 @@ Parser.prototype = { }, consume: function(e1) { - if (!this.expect(e1)) { + if (this.tokens.length === 0) { + throw $parseMinErr('ueoe', 'Unexpected end of expression: {0}', this.text); + } + + var token = this.expect(e1); + if (!token) { this.throwError('is unexpected, expecting [' + e1 + ']', this.peek()); } + return token; }, - unaryFn: function(fn, right) { + unaryFn: function(op, right) { + var fn = OPERATORS[op]; return extend(function $parseUnaryFn(self, locals) { return fn(self, locals, right); }, { @@ -503,7 +444,8 @@ Parser.prototype = { }); }, - binaryFn: function(left, fn, right, isBranching) { + binaryFn: function(left, op, right, isBranching) { + var fn = OPERATORS[op]; return extend(function $parseBinaryFn(self, locals) { return fn(self, locals, left, right); }, { @@ -512,6 +454,28 @@ Parser.prototype = { }); }, + identifier: function() { + var id = this.consume().text; + + //Continue reading each `.identifier` unless it is a method invocation + while (this.peek('.') && this.peekAhead(1).identifier && !this.peekAhead(2, '(')) { + id += this.consume().text + this.consume().text; + } + + return CONSTANTS[id] || getterFn(id, this.options, this.text); + }, + + constant: function() { + var value = this.consume().value; + + return extend(function $parseConstant() { + return value; + }, { + constant: true, + literal: true + }); + }, + statements: function() { var statements = []; while (true) { @@ -543,8 +507,7 @@ Parser.prototype = { }, filter: function(inputFn) { - var token = this.expect(); - var fn = this.$filter(token.text); + var fn = this.$filter(this.consume().text); var argsFn; var args; @@ -607,7 +570,7 @@ Parser.prototype = { var token; if ((token = this.expect('?'))) { middle = this.assignment(); - if ((token = this.expect(':'))) { + if (this.consume(':')) { var right = this.assignment(); return extend(function $parseTernary(self, locals) { @@ -615,9 +578,6 @@ Parser.prototype = { }, { constant: left.constant && middle.constant && right.constant }); - - } else { - this.throwError('expected :', token); } } @@ -628,7 +588,7 @@ Parser.prototype = { var left = this.logicalAND(); var token; while ((token = this.expect('||'))) { - left = this.binaryFn(left, token.fn, this.logicalAND(), true); + left = this.binaryFn(left, token.text, this.logicalAND(), true); } return left; }, @@ -637,7 +597,7 @@ Parser.prototype = { var left = this.equality(); var token; if ((token = this.expect('&&'))) { - left = this.binaryFn(left, token.fn, this.logicalAND(), true); + left = this.binaryFn(left, token.text, this.logicalAND(), true); } return left; }, @@ -646,7 +606,7 @@ Parser.prototype = { var left = this.relational(); var token; if ((token = this.expect('==','!=','===','!=='))) { - left = this.binaryFn(left, token.fn, this.equality()); + left = this.binaryFn(left, token.text, this.equality()); } return left; }, @@ -655,7 +615,7 @@ Parser.prototype = { var left = this.additive(); var token; if ((token = this.expect('<', '>', '<=', '>='))) { - left = this.binaryFn(left, token.fn, this.relational()); + left = this.binaryFn(left, token.text, this.relational()); } return left; }, @@ -664,7 +624,7 @@ Parser.prototype = { var left = this.multiplicative(); var token; while ((token = this.expect('+','-'))) { - left = this.binaryFn(left, token.fn, this.multiplicative()); + left = this.binaryFn(left, token.text, this.multiplicative()); } return left; }, @@ -673,7 +633,7 @@ Parser.prototype = { var left = this.unary(); var token; while ((token = this.expect('*','/','%'))) { - left = this.binaryFn(left, token.fn, this.unary()); + left = this.binaryFn(left, token.text, this.unary()); } return left; }, @@ -683,9 +643,9 @@ Parser.prototype = { if (this.expect('+')) { return this.primary(); } else if ((token = this.expect('-'))) { - return this.binaryFn(Parser.ZERO, token.fn, this.unary()); + return this.binaryFn(Parser.ZERO, token.text, this.unary()); } else if ((token = this.expect('!'))) { - return this.unaryFn(token.fn, this.unary()); + return this.unaryFn(token.text, this.unary()); } else { return this.primary(); } @@ -693,7 +653,7 @@ Parser.prototype = { fieldAccess: function(object) { var expression = this.text; - var field = this.expect().text; + var field = this.consume().text; var getter = getterFn(field, this.options, expression); return extend(function $parseFieldAccess(scope, locals, self) { @@ -778,8 +738,7 @@ Parser.prototype = { // Support trailing commas per ES5.1. break; } - var elementFn = this.expression(); - elementFns.push(elementFn); + elementFns.push(this.expression()); } while (this.expect(',')); } this.consume(']'); @@ -805,11 +764,16 @@ Parser.prototype = { // Support trailing commas per ES5.1. break; } - var token = this.expect(); - keys.push(token.string || token.text); + var token = this.consume(); + if (token.constant) { + keys.push(token.value); + } else if (token.identifier) { + keys.push(token.text); + } else { + this.throwError("invalid key", token); + } this.consume(':'); - var value = this.expression(); - valueFns.push(value); + valueFns.push(this.expression()); } while (this.expect(',')); } this.consume('}'); diff --git a/test/ng/directive/ngBindSpec.js b/test/ng/directive/ngBindSpec.js index 943adaae9259..c03afa9123bb 100644 --- a/test/ng/directive/ngBindSpec.js +++ b/test/ng/directive/ngBindSpec.js @@ -125,8 +125,8 @@ describe('ngBind*', function() { it('should complain about accidental use of interpolation', inject(function($compile) { expect(function() { $compile('
'); - }).toThrowMinErr('$parse', 'syntax', "Syntax Error: Token 'myHtml' is unexpected, " + - "expecting [:] at column 3 of the expression [{{myHtml}}] starting at [myHtml}}]."); + }).toThrowMinErr('$parse', 'syntax', + "Syntax Error: Token '{' invalid key at column 2 of the expression [{{myHtml}}] starting at [{myHtml}}]"); })); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 00d72bdd4c0c..8ec36e17413e 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -20,11 +20,30 @@ describe('parser', function() { }; }); + it('should only match number chars with isNumber', function() { + expect(Lexer.prototype.isNumber('0')).toBe(true); + expect(Lexer.prototype.isNumber('')).toBeFalsy(); + expect(Lexer.prototype.isNumber(' ')).toBeFalsy(); + expect(Lexer.prototype.isNumber(0)).toBeFalsy(); + expect(Lexer.prototype.isNumber(false)).toBeFalsy(); + expect(Lexer.prototype.isNumber(true)).toBeFalsy(); + expect(Lexer.prototype.isNumber(undefined)).toBeFalsy(); + expect(Lexer.prototype.isNumber(null)).toBeFalsy(); + }); + it('should tokenize a string', function() { var tokens = lex("a.bc[22]+1.3|f:'a\\\'c':\"d\\\"e\""); var i = 0; expect(tokens[i].index).toEqual(0); - expect(tokens[i].text).toEqual('a.bc'); + expect(tokens[i].text).toEqual('a'); + + i++; + expect(tokens[i].index).toEqual(1); + expect(tokens[i].text).toEqual('.'); + + i++; + expect(tokens[i].index).toEqual(2); + expect(tokens[i].text).toEqual('bc'); i++; expect(tokens[i].index).toEqual(4); @@ -32,7 +51,9 @@ describe('parser', function() { i++; expect(tokens[i].index).toEqual(5); - expect(tokens[i].text).toEqual(22); + expect(tokens[i].text).toEqual('22'); + expect(tokens[i].value).toEqual(22); + expect(tokens[i].constant).toEqual(true); i++; expect(tokens[i].index).toEqual(7); @@ -44,7 +65,9 @@ describe('parser', function() { i++; expect(tokens[i].index).toEqual(9); - expect(tokens[i].text).toEqual(1.3); + expect(tokens[i].text).toEqual('1.3'); + expect(tokens[i].value).toEqual(1.3); + expect(tokens[i].constant).toEqual(true); i++; expect(tokens[i].index).toEqual(12); @@ -60,7 +83,7 @@ describe('parser', function() { i++; expect(tokens[i].index).toEqual(15); - expect(tokens[i].string).toEqual("a'c"); + expect(tokens[i].value).toEqual("a'c"); i++; expect(tokens[i].index).toEqual(21); @@ -68,14 +91,15 @@ describe('parser', function() { i++; expect(tokens[i].index).toEqual(22); - expect(tokens[i].string).toEqual('d"e'); + expect(tokens[i].value).toEqual('d"e'); }); - it('should tokenize identifiers with spaces after dots', function() { - var tokens = lex('foo. bar'); - expect(tokens[0].text).toEqual('foo'); - expect(tokens[1].text).toEqual('.'); - expect(tokens[2].text).toEqual('bar'); + it('should tokenize identifiers with spaces around dots the same as without spaces', function() { + function getText(t) { return t.text; } + var spaces = lex('foo. bar . baz').map(getText); + var noSpaces = lex('foo.bar.baz').map(getText); + + expect(spaces).toEqual(noSpaces); }); it('should tokenize undefined', function() { @@ -83,7 +107,6 @@ describe('parser', function() { var i = 0; expect(tokens[i].index).toEqual(0); expect(tokens[i].text).toEqual('undefined'); - expect(undefined).toEqual(tokens[i].fn()); }); it('should tokenize quoted string', function() { @@ -91,23 +114,23 @@ describe('parser', function() { var tokens = lex(str); expect(tokens[1].index).toEqual(1); - expect(tokens[1].string).toEqual("'"); + expect(tokens[1].value).toEqual("'"); expect(tokens[3].index).toEqual(7); - expect(tokens[3].string).toEqual('"'); + expect(tokens[3].value).toEqual('"'); }); it('should tokenize escaped quoted string', function() { var str = '"\\"\\n\\f\\r\\t\\v\\u00A0"'; var tokens = lex(str); - expect(tokens[0].string).toEqual('"\n\f\r\t\v\u00A0'); + expect(tokens[0].value).toEqual('"\n\f\r\t\v\u00A0'); }); it('should tokenize unicode', function() { var tokens = lex('"\\u00A0"'); expect(tokens.length).toEqual(1); - expect(tokens[0].string).toEqual('\u00a0'); + expect(tokens[0].value).toEqual('\u00a0'); }); it('should ignore whitespace', function() { @@ -153,12 +176,12 @@ describe('parser', function() { it('should tokenize method invocation', function() { var tokens = lex("a.b.c (d) - e.f()"); expect(tokens.map(function(t) { return t.text;})). - toEqual(['a.b', '.', 'c', '(', 'd', ')', '-', 'e', '.', 'f', '(', ')']); + toEqual(['a', '.', 'b', '.', 'c', '(', 'd', ')', '-', 'e', '.', 'f', '(', ')']); }); it('should tokenize number', function() { var tokens = lex("0.5"); - expect(tokens[0].text).toEqual(0.5); + expect(tokens[0].value).toEqual(0.5); }); it('should tokenize negative number', inject(function($rootScope) { @@ -171,11 +194,11 @@ describe('parser', function() { it('should tokenize number with exponent', inject(function($rootScope) { var tokens = lex("0.5E-10"); - expect(tokens[0].text).toEqual(0.5E-10); + expect(tokens[0].value).toEqual(0.5E-10); expect($rootScope.$eval("0.5E-10")).toEqual(0.5E-10); tokens = lex("0.5E+10"); - expect(tokens[0].text).toEqual(0.5E+10); + expect(tokens[0].value).toEqual(0.5E+10); })); it('should throws exception for invalid exponent', function() { @@ -190,7 +213,7 @@ describe('parser', function() { it('should tokenize number starting with a dot', function() { var tokens = lex(".5"); - expect(tokens[0].text).toEqual(0.5); + expect(tokens[0].value).toEqual(0.5); }); it('should throw error on invalid unicode', function() { @@ -357,18 +380,26 @@ describe('parser', function() { expect(scope.$eval("a . \nb", scope)).toEqual(4); }); + it('should handle white-spaces around dots in method invocations', function() { + scope.a = {b: function() { return this.c; }, c: 4}; + expect(scope.$eval("a . b ()", scope)).toEqual(4); + expect(scope.$eval("a. b ()", scope)).toEqual(4); + expect(scope.$eval("a .b ()", scope)).toEqual(4); + expect(scope.$eval("a \n . \nb \n ()", scope)).toEqual(4); + }); + it('should throw syntax error exception for identifiers ending with a dot', function() { scope.a = {b: 4}; expect(function() { scope.$eval("a.", scope); - }).toThrowMinErr('$parse', 'syntax', - "Token 'null' is an unexpected token at column 2 of the expression [a.] starting at [.]."); + }).toThrowMinErr('$parse', 'ueoe', + "Unexpected end of expression: a."); expect(function() { scope.$eval("a .", scope); - }).toThrowMinErr('$parse', 'syntax', - "Token 'null' is an unexpected token at column 3 of the expression [a .] starting at [.]."); + }).toThrowMinErr('$parse', 'ueoe', + "Unexpected end of expression: a ."); }); it('should resolve deeply nested paths (important for CSP mode)', function() { @@ -504,13 +535,32 @@ describe('parser', function() { }); it('should evaluate object', function() { - expect(toJson(scope.$eval("{}"))).toEqual("{}"); - expect(toJson(scope.$eval("{a:'b'}"))).toEqual('{"a":"b"}'); - expect(toJson(scope.$eval("{'a':'b'}"))).toEqual('{"a":"b"}'); - expect(toJson(scope.$eval("{\"a\":'b'}"))).toEqual('{"a":"b"}'); - expect(toJson(scope.$eval("{a:'b',}"))).toEqual('{"a":"b"}'); - expect(toJson(scope.$eval("{'a':'b',}"))).toEqual('{"a":"b"}'); - expect(toJson(scope.$eval("{\"a\":'b',}"))).toEqual('{"a":"b"}'); + expect(scope.$eval("{}")).toEqual({}); + expect(scope.$eval("{a:'b'}")).toEqual({a:"b"}); + expect(scope.$eval("{'a':'b'}")).toEqual({a:"b"}); + expect(scope.$eval("{\"a\":'b'}")).toEqual({a:"b"}); + expect(scope.$eval("{a:'b',}")).toEqual({a:"b"}); + expect(scope.$eval("{'a':'b',}")).toEqual({a:"b"}); + expect(scope.$eval("{\"a\":'b',}")).toEqual({a:"b"}); + expect(scope.$eval("{'0':1}")).toEqual({0:1}); + expect(scope.$eval("{0:1}")).toEqual({0:1}); + expect(scope.$eval("{1:1}")).toEqual({1:1}); + expect(scope.$eval("{null:1}")).toEqual({null:1}); + expect(scope.$eval("{'null':1}")).toEqual({null:1}); + expect(scope.$eval("{false:1}")).toEqual({false:1}); + expect(scope.$eval("{'false':1}")).toEqual({false:1}); + expect(scope.$eval("{'':1,}")).toEqual({"":1}); + }); + + it('should throw syntax error exception for non constant/identifier JSON keys', function() { + expect(function() { scope.$eval("{[:0}"); }).toThrowMinErr("$parse", "syntax", + "Syntax Error: Token '[' invalid key at column 2 of the expression [{[:0}] starting at [[:0}]"); + expect(function() { scope.$eval("{{:0}"); }).toThrowMinErr("$parse", "syntax", + "Syntax Error: Token '{' invalid key at column 2 of the expression [{{:0}] starting at [{:0}]"); + expect(function() { scope.$eval("{?:0}"); }).toThrowMinErr("$parse", "syntax", + "Syntax Error: Token '?' invalid key at column 2 of the expression [{?:0}] starting at [?:0}]"); + expect(function() { scope.$eval("{):0}"); }).toThrowMinErr("$parse", "syntax", + "Syntax Error: Token ')' invalid key at column 2 of the expression [{):0}] starting at [):0}]"); }); it('should evaluate object access', function() { @@ -518,8 +568,8 @@ describe('parser', function() { }); it('should evaluate JSON', function() { - expect(toJson(scope.$eval("[{}]"))).toEqual("[{}]"); - expect(toJson(scope.$eval("[{a:[]}, {b:1}]"))).toEqual('[{"a":[]},{"b":1}]'); + expect(scope.$eval("[{}]")).toEqual([{}]); + expect(scope.$eval("[{a:[]}, {b:1}]")).toEqual([{a:[]}, {b:1}]); }); it('should evaluate multiple statements', function() {