From 72597c52e5a47bc2743451eb7416259ddeea6f7d Mon Sep 17 00:00:00 2001 From: harttle Date: Mon, 14 Aug 2017 22:07:08 +0800 Subject: [PATCH] cover all --- .travis.yml | 2 +- index.js | 11 +-- package.json | 2 + src/scope.js | 102 +++++++++++++------------- src/tag.js | 12 +-- src/util/assert.js | 3 - src/util/underscore.js | 14 +++- tags/tablerow.js | 158 +++++++++++++++++++--------------------- test/liquid.js | 6 ++ test/scope.js | 23 +++++- test/tags/for.js | 84 ++++++++++++--------- test/tags/tablerow.js | 30 +++++--- test/util/assert.js | 18 +++++ test/util/underscore.js | 8 ++ 14 files changed, 268 insertions(+), 205 deletions(-) create mode 100644 test/util/assert.js diff --git a/.travis.yml b/.travis.yml index 63c100fde..6d0de7118 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,4 +4,4 @@ node_js: before_script: - npm install -g mocha after_script: - - NODE_ENV=test ./node_modules/.bin/istanbul cover --report lcovonly ./node_modules/mocha/bin/_mocha -- -R spec --recursive && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage + - npm run lcov && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage diff --git a/index.js b/index.js index efb65cdd0..78768a01b 100644 --- a/index.js +++ b/index.js @@ -43,15 +43,10 @@ var _engine = { return this.renderer.renderTemplates(tpl, scope) }, parseAndRender: function (html, ctx, opts) { + console.log('parse and render') return Promise.resolve() .then(() => this.parse(html)) .then(tpl => this.render(tpl, ctx, opts)) - .catch(e => { - if (e instanceof Errors.RenderBreakError) { - return e.html - } - throw e - }) }, renderFile: function (filepath, ctx, opts) { opts = _.assign({}, opts) @@ -93,8 +88,8 @@ var _engine = { return Promise.resolve(tpl) } return readFileAsync(filepath) - .then(str => this.parse(str)) - .then(tpl => (this.cache[filepath] = tpl)) + .then(str => this.parse(str)) + .then(tpl => (this.cache[filepath] = tpl)) } else { return readFileAsync(filepath).then(str => this.parse(str, filepath)) } diff --git a/package.json b/package.json index 3bd438af1..72bce2033 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,8 @@ "scripts": { "lint": "eslint src/ test/", "test": "npm run lint && mocha --recursive", + "coverage": "NODE_ENV=test istanbul cover --report html ./node_modules/mocha/bin/_mocha -- -R spec --recursive", + "lcov": "NODE_ENV=test istanbul cover --report lcovonly ./node_modules/mocha/bin/_mocha -- -R spec --recursive", "dist": "make dist", "preversion": "npm test", "version": "npm run dist && git add -A dist", diff --git a/src/scope.js b/src/scope.js index 25bc50c41..a8ef4b8f5 100644 --- a/src/scope.js +++ b/src/scope.js @@ -1,7 +1,6 @@ const _ = require('./util/underscore.js') const lexical = require('./lexical.js') const assert = require('./util/assert.js') -const toStr = Object.prototype.toString var Scope = { getAll: function () { @@ -12,32 +11,16 @@ var Scope = { return ctx }, get: function (str) { - for (var i = this.scopes.length - 1; i >= 0; i--) { - try { - return this.getPropertyByPath(this.scopes[i], str) - } catch (e) { - if (/undefined variable/.test(e.message)) { - continue - } - if (/Cannot read property/.test(e.message)) { - if (this.opts.strict_variables) { - e.message += ': ' + str - throw e - } else { - continue - } - } else { - e.message += ': ' + str - throw e - } + try { + return this.getPropertyByPath(this.scopes, str) + } catch (e) { + if (!/undefined variable/.test(e.message) || this.opts.strict_variables) { + throw e } } - if (this.opts.strict_variables) { - throw new TypeError('undefined variable: ' + str) - } }, set: function (k, v) { - this.setPropertyByPath(this.scopes[this.scopes.length - 1], k, v) + setPropertyByPath(this.scopes[this.scopes.length - 1], k, v) return this }, push: function (ctx) { @@ -54,39 +37,23 @@ var Scope = { shift: function () { return this.scopes.shift() }, - setPropertyByPath: function (obj, path, val) { - if (_.isString(path)) { - var paths = path.replace(/\[/g, '.').replace(/\]/g, '').split('.') - for (var i = 0; i < paths.length; i++) { - var key = paths[i] - if (i === paths.length - 1) { - return (obj[key] = val) - } - if (undefined === obj[key]) obj[key] = {} - // case for readonly objects - obj = obj[key] || {} - } - } - }, - getPropertyByPath: function (obj, path) { + getPropertyByPath: function (scopes, path) { var paths = this.propertyAccessSeq(path + '') - var varName = paths.shift() - if (!obj.hasOwnProperty(varName)) { - throw new TypeError('undefined variable') + if (!paths.length) { + throw new TypeError('undefined variable: ' + path) } - var variable = obj[varName] - var lastName = paths.pop() - paths.forEach(p => (variable = variable[p])) - if (undefined !== lastName) { - if (lastName === 'size' && - (toStr.call(variable) === '[object Array]' || - toStr.call(variable) === '[object String]')) { - return variable.length - } - variable = variable[lastName] - } - return variable + var key = paths.shift() + var value = getValueFromScopes(key, scopes) + return paths.reduce( + (value, key) => { + if (_.isNil(value)) { + throw new TypeError('undefined variable: ' + key) + } + return getValueFromParent(key, value) + }, + value + ) }, /* @@ -144,6 +111,35 @@ var Scope = { } } +function setPropertyByPath (obj, path, val) { + var paths = (path + '').replace(/\[/g, '.').replace(/\]/g, '').split('.') + for (var i = 0; i < paths.length; i++) { + var key = paths[i] + if (i === paths.length - 1) { + return (obj[key] = val) + } + if (undefined === obj[key]) obj[key] = {} + // case for readonly objects + obj = obj[key] || {} + } +} + +function getValueFromParent (key, value) { + return (key === 'size' && (_.isArray(value) || _.isString(value))) + ? value.length + : value[key] +} + +function getValueFromScopes (key, scopes) { + for (var i = scopes.length - 1; i > -1; i--) { + var scope = scopes[i] + if (scope.hasOwnProperty(key)) { + return scope[key] + } + } + throw new TypeError('undefined variable: ' + key) +} + function matchRightBracket (str, begin) { var stack = 1 // count of '[' - count of ']' for (var i = begin; i < str.length; i++) { diff --git a/src/tag.js b/src/tag.js index a43eec3e3..ecbf591fb 100644 --- a/src/tag.js +++ b/src/tag.js @@ -1,5 +1,4 @@ const lexical = require('./lexical.js') -const _ = require('./util/underscore.js') const Promise = require('any-promise') const Syntax = require('./syntax.js') const assert = require('./util/assert.js') @@ -28,14 +27,9 @@ module.exports = function () { } return Promise.resolve() .then(() => typeof impl.render === 'function' - ? impl.render(scope, obj) : '') - .catch(function (e) { - if (_.isError(e)) { - throw e - } - var msg = `Please reject with an Error in ${impl.render}, got ${e}` - throw new Error(msg) - }) + ? impl.render(scope, obj) + : '' + ) }, parse: function (token, tokens) { this.type = 'tag' diff --git a/src/util/assert.js b/src/util/assert.js index e101d0ad7..c673a9ca1 100644 --- a/src/util/assert.js +++ b/src/util/assert.js @@ -2,9 +2,6 @@ const AssertionError = require('./error.js').AssertionError function assert (predicate, message) { if (!predicate) { - if (message instanceof Error) { - throw message - } message = message || `expect ${predicate} to be true` throw new AssertionError(message) } diff --git a/src/util/underscore.js b/src/util/underscore.js index 3870255c6..0322f6f47 100644 --- a/src/util/underscore.js +++ b/src/util/underscore.js @@ -1,3 +1,5 @@ +const toStr = Object.prototype.toString + /* * Checks if value is classified as a String primitive or object. * @param {any} value The value to check. @@ -7,6 +9,15 @@ function isString (value) { return value instanceof String || typeof value === 'string' } +function isNil (value) { + return value === null || value === undefined +} + +function isArray (value) { + // be compatible with IE 8 + return toStr.call(value) === '[object Array]' +} + function isError (value) { var signature = Object.prototype.toString.call(value) // [object XXXError] @@ -53,7 +64,6 @@ function assign (object) { } function _assignBinary (dst, src) { - if (!dst) return dst forOwn(src, function (v, k) { dst[k] = v }) @@ -115,6 +125,8 @@ function range (start, stop, step) { exports.isString = isString exports.isObject = isObject +exports.isArray = isArray +exports.isNil = isNil exports.isError = isError exports.range = range diff --git a/tags/tablerow.js b/tags/tablerow.js index bd5407eff..b45dac04b 100644 --- a/tags/tablerow.js +++ b/tags/tablerow.js @@ -1,95 +1,89 @@ -const Liquid = require('..'); -const Promise = require('any-promise'); -const lexical = Liquid.lexical; -const assert = require('../src/util/assert.js'); +const Liquid = require('..') +const Promise = require('any-promise') +const lexical = Liquid.lexical +const assert = require('../src/util/assert.js') const re = new RegExp(`^(${lexical.identifier.source})\\s+in\\s+` + - `(${lexical.value.source})` + - `(?:\\s+${lexical.hash.source})*$`); + `(${lexical.value.source})` + + `(?:\\s+${lexical.hash.source})*$`) -module.exports = function(liquid) { - liquid.registerTag('tablerow', { +module.exports = function (liquid) { + liquid.registerTag('tablerow', { - parse: function(tagToken, remainTokens) { - var match = re.exec(tagToken.args); - assert(match, `illegal tag: ${tagToken.raw}`); - this.variable = match[1]; - this.collection = match[2]; + parse: function (tagToken, remainTokens) { + var match = re.exec(tagToken.args) + assert(match, `illegal tag: ${tagToken.raw}`) - this.templates = []; + this.variable = match[1] + this.collection = match[2] + this.templates = [] - var p, stream = liquid.parser.parseStream(remainTokens) - .on('start', x => p = this.templates) - .on('tag:endtablerow', token => stream.stop()) - .on('template', tpl => p.push(tpl)) - .on('end', x => { - throw new Error(`tag ${tagToken.raw} not closed`); - }); + var p + var stream = liquid.parser.parseStream(remainTokens) + .on('start', () => (p = this.templates)) + .on('tag:endtablerow', token => stream.stop()) + .on('template', tpl => p.push(tpl)) + .on('end', () => { + throw new Error(`tag ${tagToken.raw} not closed`) + }) - stream.start(); - }, + stream.start() + }, - render: function(scope, hash) { - var collection = Liquid.evalExp(this.collection, scope) || []; + render: function (scope, hash) { + var collection = Liquid.evalExp(this.collection, scope) || [] - var html = ''; - var offset = hash.offset || 0; - var limit = (hash.limit === undefined) ? collection.length : hash.limit; + var html = '
' + var offset = hash.offset || 0 + var limit = (hash.limit === undefined) ? collection.length : hash.limit - var cols = hash.cols, row, col; - if (!cols) throw new Error(`illegal cols: ${cols}`); + var cols = hash.cols + var row + var col + if (!cols) throw new Error(`illegal cols: ${cols}`) - // build array of arguments to pass to sequential promises... - collection = collection.slice(offset, offset + limit); - var contexts = []; - collection.some((item, i) => { - var ctx = {}; - ctx[this.variable] = item; - // We are just putting together an array of the arguments we will be passing to our sequential promises - contexts.push(ctx); - }); + // build array of arguments to pass to sequential promises... + collection = collection.slice(offset, offset + limit) + var contexts = [] + collection.some((item, i) => { + var ctx = {} + ctx[this.variable] = item + contexts.push(ctx) + }) - // This executes an array of promises sequentially for every argument in the contexts array - http://webcache.googleusercontent.com/search?q=cache:rNbMUn9TPtkJ:joost.vunderink.net/blog/2014/12/15/processing-an-array-of-promises-sequentially-in-node-js/+&cd=5&hl=en&ct=clnk&gl=us - // It's fundamentally equivalent to the following... - // emptyPromise.then(renderTemplates(args0).then(renderTemplates(args1).then(renderTemplates(args2)... - var lastPromise = contexts.reduce((promise, context, currentIndex) => { - return promise.then((partial) => { - row = Math.floor(currentIndex / cols) + 1; - col = (currentIndex % cols) + 1; - if(col === 1) { - if(row !== 1){ - html += ''; - } - html += ``; - } + var lastPromise = contexts.reduce((promise, context, currentIndex) => promise + .then((partial) => { + row = Math.floor(currentIndex / cols) + 1 + col = (currentIndex % cols) + 1 + if (col === 1) { + if (row !== 1) { + html += '' + } + html += `` + } - //ctx[this.variable] = context; + // ctx[this.variable] = context; + html += `' + return html + }), Promise.resolve('')) - return html += `'; - }); - }, Promise.resolve('')); // start the reduce chain with a resolved Promise. After first run, the "promise" argument - // in our reduce callback will be the returned promise from our "then" above. In this - // case, the promise returned from liquid.renderer.renderTemplates. - - return lastPromise - .then(() => { - if(row > 0) { - html += ''; - } - html += '
` + return html + }) + .then((partial) => { + scope.push(context) + return liquid.renderer.renderTemplates(this.templates, scope) + }) + .then((partial) => { + scope.pop(context) + html += partial + html += '`; - }) - .then((partial) => { - scope.push(context); - return liquid.renderer.renderTemplates(this.templates, scope) - }) - .then((partial) => { - scope.pop(context); - html += partial; - return html += '
'; - return html; - }) - .catch((error) => { - throw error; - }); - } - }); -}; + return lastPromise + .then(() => { + if (row > 0) { + html += '' + } + html += '' + return html + }) + } + }) +} diff --git a/test/liquid.js b/test/liquid.js index 29dc5e0d6..7bb711127 100644 --- a/test/liquid.js +++ b/test/liquid.js @@ -34,6 +34,12 @@ describe('liquid', function () { afterEach(function () { mock.restore() }) + describe('Liquid', function () { + it('should ignore invalid root option', function () { + var liquid = Liquid({ root: /regex/ }) + expect(liquid.options.root).to.deep.equal([]) + }) + }) describe('{{output}}', function () { it('should output object', function () { return expect(engine.parseAndRender('{{obj}}', ctx)).to.eventually.equal('{"foo":"bar"}') diff --git a/test/scope.js b/test/scope.js index 97f5d9c5a..81f54f006 100644 --- a/test/scope.js +++ b/test/scope.js @@ -98,7 +98,13 @@ describe('scope', function () { expect(scope.get('bar[foo]')).to.equal('coo') }) - it('should support nested case', function () { + it('should return undefined when not exist', function () { + expect(scope.get('foo.foo.foo')).to.be.undefined + }) + }) + + describe('#set', function () { + it('should set nested value', function () { scope.set('posts', { 'first': { 'name': 'A Nice Day' @@ -109,8 +115,12 @@ describe('scope', function () { }) expect(scope.get('posts[category.diary[0]].name'), 'A Nice Day') }) - }) + it('should create parents if needed', function () { + scope.set('foo.bar.coo', 'COO') + expect(scope.get('foo.bar.coo'), 'COO') + }) + }) describe('strict_variables', function () { var scope beforeEach(function () { @@ -118,12 +128,19 @@ describe('scope', function () { strict_variables: true }) }) - it('should throw undefined in strict mode', function () { + it('should throw when variable not defined', function () { function fn () { scope.get('notdefined') } expect(fn).to.throw(/undefined variable: notdefined/) }) + it('should throw when deep variable not exist', function () { + scope.set('foo', 'bar') + function fn () { + scope.get('foo.bar.not.defined') + } + expect(fn).to.throw(/undefined variable: not/) + }) it('should find variable in parent scope', function () { scope.set('foo', 'foo') scope.push({ diff --git a/test/tags/for.js b/test/tags/for.js index 6416dde40..32d4911a4 100644 --- a/test/tags/for.js +++ b/test/tags/for.js @@ -7,6 +7,9 @@ describe('tags/for', function () { var liquid, ctx before(function () { liquid = Liquid() + liquid.registerTag('throwingTag', { + render: function () { throw new Error('intended render error') } + }) ctx = { one: 1, // eslint-disable-next-line @@ -30,10 +33,18 @@ describe('tags/for', function () { .to.eventually.equal('foo-coo-') }) - it('should throw when for not closed', function () { - var src = '{%for c in alpha%}{{c}}' - return expect(liquid.parseAndRender(src, ctx)) - .to.be.rejectedWith(/tag .* not closed/) + describe('illegal', function () { + it('should reject when for not closed', function () { + var src = '{%for c in alpha%}{{c}}' + return expect(liquid.parseAndRender(src, ctx)) + .to.be.rejectedWith(/tag .* not closed/) + }) + + it('should reject when inner templates rejected', function () { + var src = '{%for c in alpha%}{%throwingTag%}{%endfor%}' + return expect(liquid.parseAndRender(src, ctx)) + .to.be.rejectedWith(/intended render error/) + }) }) describe('else', function () { @@ -43,6 +54,12 @@ describe('tags/for', function () { .to.eventually.equal('b') }) + it('should treat non-empty string as one single element', function () { + var src = '{%for c in "abc"%}x{{c}}{%else%}y{%endfor%}' + return expect(liquid.parseAndRender(src, ctx)) + .to.eventually.equal('xabc') + }) + it('should goto else for empty string', function () { var src = '{%for c in ""%}a{%else%}b{%endfor%}' return expect(liquid.parseAndRender(src, ctx)) @@ -92,42 +109,43 @@ describe('tags/for', function () { }) it('should support for with break', function () { var src = '{% for i in (one..5) %}' + - '{% if i == 4 %}{% break %}{% endif %}' + - '{{ i }}' + - '{% endfor %}' - // return liquid.parseAndRender(src, ctx).catch(e => { - // console.log(e.stack); - // }); + '{% if i == 4 %}{% break %}{% endif %}' + + '{{ i }}' + + '{% endfor %}' return expect(liquid.parseAndRender(src, ctx)) .to.eventually.equal('123') }) - it('should support for with limit', function () { - var src = '{% for i in (1..5) limit:2 %}{{ i }}{% endfor %}' - return expect(liquid.parseAndRender(src, ctx)) - .to.eventually.equal('12') - }) - it('should support for with limit and offset', function () { - var src = '{% for i in (1..10) limit:2 offset:5%}{{ i }}{% endfor %}' - return expect(liquid.parseAndRender(src, ctx)) - .to.eventually.equal('67') + describe('limit', function () { + it('should support for with limit', function () { + var src = '{% for i in (1..5) limit:2 %}{{ i }}{% endfor %}' + return expect(liquid.parseAndRender(src, ctx)) + .to.eventually.equal('12') + }) + it('should support for with limit and offset', function () { + var src = '{% for i in (1..10) limit:2 offset:5%}{{ i }}{% endfor %}' + return expect(liquid.parseAndRender(src, ctx)) + .to.eventually.equal('67') + }) }) - it('should support for reversed in the last position', function () { - var src = '{% for i in (1..5) limit:2 reversed %}{{ i }}{% endfor %}' - return expect(liquid.parseAndRender(src, ctx)) - .to.eventually.equal('21') - }) + describe('reverse', function () { + it('should support for reversed in the last position', function () { + var src = '{% for i in (1..5) limit:2 reversed %}{{ i }}{% endfor %}' + return expect(liquid.parseAndRender(src, ctx)) + .to.eventually.equal('21') + }) - it('should support for reversed in the first position', function () { - var src = '{% for i in (1..5) reversed limit:2 %}{{ i }}{% endfor %}' - return expect(liquid.parseAndRender(src, ctx)) - .to.eventually.equal('21') - }) + it('should support for reversed in the first position', function () { + var src = '{% for i in (1..5) reversed limit:2 %}{{ i }}{% endfor %}' + return expect(liquid.parseAndRender(src, ctx)) + .to.eventually.equal('21') + }) - it('should support for reversed in the middle position', function () { - var src = '{% for i in (1..5) offset:2 reversed limit:4 %}{{ i }}{% endfor %}' - return expect(liquid.parseAndRender(src, ctx)) - .to.eventually.equal('543') + it('should support for reversed in the middle position', function () { + var src = '{% for i in (1..5) offset:2 reversed limit:4 %}{{ i }}{% endfor %}' + return expect(liquid.parseAndRender(src, ctx)) + .to.eventually.equal('543') + }) }) }) diff --git a/test/tags/tablerow.js b/test/tags/tablerow.js index 66a660fce..61de6aec7 100644 --- a/test/tags/tablerow.js +++ b/test/tags/tablerow.js @@ -12,9 +12,9 @@ describe('tags/tablerow', function () { alpha: ['a', 'b', 'c'] } var dst = '' + - '' + - '' + - '
ab
c
' + 'ab' + + 'c' + + '' return expect(liquid.parseAndRender(src, ctx)).to.eventually.equal(dst) }) @@ -24,6 +24,12 @@ describe('tags/tablerow', function () { return expect(liquid.parseAndRender(src)).to.eventually.equal(dst) }) + it('should support empty array', function () { + var src = '{% tablerow i in alpha.z cols:2 %}{{ i }}{% endtablerow %}' + var dst = '
' + return expect(liquid.parseAndRender(src)).to.eventually.equal(dst) + }) + it('should throw when tablerow not closed', function () { var src = '{% tablerow i in (1..0) cols:2 %}{{ i }}' return expect(liquid.parseAndRender(src)) @@ -33,10 +39,10 @@ describe('tags/tablerow', function () { it('should support tablerow with range', function () { var src = '{% tablerow i in (1..5) cols:2 %}{{ i }}{% endtablerow %}' var dst = '' + - '' + - '' + - '' + - '
12
34
5
' + '12' + + '34' + + '5' + + '' return expect(liquid.parseAndRender(src)).to.eventually.equal(dst) }) @@ -54,17 +60,17 @@ describe('tags/tablerow', function () { it('should support tablerow with limit', function () { var src = '{% tablerow i in (1..5) cols:2 limit:3 %}{{ i }}{% endtablerow %}' var dst = '' + - '' + - '' + - '
12
3
' + '12' + + '3' + + '' return expect(liquid.parseAndRender(src)).to.eventually.equal(dst) }) it('should support tablerow with offset', function () { var src = '{% tablerow i in (1..5) cols:2 offset:3 %}{{ i }}{% endtablerow %}' var dst = '' + - '' + - '
45
' + '45' + + '' return expect(liquid.parseAndRender(src)).to.eventually.equal(dst) }) }) diff --git a/test/util/assert.js b/test/util/assert.js new file mode 100644 index 000000000..fe0c6acc5 --- /dev/null +++ b/test/util/assert.js @@ -0,0 +1,18 @@ +const chai = require('chai') +const expect = chai.expect +const assert = require('../../src/util/assert.js') + +describe('assert', function () { + it('should not throw if predicate is truthy', function () { + var fn = () => assert('foo', 'bar') + expect(fn).to.not.throw() + }) + it('should not throw if predicate is truthy', function () { + var fn = () => assert('', 'bar') + expect(fn).to.throw(/bar/) + }) + it('should populate default message', function () { + var fn = () => assert(false) + expect(fn).to.throw(/expect false to be true/) + }) +}) diff --git a/test/util/underscore.js b/test/util/underscore.js index 06d2600b6..0c9c731a6 100644 --- a/test/util/underscore.js +++ b/test/util/underscore.js @@ -72,6 +72,14 @@ describe('util/underscore', function () { expect(log).to.have.been.calledWith('[foo]', 'bar') }) }) + describe('.range()', function () { + it('should return a range of integers', function () { + expect(_.range(3, 5)).to.deep.equal([3, 4]) + }) + it('should treat start as 0 if omitted', function () { + expect(_.range(3)).to.deep.equal([0, 1, 2]) + }) + }) describe('.assign()', function () { it('should handle null dst', function () { expect(_.assign(null, {