diff --git a/docs/source/tutorials/differences.md b/docs/source/tutorials/differences.md index fef364d2a..b8b5d0ab5 100644 --- a/docs/source/tutorials/differences.md +++ b/docs/source/tutorials/differences.md @@ -34,7 +34,7 @@ Though we're trying to be compatible with the Ruby version, there are still some * LiquidJS-defined filters: [json][json], group_by, group_by_exp, where_exp, jsonify, inspect, etc. * Tags/filters that don't depend on Shopify platform are borrowed from [Shopify][shopify-tags]. * Tags/filters that don't depend on Jekyll framework are borrowed from [Jekyll][jekyll-filters]. -* Some tags/filters behave differently: [date][date] filter. +* Some tags/filters behave differently: [date][date] filter, malformed tags (like duplicated `else`, extra args for `endif`) throw errors in LiquidJS. [date]: https://liquidjs.com/filters/date.html [layout]: ../tags/layout.html diff --git a/docs/source/zh-cn/tutorials/differences.md b/docs/source/zh-cn/tutorials/differences.md index 9c34dafa9..f89b44278 100644 --- a/docs/source/zh-cn/tutorials/differences.md +++ b/docs/source/zh-cn/tutorials/differences.md @@ -34,7 +34,7 @@ LiquidJS 一直很重视兼容于 Ruby 版本的 Liquid。Liquid 模板语言最 * LiquidJS 自己定义的过滤器:[json][json]。 * 从 [Shopify][shopify-tags] 借来的不依赖 Shopify 平台的标签/过滤器。 * 从 [Jekyll][jekyll-filters] 借来的不依赖 Jekyll 框架的标签/过滤器。 -* 有些过滤器和标签表现不同:比如 [date][date]。 +* 有些过滤器和标签表现不同:比如 [date][date],非法的标签(比如重复的 `else`,`endif` 的多余参数)在 LiquidJS 中会抛出异常。 [layout]: ../tags/layout.html [render]: ../tags/render.html diff --git a/src/tags/for.ts b/src/tags/for.ts index 4d1f2f3bd..53f76e748 100644 --- a/src/tags/for.ts +++ b/src/tags/for.ts @@ -1,10 +1,10 @@ import { Hash, ValueToken, Liquid, Tag, evalToken, Emitter, TagToken, TopLevelToken, Context, Template, ParseStream } from '..' -import { toEnumerable } from '../util' +import { assertEmpty, toEnumerable } from '../util' import { ForloopDrop } from '../drop/forloop-drop' const MODIFIERS = ['offset', 'limit', 'reversed'] -type valueof = T[keyof T] +type valueOf = T[keyof T] export default class extends Tag { variable: string @@ -31,12 +31,10 @@ export default class extends Tag { let p const stream: ParseStream = this.liquid.parser.parseStream(remainTokens) .on('start', () => (p = this.templates)) - .on('tag:else', () => (p = this.elseTemplates)) - .on('tag:endfor', () => stream.stop()) + .on('tag:else', tag => { assertEmpty(tag.args); p = this.elseTemplates }) + .on('tag:endfor', tag => { assertEmpty(tag.args); stream.stop() }) .on('template', (tpl: Template) => p.push(tpl)) - .on('end', () => { - throw new Error(`tag ${token.getText()} not closed`) - }) + .on('end', () => { throw new Error(`tag ${token.getText()} not closed`) }) stream.start() } @@ -58,7 +56,7 @@ export default class extends Tag { ? Object.keys(hash).filter(x => MODIFIERS.includes(x)) : MODIFIERS.filter(x => hash[x] !== undefined) - collection = modifiers.reduce((collection, modifier: valueof) => { + collection = modifiers.reduce((collection, modifier: valueOf) => { if (modifier === 'offset') return offset(collection, hash['offset']) if (modifier === 'limit') return limit(collection, hash['limit']) return reversed(collection) diff --git a/src/tags/if.ts b/src/tags/if.ts index f563c8060..45114d0f7 100644 --- a/src/tags/if.ts +++ b/src/tags/if.ts @@ -1,38 +1,32 @@ import { Liquid, Tag, Value, Emitter, isTruthy, TagToken, TopLevelToken, Context, Template } from '..' +import { assert, assertEmpty } from '../util' export default class extends Tag { branches: { value: Value, templates: Template[] }[] = [] - elseTemplates: Template[] = [] + elseTemplates: Template[] | undefined constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) { super(tagToken, remainTokens, liquid) let p: Template[] = [] - let elseCount = 0 liquid.parser.parseStream(remainTokens) .on('start', () => this.branches.push({ value: new Value(tagToken.args, this.liquid), templates: (p = []) })) .on('tag:elsif', (token: TagToken) => { - if (elseCount > 0) { - p = [] - return - } + assert(!this.elseTemplates, 'unexpected elsif after else') this.branches.push({ value: new Value(token.args, this.liquid), templates: (p = []) }) }) - .on('tag:else', () => { - elseCount++ - p = this.elseTemplates - }) - .on('tag:endif', function () { this.stop() }) - .on('template', (tpl: Template) => { - if (p !== this.elseTemplates || elseCount === 1) { - p.push(tpl) - } - }) + .on('tag:else', tag => { + assertEmpty(tag.args) + assert(!this.elseTemplates, 'duplicated else') + p = this.elseTemplates = [] + }) + .on('tag:endif', function (tag) { assertEmpty(tag.args); this.stop() }) + .on('template', (tpl: Template) => p.push(tpl)) .on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) }) .start() } @@ -47,6 +41,6 @@ export default class extends Tag { return } } - yield r.renderTemplates(this.elseTemplates, ctx, emitter) + yield r.renderTemplates(this.elseTemplates || [], ctx, emitter) } } diff --git a/src/util/assert.ts b/src/util/assert.ts index 67dcd7eca..913779e0f 100644 --- a/src/util/assert.ts +++ b/src/util/assert.ts @@ -8,3 +8,7 @@ export function assert (predicate: T | null | undefined, message?: string | throw new AssertionError(msg) } } + +export function assertEmpty (predicate: T | null | undefined, message = `unexpected ${JSON.stringify(predicate)}`) { + assert(!predicate, message) +} diff --git a/test/e2e/issues.spec.ts b/test/e2e/issues.spec.ts index 60437823b..4eb36b352 100644 --- a/test/e2e/issues.spec.ts +++ b/test/e2e/issues.spec.ts @@ -471,19 +471,11 @@ describe('Issues', function () { }) it('#670 Should not render anything after an else branch', () => { const engine = new Liquid() - const result = engine.parseAndRenderSync('{% assign value = "this" %}' + - '{% if false %}don\'t show' + - '{% else %}show {{ value }}' + - '{% else %}don\'t show{% endif %}', {}) - expect(result).toEqual('show this') + expect(() => engine.parseAndRenderSync('{% assign value = "this" %}{% if false %}{% else %}{% else %}{% endif %}')).toThrow('duplicated else') }) it('#672 Should not render an elseif after an else branch', () => { const engine = new Liquid() - const result = engine.parseAndRenderSync('{% if false %}don\'t show' + - '{% else %}show' + - '{% elsif true %}don\'t show' + - '{% endif %}', {}) - expect(result).toEqual('show') + expect(() => engine.parseAndRenderSync('{% if false %}{% else %}{% elsif true %}{% endif %}')).toThrow('unexpected elsif after else') }) it('#675 10.10.1 Operator: contains regression', () => { const engine = new Liquid() diff --git a/test/integration/tags/for.spec.ts b/test/integration/tags/for.spec.ts index 0cfcd31a6..fe16deb37 100644 --- a/test/integration/tags/for.spec.ts +++ b/test/integration/tags/for.spec.ts @@ -78,10 +78,10 @@ describe('tags/for', function () { .rejects.toThrow('illegal tag: {%for c alpha%}, line:1, col:1') }) - it('should reject when inner templates rejected', function () { - const src = '{%for c in alpha%}{%throwingTag%}{%endfor%}' + it('should throw for additional args', function () { + const src = "{% for f in foo %} foo {% else foo = 'blah' %} {% endfor %}" return expect(liquid.parseAndRender(src, scope)) - .rejects.toThrow(/intended render error/) + .rejects.toThrow(`unexpected "foo = 'blah'", line:1, col:1`) }) }) diff --git a/test/integration/tags/if.spec.ts b/test/integration/tags/if.spec.ts index 5dd25850c..2573861e0 100644 --- a/test/integration/tags/if.spec.ts +++ b/test/integration/tags/if.spec.ts @@ -26,6 +26,12 @@ describe('tags/if', function () { return expect(html).toBe('') }) + it('should throw for additional args', function () { + const src = "{% if foo %} foo {% else foo = 'blah' %} {% endif %}" + return expect(liquid.parseAndRender(src, scope)) + .rejects.toThrow(`unexpected "foo = 'blah'", line:1, col:1`) + }) + describe('single value as condition', function () { it('should support boolean', async function () { const src = '{% if false %}1{%elsif true%}2{%else%}3{%endif%}' @@ -155,12 +161,12 @@ describe('tags/if', function () { const html = await liquid.parseAndRender(src, scope) return expect(html).toBe('no') }) - it('should not render anything after an else branch even when first else branch is empty', () => { - const engine = new Liquid() - const result = engine.parseAndRenderSync('{% if false %}don\'t show' + - '{% else %}' + - '{% else %}don\'t show' + - '%{% endif %}', {}) - expect(result).toEqual('') + it('should throw for duplicated else', () => { + expect(() => liquid.parseAndRenderSync('{% if false %}{% else %}{% else %}{% endif %}')) + .toThrow(`duplicated else`) + }) + it('should throw for unexpected elsif', () => { + expect(() => liquid.parseAndRenderSync('{% if false %}{% else %}{% elsif true %}{% endif %}')) + .toThrow(`unexpected elsif after else`) }) })