feat: Add support of inner expressions enclosed by parentheses (#863)

* Add support of inner expressions enclosed by parentheses * Add support of inner expressions enclosed by parentheses Made-with: Cursor * simplify implementation * fix lint * fix test * Enhance tests for parenthesized filter chains in Liquid tags. Added scenarios for enabled and disabled grouped expressions in case, for, if, unless tags, ensuring proper handling of expressions and error throwing for invalid syntax. * test: remove duplicate readGroupedExpression test block The readGroupedExpression() test suite was duplicated twice in the spec file. Removed the duplicate block to avoid redundant test execution. * refactor: extract extractGroupedExpressionTokenVariables helper Extract inline grouped expression variable extraction logic into a dedicated function for consistency with other extractors (extractFilteredValueVariables, extractPropertyAccessVariable). This addresses PR #863 comment 7 - improves code organization and maintainability. * refactor(types): explicit type for collection in for tag collection: ValueToken | GroupedExpressionToken Addresses PR #863 comment 5. * refactor: evaluate grouped expressions at render time with resolvedFilters Addresses PR review comments 4, 6, 8, 9 - moves grouped expression evaluation from parse-time resolution to render-time lazy evaluation following the generator-based async/sync duality pattern used throughout liquidjs. Key changes: - Replace resolvedValue (Value instance) with resolvedFilters (Filter[]) - Rename resolveGroupedExpressions() to resolveGroupedExpressionFilters() - Move evaluation logic to evalGroupedExpressionToken() at render time - Build Filter instances at parse time (carry liquid reference for render) - Evaluate expression and apply filters lazily via generators - Add support for tablerow tag with grouped expressions - Remove duplicate getFilter() method in Value class Maintains proper layering (tokens → render → templates) and consistency with Value.value() pattern. Filter resolution still happens at parse time since it requires liquid.filters access, but actual evaluation is deferred to render time. Tags that store raw ValueToken (for, case when-values, tablerow) still need explicit resolveGroupedExpressionFilters() calls. Tags that wrap with new Value() get automatic recursive resolution via Value constructor. * refactor: reuse FilteredValueToken and fix architectural layering Replace GroupedExpressionToken with existing FilteredValueToken to avoid code duplication and fix layering violation where tokens depended on templates (Filter instances). Key changes: - Reuse FilteredValueToken instead of GroupedExpressionToken - Simplify readGroupOrRange() to return FilteredValueToken | RangeToken - Add liquid reference to Context for runtime filter resolution - Build Filter instances at render time in evalFilteredValueToken() - Remove resolveGroupedExpressionFilters() and parse-time resolution - Remove explicit resolution calls from tag constructors This maintains proper architectural layering (tokens → render → templates) with no backward dependencies, as requested in PR review feedback. All 1537 tests pass. * revert redundant' * refactor: make getFilter private and improve code organization * test: fix test name in case.spec.ts for when disabled block * refactor: no need for Deprecated flag * test: fix test name and logic to properly test if tag with nested expressions * feat: support real parenthesis grouping in grouped expressions Allow arbitrary expressions inside parentheses (e.g. ((a | upcase) > 3) and (1 < 3)) when groupedExpressions is enabled, reusing readFilteredValue for the general case while keeping range and filter-chain fast paths. * feat: enhance expression tokenization with new generator methods Added `readExpressionTokensFromHere` and `readGroupedExpressionTokens` methods to improve the handling of expression tokens. This refactor simplifies the token reading process and maintains compatibility with existing grouped expressions, ensuring proper evaluation and filtering. * add tests * address comments --------- Co-authored-by: Omri Rosner <[email protected]>
2026-09-16 21:00:40 -07:00 · 2026-08-01 18:15:54 +08:00
parent 39c87437c5
commit afa5f54004
27 changed files with 393 additions and 47 deletions
@@ -1,4 +1,4 @@
-import { LiquidTagToken, HTMLToken, QuotedToken, OutputToken, TagToken, OperatorToken, RangeToken, PropertyAccessToken, NumberToken, IdentifierToken } from '../tokens'
+import { LiquidTagToken, HTMLToken, QuotedToken, OutputToken, TagToken, OperatorToken, RangeToken, PropertyAccessToken, NumberToken, IdentifierToken, FilteredValueToken } from '../tokens'
 import { Tokenizer } from './tokenizer'
 import { defaultOperators } from '../render/operator'
 import { createTrie } from '../util/operator-trie'
@@ -229,24 +229,115 @@ describe('Tokenizer', function () {
  })
  describe('#readRange()', () => {
    it('should read `(1..3)`', () => {
-      const range = new Tokenizer('(1..3)').readRange()
+      const range = new Tokenizer('(1..3)').readGroupOrRange()
+      expect(range).toBeDefined()
      expect(range).toBeInstanceOf(RangeToken)
      expect(range!.getText()).toEqual('(1..3)')
-      const { lhs, rhs } = range!
-      expect(lhs).toBeInstanceOf(NumberToken)
-      expect(lhs.getText()).toBe('1')
-      expect(rhs).toBeInstanceOf(NumberToken)
-      expect(rhs.getText()).toBe('3')
+      expect((range as RangeToken).lhs).toBeInstanceOf(NumberToken)
+      expect((range as RangeToken).lhs.getText()).toBe('1')
+      expect((range as RangeToken).rhs).toBeInstanceOf(NumberToken)
+      expect((range as RangeToken).rhs.getText()).toBe('3')
    })
    it('should throw for `(..3)`', () => {
-      expect(() => new Tokenizer('(..3)').readRange()).toThrow('unexpected token "..3)", value expected')
+      expect(() => new Tokenizer('(..3)').readGroupOrRange()).toThrow('unexpected token "..3)", value expected')
    })
    it('should read `(a.b..c["..d"])`', () => {
-      const range = new Tokenizer('(a.b..c["..d"])').readRange()
+      const range = new Tokenizer('(a.b..c["..d"])').readGroupOrRange()
+      expect(range).toBeDefined()
      expect(range).toBeInstanceOf(RangeToken)
      expect(range!.getText()).toEqual('(a.b..c["..d"])')
    })
  })
+  describe('#readGroupedExpression()', () => {
+    function createGrouped (input: string): Tokenizer {
+      const t = new Tokenizer(input, defaultOperators)
+      t.groupedExpressions = true
+      return t
+    }
+    it('should read `(foo | upcase)` as FilteredValueToken', () => {
+      const token = createGrouped('(foo | upcase)').readValue()
+      expect(token).toBeInstanceOf(FilteredValueToken)
+      const grouped = token as FilteredValueToken
+      expect(grouped.getText()).toBe('(foo | upcase)')
+      expect(grouped.initial.postfix).toHaveLength(1)
+      expect(grouped.filters).toHaveLength(1)
+      expect(grouped.filters[0].name).toBe('upcase')
+    })
+    it('should read `(foo | append: "!")` with filter argument', () => {
+      const token = createGrouped('(foo | append: "!")').readValue()
+      expect(token).toBeInstanceOf(FilteredValueToken)
+      const grouped = token as FilteredValueToken
+      expect(grouped.filters).toHaveLength(1)
+      expect(grouped.filters[0].name).toBe('append')
+      expect(grouped.filters[0].args).toHaveLength(1)
+    })
+    it('should read nested `((foo | append: "!") | upcase)`', () => {
+      const token = createGrouped('((foo | append: "!") | upcase)').readValue()
+      expect(token).toBeInstanceOf(FilteredValueToken)
+      const grouped = token as FilteredValueToken
+      expect(grouped.filters).toHaveLength(1)
+      expect(grouped.filters[0].name).toBe('upcase')
+      expect(grouped.initial.postfix).toHaveLength(1)
+      expect(grouped.initial.postfix[0]).toBeInstanceOf(FilteredValueToken)
+    })
+    it('should parse `(a | upcase) == "BAR"` as expression', () => {
+      const exp = [...createGrouped('(a | upcase) == "BAR"').readExpressionTokens()]
+      expect(exp).toHaveLength(3)
+      expect(exp[0]).toBeInstanceOf(FilteredValueToken)
+      expect(exp[1]).toBeInstanceOf(OperatorToken)
+      expect(exp[1].getText()).toBe('==')
+      expect(exp[2]).toBeInstanceOf(QuotedToken)
+    })
+    it('should read `((a | upcase) > 3)` as outer FilteredValueToken with comparison inside parens', () => {
+      const token = createGrouped('((a | upcase) > 3)').readValue()
+      expect(token).toBeInstanceOf(FilteredValueToken)
+      const outer = token as FilteredValueToken
+      expect(outer.filters).toHaveLength(0)
+      expect(outer.getText()).toBe('((a | upcase) > 3)')
+      const [first, second, third] = outer.initial.postfix
+      expect(first).toBeInstanceOf(FilteredValueToken)
+      expect(second).toBeInstanceOf(NumberToken)
+      expect(third).toBeInstanceOf(OperatorToken)
+      expect((first as FilteredValueToken).filters[0].name).toBe('upcase')
+    })
+    it('should read `(1 < 3)` as grouped comparison with no filters', () => {
+      const token = createGrouped('(1 < 3)').readValue() as FilteredValueToken
+      expect(token.filters).toHaveLength(0)
+      expect(token.initial.postfix).toHaveLength(3)
+      expect(token.initial.postfix[0]).toBeInstanceOf(NumberToken)
+      expect(token.initial.postfix[1]).toBeInstanceOf(NumberToken)
+      expect((token.initial.postfix[2] as OperatorToken).operator).toBe('<')
+    })
+    it('should read redundant parens `(x)` as FilteredValueToken', () => {
+      const token = createGrouped('(x)').readValue() as FilteredValueToken
+      expect(token.filters).toHaveLength(0)
+      expect(token.initial.postfix).toHaveLength(1)
+    })
+    it('should read expression plus filters inside parens `(a == b | default: "x")`', () => {
+      const token = createGrouped('(a == b | default: "x")').readValue() as FilteredValueToken
+      expect(token.filters).toHaveLength(1)
+      expect(token.filters[0].name).toBe('default')
+      expect(token.initial.postfix.map((t) => t.getText()).join(' ')).toMatch(/a.*b.*==/)
+    })
+    it('should parse `((a | upcase) > 3) and (1 < 3)` as three expression tokens', () => {
+      const exp = [...createGrouped('((a | upcase) > 3) and (1 < 3)').readExpressionTokens()]
+      expect(exp).toHaveLength(3)
+      expect(exp[0]).toBeInstanceOf(FilteredValueToken)
+      expect(exp[1]).toBeInstanceOf(OperatorToken)
+      expect(exp[1].getText()).toBe('and')
+      expect(exp[2]).toBeInstanceOf(FilteredValueToken)
+    })
+    it('should still parse `(1..3)` as RangeToken', () => {
+      const token = createGrouped('(1..3)').readValue()
+      expect(token).toBeInstanceOf(RangeToken)
+    })
+    it('should throw for unclosed parens', () => {
+      expect(() => createGrouped('(foo | upcase').readValue()).toThrow('unbalanced parentheses')
+    })
+    it('should fall back to readRange when flag is off', () => {
+      expect(() => new Tokenizer('(foo | upcase)', defaultOperators).readValue()).toThrow('invalid range syntax')
+    })
+  })
  describe('#readFilter()', () => {
    it('should read a simple filter', function () {
      const tokenizer = new Tokenizer('| plus')