recursively parse brackets on variable lookup

[ggmichaelgo]

Problem

With dynamic variable lookup, Liquid is not properly parsing the expression.
With this template:

{{ [list[settings.zero]] }}

VariableLookup matches [list[settings.zero] for the name without the extra closing bracket.
This causes an issue because VariableLookup parses the expression with Expression.parse(name[1..-2]) which is list[settings.zero and Liquid creates a VariableLookup where the name is list and lookups are ["settings", "zero"].

This problem also exists for the double-nested variable lookups such as this template: list[llist[setting.value]]

Solution

We can solve this issue by recursively parsing the dynamic lookups:

\[(?:[^\[\]]+|\g<0>)*\]

\[          # match open bracket
(?:         # start non-capturing group
  [^\[\]]+  # match non-square brackets
  |         # OR
  \g<0>     # go back to the beginning of the regex
)*          # end of non-capturing group
\]          # match closing bracket

[dylanahsmith]

Recursive parsing is actually something that Liquid::Parser does correctly, which would cause it to recognize this syntax as strictly valid. However, it then builds an expression String that it passes to the lax parser, which causes it to have this behaviour.

liquid-c actually parses this properly when the disable_liquid_c_nodes: true parse option isn't being used, since it will build the expression directly when the liquid code is strictly parseable.

As such, it seems like this is more clearly a bug in the strict parser than the lax parser.

This causes an issue because VariableLookup parses the expression with Expression.parse(name[1..-2]) which is list[settings.zero and Liquid creates a VariableLookup where the name is list and lookups are ["settings", "zero"].

Unfortunately, this highlights how hard it is to tell whether a liquid change will break existing code, since this can lead to code that accidentally works or fails more gracefully (e.g. avoiding broken conditional code). That means that if it is too disruptive, we may need to revert, but we would still want the fix to the strict parser.

Fundamentally, the approach taken to the strict parser is quite fragile and inefficient by parsing, rebuilding the markup, then using the lax parser to parse that rebuilt markup. Although, it looks like a fairly large refactor might be needed to support that (e.g. constructors are coupled to parsing and there is some quirky coupling to markup). So it would be nice to not depend on that work to fix this liquid-c inconsistency.