Skip to content

Add strict2_parse to increment and decrement tags#2067

Open
aswamy wants to merge 1 commit intostrict2-assign-capturefrom
strict2-increment-decrement
Open

Add strict2_parse to increment and decrement tags#2067
aswamy wants to merge 1 commit intostrict2-assign-capturefrom
strict2-increment-decrement

Conversation

@aswamy
Copy link
Copy Markdown
Contributor

@aswamy aswamy commented Mar 26, 2026

Note

Stacked on Shopify/liquid#2065 (assign/capture strict2_parse)

Summary

Adds strict2_parse to Increment and Decrement tags. Both tags previously accepted any string as a variable name via markup.strip — no Parser validation was performed in any mode.

Changes:

  • Both tags now include ParserSwitching and use parse_with_selected_parser
  • lax_parse: preserves the original markup.strip behavior
  • strict_parse: delegates to lax_parse
  • strict2_parse: validates the variable name via p.consume(:id) + p.consume(:end_of_string)

This means {% increment foo bar %} and {% decrement 11aa %} are now rejected in strict2 mode.

Tophat

bundle install
bundle exec irb -r liquid
# Rejected in strict2
Liquid::Template.parse('{% increment foo bar %}', error_mode: :strict2)
# => Liquid::SyntaxError

Liquid::Template.parse('{% decrement 11aa %}', error_mode: :strict2)
# => Liquid::SyntaxError

# Accepted
Liquid::Template.parse('{% increment my-var %}', error_mode: :strict2)
# => OK

# Lax mode unchanged
Liquid::Template.parse('{% increment foo bar %}', error_mode: :lax)
# => OK (preserves backward compatibility)

🤖 Generated with Claude Code

@aswamy aswamy force-pushed the strict2-increment-decrement branch from b6bff03 to 3f70d61 Compare March 26, 2026 20:40
Comment on lines +43 to +50
def strict2_parse(markup)
stripped = markup.strip
return @variable_name = stripped if stripped.empty?

p = @parse_context.new_parser(stripped)
@variable_name = p.consume(:id)
p.consume(:end_of_string)
end
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought of merging the code between increment and decrement since they share so much, but didn't see it with other similar tags like case and if so i just kept it verbose

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fair to leave them separate. I do have a question.

The guard return @variable_name = stripped if stripped.empty? - I think we want to remove it because in other tags like capture if we don't find a variable name we raise a syntax error through p.consume(:id)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah i think that's fair. I looked at how lax mode handles {% increment %} (without a variable name) and apparently it's allowed. Internally, it looks like it's creating an empty-string variable that can be accessed via self[""]. Ill rewrite this into a valid variable name. Good catch!

@aswamy aswamy marked this pull request as ready for review March 26, 2026 20:54
@aswamy aswamy requested review from a team, EvilGenius13 and graygilmore March 26, 2026 20:56
@aswamy aswamy force-pushed the strict2-increment-decrement branch from 3f70d61 to 5365de4 Compare March 26, 2026 20:57
@aswamy aswamy added the #gsd:49922 Liquid Array Literal Support label Mar 26, 2026
Both tags previously accepted any string as a variable name via
`markup.strip`. Now they use `parse_with_selected_parser` and validate
the variable name with `p.consume(:id)` in strict2 mode.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@aswamy aswamy force-pushed the strict2-increment-decrement branch from 5365de4 to c990360 Compare March 27, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

#gsd:49922 Liquid Array Literal Support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants