Skip to content

Commit

Permalink
LiquidParser: Fix antlr warning about matching an empty string
Browse files Browse the repository at this point in the history
antlr rightfully complains: "rule output contains an optional block with
at least one alternative that can match an empty string" (which hints at
a potential performance problem).

This is due to "not_out_end" matching with "*" (which can yield an empty
string), even though we already specify "?" at "unparsed=not_out_end?".

Fix the parser grammar, and adjust two test cases where we actually
tested for that undesired behavior.

Also see
https://stackoverflow.com/questions/26041293/antlr-4-warning-rule-contains-an-optional-block-with-at-least-one-alternative
for an explanation.

Regression introduced in #269
  • Loading branch information
kohlschuetter committed Jun 27, 2023
1 parent 9fd5a5c commit edc1d9d
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/main/antlr4/liquid/parser/v4/LiquidParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,13 @@ jekyll_include_params
;

output
: {evaluateInOutputTag}? outStart evaluate=expr filter* OutEnd
: {isEvaluateInOutputTag()}? outStart evaluate=expr filter* OutEnd
| {isStrict()}? outStart term filter* OutEnd
| {isWarn() || isLax()}? outStart term filter* unparsed=not_out_end? OutEnd
;

not_out_end
: ~( OutEnd )*
: ( ~OutEnd )+
;

filter
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/liqp/parser/v4/LiquidParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -387,12 +387,12 @@ public void testOutput() {

assertThat(
texts("{{ true }}", "output"),
equalTo(array("{{", "true", "", "}}"))
equalTo(array("{{", "true", "}}"))
);

assertThat(
texts("{{ 'some string here' | uppercase }}", "output"),
equalTo(array("{{", "'some string here'", "|uppercase", "", "}}"))
equalTo(array("{{", "'some string here'", "|uppercase", "}}"))
);
}

Expand Down

0 comments on commit edc1d9d

Please sign in to comment.