fix: filters break when argument contains [()|, fixes #89

src/template/filter/filter.ts

@@ -12,29 +12,13 @@ export default class Filter {
   args: string[]
   private static impls: {[key: string]: FilterImpl} = {}
 
-  constructor (str: string, strictFilters: boolean = false) {
-    const match = lexical.filterLine.exec(str) as string[]
-    assert(match, 'illegal filter: ' + str)
-
-    const name = match[1]
-    const argList = match[2] || ''
+  constructor (name: string, args: string[], strictFilters: boolean) {
     const impl = Filter.impls[name]
     if (!impl && strictFilters) throw new TypeError(`undefined filter: ${name}`)
 
     this.name = name
     this.impl = impl || (x => x)
-    this.args = this.parseArgs(argList)
-  }
-  parseArgs (argList: string): string[] {
-    let match; const args: string[] = []
-    while ((match = valueRE.exec(argList.trim()))) {
-      const v = match[0]
-      const re = new RegExp(`${v}\\s*:`, 'g')
-      const keyMatch = re.exec(match.input)
-      const currentMatchIsKey = keyMatch && keyMatch.index === match.index
-      currentMatchIsKey ? args.push(`'${v}'`) : args.push(v)
-    }
-    return args
+    this.args = args
   }
   render (value: any, scope: Scope): any {
     const args = this.args.map(arg => evalValue(arg, scope))

src/template/output.ts

@@ -7,7 +7,7 @@ import OutputToken from 'src/parser/output-token'
 
 export default class Output extends Template<OutputToken> implements ITemplate {
   value: Value
-  constructor (token: OutputToken, strictFilters?: boolean) {
+  constructor (token: OutputToken, strictFilters: boolean) {
     super(token)
     this.value = new Value(token.value, strictFilters)
   }

src/template/value.ts

@@ -1,21 +1,84 @@
 import { evalExp } from 'src/render/syntax'
 import * as lexical from 'src/parser/lexical'
-import assert from 'src/util/assert'
 import Filter from './filter/filter'
 import Scope from 'src/scope/scope'
 
+
+enum ParseState {
+  INIT = 0,
+  FILTER_NAME = 1,
+  FILTER_ARG = 2
+}
+
 export default class {
   initial: any
   filters: Array<Filter> = []
-  constructor (str: string, strictFilters?: boolean) {
-    let match: RegExpExecArray | null = lexical.matchValue(str) as RegExpExecArray
-    assert(match, `illegal value string: ${str}`)
 
-    this.initial = match[0]
-    str = str.substr(match.index + match[0].length)
+  /**
+   * @param str value string, like: "i have a dream | truncate: 3
+   */
+  constructor (str: string, strictFilters: boolean) {
+    const N = str.length
+    let buffer = ''
+    let quoted = ''
+    let state = ParseState.INIT
+    let sealed = false
 
-    while ((match = lexical.filter.exec(str))) {
-      this.filters.push(new Filter(match[0].trim(), strictFilters))
+    let filterName = ''
+    let filterArgs: string[] = []
+
+    for(let i = 0; i < str.length; i++) {
+      if (quoted) {
+        if (str[i] == quoted) {
+          quoted = ''
+          sealed = true
+        }
+        buffer += str[i]
+      }
+      else if (/\s/.test(str[i])) {
+        if (!buffer) continue
+        else sealed = true
+      }
+      else if (str[i] === '|') {
+        if (state === ParseState.INIT) {
+          this.initial = buffer
+        }
+        else {
+          if (state === ParseState.FILTER_NAME) filterName = buffer
+          else filterArgs.push(buffer)
+          this.filters.push(new Filter(filterName, filterArgs, strictFilters))
+          filterName = ''
+          filterArgs = []
+        }
+        state = ParseState.FILTER_NAME
+        buffer = ''
+        sealed = false
+      }
+      else if (state === ParseState.FILTER_NAME && str[i] === ':') {
+        filterName = buffer
+        state = ParseState.FILTER_ARG
+        buffer = ''
+        sealed = false
+      }
+      else if (state === ParseState.FILTER_ARG && str[i] === ',') {
+        filterArgs.push(buffer)
+        buffer = ''
+        sealed = false
+      }
+      else if (sealed) continue
+      else {
+        if ((str[i] === '"' || str[i] === "'") && !quoted) quoted = str[i]
+        buffer += str[i]
+      }
+    }
+
+    if (buffer) {
+      if (state === ParseState.INIT) this.initial = buffer
+      else if (state === ParseState.FILTER_NAME) this.filters.push(new Filter(buffer, [], strictFilters))
+      else {
+        filterArgs.push(buffer)
+        this.filters.push(new Filter(filterName, filterArgs, strictFilters))
+      }
     }
   }
   value (scope: Scope) {

test/unit/liquid/liquid.ts

@@ -23,6 +23,18 @@ describe('Liquid', function () {
       expect(html).to.equal('true')
     })
   })
+  describe('#parseAndRender', function () {
+    const engine = new Liquid()
+    it('should parse and render variable output', async function () {
+      const html = await engine.parseAndRender('{{"foo"}}')
+      expect(html).to.equal('foo')
+    })
+    it('should parse and render complex output', async function () {
+      const tpl = '{{ "Welcome|to]Liquid" | split: "|" | join: "("}}'
+      const html = await engine.parseAndRender(tpl)
+      expect(html).to.equal('Welcome(to]Liquid')
+    })
+  })
   describe('#express()', function () {
     const liquid = new Liquid({ root: '/root' })
     const render = liquid.express()

test/unit/template/filter.ts

@@ -1,95 +0,0 @@
-import * as chai from 'chai'
-import * as sinon from 'sinon'
-import * as sinonChai from 'sinon-chai'
-import Filter from 'src/template/filter/filter'
-import Scope from 'src/scope/scope'
-
-chai.use(sinonChai)
-const expect = chai.expect
-
-describe('filter', function () {
-  let scope: Scope
-  beforeEach(function () {
-    Filter.clear()
-    scope = new Scope()
-  })
-  it('should return default filter when not registered', function () {
-    const result = new Filter('foo')
-    expect(result.name).to.equal('foo')
-  })
-
-  it('should throw when filter name illegal', function () {
-    expect(function () {
-      new Filter('/')
-    }).to.throw(/illegal filter/)
-  })
-
-  it('should parse argument syntax', function () {
-    Filter.register('foo', x => x)
-    const f = new Filter('foo: a, "b"')
-
-    expect(f.name).to.equal('foo')
-    expect(f.args).to.deep.equal(['a', '"b"'])
-  })
-
-  it('should register a simple filter', function () {
-    Filter.register('upcase', x => x.toUpperCase())
-    expect(new Filter('upcase').render('foo', scope)).to.equal('FOO')
-  })
-
-  it('should register a argumented filter', function () {
-    Filter.register('add', (a, b) => a + b)
-    expect(new Filter('add: 2').render(3, scope)).to.equal(5)
-  })
-
-  it('should register a multi-argumented filter', function () {
-    Filter.register('add', (a, b, c) => a + b + c)
-    expect(new Filter('add: 2, "c"').render(3, scope)).to.equal('5c')
-  })
-
-  it('should call filter with corrct arguments', function () {
-    const spy = sinon.spy()
-    Filter.register('foo', spy)
-    new Filter('foo: 33').render('foo', scope)
-    expect(spy).to.have.been.calledWith('foo', 33)
-  })
-
-  it('should support arguments as named key/values', function () {
-    Filter.register('foo', x => x)
-    const f = new Filter('foo: key1: "literal1", key2: value2')
-    expect(f.name).to.equal('foo')
-    expect(f.args).to.deep.equal([ '\'key1\'', '"literal1"', '\'key2\'', 'value2' ])
-  })
-
-  it('should support arguments as named key/values with inline literals', function () {
-    Filter.register('foo', x => x)
-    const f = new Filter('foo: "test0", key1: "literal1", key2: value2')
-    expect(f.name).to.equal('foo')
-    expect(f.args).to.deep.equal([ '"test0"', '\'key1\'', '"literal1"', '\'key2\'', 'value2' ])
-  })
-
-  it('should support arguments as named key/values with inline values', function () {
-    Filter.register('foo', x => x)
-    const f = new Filter('foo: test0, key1: "literal1", key2: value2')
-    expect(f.name).to.equal('foo')
-    expect(f.args).to.deep.equal([ 'test0', '\'key1\'', '"literal1"', '\'key2\'', 'value2' ])
-  })
-
-  it('should support argument values named same as keys', function () {
-    Filter.register('foo', x => x)
-    const f = new Filter('foo: a: a')
-    expect(f.name).to.equal('foo')
-    expect(f.args).to.deep.equal(['\'a\'', 'a'])
-  })
-
-  it('should support argument literals named same as keys', function () {
-    Filter.register('foo', x => x)
-    const f = new Filter('foo: a: "a"')
-    expect(f.name).to.equal('foo')
-    expect(f.args).to.deep.equal(['\'a\'', '"a"'])
-  })
-
-  it('should not throw undefined filter by default', function () {
-    expect(new Filter('undefined').render('foo', scope)).to.equal('foo')
-  })
-})

test/unit/template/filter/filter.ts

@@ -0,0 +1,51 @@
+import * as chai from 'chai'
+import * as sinon from 'sinon'
+import * as sinonChai from 'sinon-chai'
+import Filter from 'src/template/filter/filter'
+import Scope from 'src/scope/scope'
+
+chai.use(sinonChai)
+const expect = chai.expect
+
+describe('filter', function () {
+  let scope: Scope
+  beforeEach(function () {
+    Filter.clear()
+    scope = new Scope()
+  })
+  it('should create default filter if not registered', function () {
+    const result = new Filter('foo', [], false)
+    expect(result.name).to.equal('foo')
+  })
+
+  it('should render input if filter not registered', function () {
+    expect(new Filter('undefined', [], false).render('foo', scope)).to.equal('foo')
+  })
+
+  it('should call filter impl with corrct arguments', function () {
+    const spy = sinon.spy()
+    Filter.register('foo', spy)
+    new Filter('foo', ['33'], false).render('foo', scope)
+    expect(spy).to.have.been.calledWith('foo', 33)
+  })
+  it('should render a simple filter', function () {
+    Filter.register('upcase', x => x.toUpperCase())
+    expect(new Filter('upcase', [], false).render('foo', scope)).to.equal('FOO')
+  })
+
+  it('should render filters with argument', function () {
+    Filter.register('add', (a, b) => a + b)
+    expect(new Filter('add', ["2"], false).render(3, scope)).to.equal(5)
+  })
+
+  it('should render filters with multiple arguments', function () {
+    Filter.register('add', (a, b, c) => a + b + c)
+    expect(new Filter('add', ['2', '"c"'], false).render(3, scope)).to.equal('5c')
+  })
+
+  it('should not throw when filter name illegal', function () {
+    expect(function () {
+      new Filter('/', [], false)
+    }).to.not.throw()
+  })
+})

test/unit/template/output.ts

@@ -15,7 +15,7 @@ describe('Output', function () {
     const scope = new Scope({
       bar: { to_liquid: () => 'custom' }
     })
-    const output = new Output({ value: 'bar' } as OutputToken)
+    const output = new Output({ value: 'bar' } as OutputToken, false)
     const html = await output.render(scope)
     return expect(html).to.equal('custom')
   })
@@ -23,38 +23,38 @@ describe('Output', function () {
     const scope = new Scope({
       foo: { obj: { arr: ['a', 2] } }
     })
-    const output = new Output({ value: 'foo' } as OutputToken)
+    const output = new Output({ value: 'foo' } as OutputToken, false)
     const html = await output.render(scope)
     return expect(html).to.equal('{"obj":{"arr":["a",2]}}')
   })
   it('should skip circular property', async function () {
     const ctx = { foo: { num: 2 }, bar: 'bar' } as any
     ctx.foo.circular = ctx
-    const output = new Output({ value: 'foo' } as OutputToken)
+    const output = new Output({ value: 'foo' } as OutputToken, false)
     const html = await output.render(new Scope(ctx))
     return expect(html).equal('{"num":2,"circular":{"bar":"bar"}}')
   })
   it('should skip function property', async function () {
     const scope = new Scope({ obj: { foo: 'foo', bar: (x: any) => x } })
-    const output = new Output({ value: 'obj' } as OutputToken)
+    const output = new Output({ value: 'obj' } as OutputToken, false)
     const html = await output.render(scope)
     return expect(html).to.equal('{"foo":"foo"}')
   })
   it('should respect to .toString()', async () => {
     const scope = new Scope({ obj: { toString: () => 'FOO' } })
-    const output = new Output({ value: 'obj' } as OutputToken)
+    const output = new Output({ value: 'obj' } as OutputToken, false)
     const str = await output.render(scope)
     return expect(str).to.equal('FOO')
   })
   it('should respect to .to_s()', async () => {
     const scope = new Scope({ obj: { to_s: () => 'FOO' } })
-    const output = new Output({ value: 'obj' } as OutputToken)
+    const output = new Output({ value: 'obj' } as OutputToken, false)
     const str = await output.render(scope)
     return expect(str).to.equal('FOO')
   })
   it('should respect to .liquid_method_missing()', async () => {
     const scope = new Scope({ obj: { liquid_method_missing: (x: string) => x.toUpperCase() } })
-    const output = new Output({ value: 'obj.foo' } as OutputToken)
+    const output = new Output({ value: 'obj.foo' } as OutputToken, false)
     const str = await output.render(scope)
     return expect(str).to.equal('FOO')
   })

test/unit/template/value.ts

@@ -8,57 +8,50 @@ import Value from 'src/template/value'
 chai.use(sinonChai)
 
 const expect = chai.expect
-const add = (l: number, r: number) => l + r
 
 describe('Value', function () {
   beforeEach(() => Filter.clear())
 
-  it('should throw when value string illegal', function () {
-    expect(function () {
-      new Value('/')
-    }).to.throw(/illegal value string/)
-  })
-
-  it('should parse value string', function () {
-    const tpl: any = new Value('foo')
+  it('should parse "foo', function () {
+    const tpl: any = new Value('foo', false)
     expect(tpl.initial).to.equal('foo')
     expect(tpl.filters).to.deep.equal([])
   })
 
-  it('should parse value string with a simple filter', function () {
-    Filter.register('add', add)
-    const tpl: any = new Value('foo | add: 3, "foo"')
+  it('should parse "foo | add"', function () {
+    const tpl: any = new Value('foo | add', false)
     expect(tpl.initial).to.equal('foo')
     expect(tpl.filters.length).to.equal(1)
-    expect(tpl.filters[0].impl).to.equal(add)
+    expect(tpl.filters[0].args).to.eql([])
   })
 
-  it('should parse value string with filters', function () {
-    const tpl: any = new Value('foo | add: "|" | add')
+  it('should parse "foo | add: 3, false"', function () {
+    const tpl: any = new Value('foo | add: 3, "foo"', false)
     expect(tpl.initial).to.equal('foo')
-    expect(tpl.filters.length).to.equal(2)
+    expect(tpl.filters.length).to.equal(1)
+    expect(tpl.filters[0].args).to.eql(['3', '"foo"'])
   })
 
-  it('should eval value', function () {
-    Filter.register('date', (l, r) => l + r)
-    Filter.register('time', (l, r) => l + 3 * r)
-    const tpl = new Value('foo.bar[0] | date: "b" | time:2')
-    const scope = new Scope({
-      foo: { bar: ['a'] }
-    })
-    expect(tpl.value(scope)).to.equal('ab6')
+  it('should parse "foo | add: "|", 3', function () {
+    const tpl: any = new Value('foo | add: "|", 3', false)
+    expect(tpl.initial).to.equal('foo')
+    expect(tpl.filters.length).to.equal(1)
+    expect(tpl.filters[0].args).to.eql(['"|"', '3'])
   })
-  it('should reserve type', function () {
-    Filter.register('arr', () => [1])
-    const tpl = new Value('"x" | arr')
-    expect(tpl.value(new Scope())).to.deep.equal([1])
+
+  it('should parse "foo | add: "|", 3', function () {
+    const tpl: any = new Value('foo | add: "|", 3', false)
+    expect(tpl.initial).to.equal('foo')
+    expect(tpl.filters.length).to.equal(1)
+    expect(tpl.filters[0].args).to.eql(['"|"', '3'])
   })
-  it('should eval filter with correct arguments', function () {
+
+  it('should call chained filters correctly', function () {
     const date = sinon.stub().returns('y')
     const time = sinon.spy()
     Filter.register('date', date)
     Filter.register('time', time)
-    const tpl = new Value('foo.bar | date: "b" | time:2')
+    const tpl = new Value('foo.bar | date: "b" | time:2', false)
     const scope = new Scope({
       foo: { bar: 'bar' }
     })

test/unit/util/error.ts

@@ -193,7 +193,7 @@ describe('error', function () {
         }
       })
     })
-    it('should throw RenderError when filter not defined', async function () {
+    it('should throw ParseError when filter not defined', async function () {
       const err = await expect(strictEngine.parseAndRender('{{1 | a}}')).be.rejected
       expect(err).to.have.property('name', 'ParseError')
       expect(err.message).to.contain('undefined filter: a')