fix: in conditionals, don't render anything after an else branch (#671)

* fix: only render the first 'else' template in the case of multiples * fix: empty else block and cases with when conditions * fix: don't render elsif after else * chore: Update src/tags/unless.ts * chore: Update src/tags/if.ts * chore: Update src/tags/case.ts --------- Co-authored-by: Jun Yang <harttleharttle@gmail.com>
harttle · Feb 18, 2024 · f816955 · f816955
1 parent 1937aa1
commit f816955
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 17 deletions.
diff --git a/src/tags/case.ts b/src/tags/case.ts
@@ -10,8 +10,13 @@ export default class extends Tag {
     this.elseTemplates = []
 
     let p: Template[] = []
+    let elseCount = 0
     const stream: ParseStream = this.liquid.parser.parseStream(remainTokens)
       .on('tag:when', (token: TagToken) => {
+        if (elseCount > 0) {
+          return
+        }
+
         p = []
 
         const values: ValueToken[] = []
@@ -29,9 +34,16 @@ export default class extends Tag {
           templates: p
         })
       })
-      .on('tag:else', () => (p = this.elseTemplates))
+      .on('tag:else', () => {
+        elseCount++
+        p = this.elseTemplates
+      })
       .on('tag:endcase', () => stream.stop())
-      .on('template', (tpl: Template) => p.push(tpl))
+      .on('template', (tpl: Template) => {
+        if (p !== this.elseTemplates || elseCount === 1) {
+          p.push(tpl)
+        }
+      })
       .on('end', () => {
         throw new Error(`tag ${tagToken.getText()} not closed`)
       })

diff --git a/src/tags/if.ts b/src/tags/if.ts
@@ -6,19 +6,33 @@ export default class extends Tag {
 
   constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
     super(tagToken, remainTokens, liquid)
-    let p
+    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) => this.branches.push({
-        value: new Value(token.args, this.liquid),
-        templates: (p = [])
-      }))
-      .on('tag:else', () => (p = this.elseTemplates))
+      .on('tag:elsif', (token: TagToken) => {
+        if (elseCount > 0) {
+          p = []
+          return
+        }
+        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) => p.push(tpl))
+      .on('template', (tpl: Template) => {
+        if (p !== this.elseTemplates || elseCount === 1) {
+          p.push(tpl)
+        }
+      })
       .on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) })
       .start()
   }

diff --git a/src/tags/unless.ts b/src/tags/unless.ts
@@ -5,21 +5,35 @@ export default class extends Tag {
   elseTemplates: Template[] = []
   constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
     super(tagToken, remainTokens, liquid)
-    let p
+    let p: Template[] = []
+    let elseCount = 0
     this.liquid.parser.parseStream(remainTokens)
       .on('start', () => this.branches.push({
         value: new Value(tagToken.args, this.liquid),
         test: isFalsy,
         templates: (p = [])
       }))
-      .on('tag:elsif', (token: TagToken) => this.branches.push({
-        value: new Value(token.args, this.liquid),
-        test: isTruthy,
-        templates: (p = [])
-      }))
-      .on('tag:else', () => (p = this.elseTemplates))
+      .on('tag:elsif', (token: TagToken) => {
+        if (elseCount > 0) {
+          p = []
+          return
+        }
+        this.branches.push({
+          value: new Value(token.args, this.liquid),
+          test: isTruthy,
+          templates: (p = [])
+        })
+      })
+      .on('tag:else', () => {
+        elseCount++
+        p = this.elseTemplates
+      })
       .on('tag:endunless', function () { this.stop() })
-      .on('template', (tpl: Template) => p.push(tpl))
+      .on('template', (tpl: Template) => {
+        if (p !== this.elseTemplates || elseCount === 1) {
+          p.push(tpl)
+        }
+      })
       .on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) })
       .start()
   }

diff --git a/test/e2e/issues.spec.ts b/test/e2e/issues.spec.ts
@@ -469,4 +469,20 @@ describe('Issues', function () {
     const result = engine.parseAndRenderSync('{{ÜLKE}}', { ÜLKE: 'Türkiye' })
     expect(result).toEqual('Türkiye')
   })
+  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')
+  })
+  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')
+  })
 })
diff --git a/test/integration/tags/case.spec.ts b/test/integration/tags/case.spec.ts
@@ -91,4 +91,32 @@ describe('tags/case', function () {
     const html = await liquid.parseAndRender(src)
     return expect(html).toBe('and or or')
   })
+  it('should not render anything after an else branch', async function () {
+    const html = await liquid.parseAndRenderSync('{% assign value = "this" %}' +
+    '{% case true %}' +
+      '{% when false %}don\'t show' +
+      '{% else %}show {{ value }}' +
+      '{% else %}don\'t show' +
+    '{% endcase %}', {})
+    expect(html).toEqual('show this')
+  })
+  it('should not render anything after an else branch even when first else branch is empty', async function () {
+    const html = await liquid.parseAndRenderSync('{% case true %}' +
+      '{% when false %}don\'t show' +
+      '{% else %}' +
+      '{% else %}don\'t show' +
+    '{% endcase %}', {})
+    expect(html).toEqual('')
+  })
+  it('should not render anything after an else branch even when there are \'when\' conditions', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% assign value = "this" %}' +
+    '{% case true -%}' +
+      '{% when false -%}don\'t show' +
+      '{% else %}show {{ value }}' +
+      '{% else %}don\'t show' +
+      '{%- when true -%}don\'t show' +
+    '{%- endcase %}', {})
+    expect(result).toEqual('show this')
+  })
 })
diff --git a/test/integration/tags/if.spec.ts b/test/integration/tags/if.spec.ts
@@ -143,4 +143,12 @@ describe('tags/if', function () {
     const html = await liquid.parseAndRender(src, scope)
     return expect(html).toBe('success')
   })
+  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('')
+  })
 })
diff --git a/test/integration/tags/unless.spec.ts b/test/integration/tags/unless.spec.ts
@@ -47,6 +47,29 @@ describe('tags/unless', function () {
       Inside wonderland
       After wonderland`)
   })
+  it('should not render anything after an else branch', async function () {
+    const html = await liquid.parseAndRenderSync('{% assign value = "this" %}' +
+      '{% unless true %}don\'t show' +
+      '{% else %}show {{ value }}' +
+      '{% else %}don\'t show' +
+    '{% endunless %}', {})
+    expect(html).toEqual('show this')
+  })
+  it('should not render anything after an else branch even when first else branch is empty', async function () {
+    const html = await liquid.parseAndRenderSync('{% unless true %}don\'t show' +
+      '{% else %}' +
+      '{% else %}don\'t show' +
+    '{% endunless %}', {})
+    expect(html).toEqual('')
+  })
+  it('should not render an elseif after an else branch', () => {
+    const engine = new Liquid()
+    const result = engine.parseAndRenderSync('{% unless true %}don\'t show' +
+      '{% else %}show' +
+      '{% elsif true %}don\'t show' +
+    '{% endunless %}', {})
+    expect(result).toEqual('show')
+  })
 
   describe('sync support', function () {
     it('should render else when predicate yields true', function () {