Merge pull request #322 from bkiers/317-pop-out-after-comment-in-tag

[317] Pop out of mode when no tokens can possibly be created

Author: bkiers

src/main/antlr4/liquid/parser/v4/LiquidLexer.g4

@@ -188,7 +188,10 @@ mode IN_TAG;
 mode IN_TAG_ID;
   WS2 : WhitespaceChar+ -> channel(HIDDEN);
 
-  CommentInTagId : ( '#' ( {!linebreakOrTagEndAhead()}? . )* [ \t\r\n]* )+ -> channel(HIDDEN);
+  // When we encounter a comment in this "IN_TAG_ID" mode, there is no need to try to create more tokens
+  // since that will always result in an `InvalidEndTag` token. After all, a "#" comment consumes all
+  // characters until it "sees" an end-tag. Just pop out in that case.
+  CommentInTagId : ( '#' ( {!linebreakOrTagEndAhead()}? . )* [ \t\r\n]* )+ -> channel(HIDDEN), popMode;
 
   InvalidEndTag
      : ( '}}' SpaceOrTab* LineBreak? {stripSpacesAroundTags && stripSingleLine}?

src/main/antlr4/liquid/parser/v4/LiquidParser.g4

@@ -82,6 +82,7 @@ tag
  | include_relative_tag
  | continue_tag
  | simple_tag
+ | empty_tag
  | other_tag
  ;
 
@@ -112,6 +113,11 @@ simple_tag
   : TagStart SimpleTagId other_tag_parameters? TagEnd
   ;
 
+// A tag like `{% # inline comment %}` is considered empty: `{% %}`
+empty_tag
+ : TagStart TagEnd
+ ;
+
 raw_tag
  : TagStart RawStart raw_body RawEnd TagEnd
  ;

src/main/java/liqp/nodes/BlockNode.java

@@ -33,6 +33,13 @@ public List<LNode> getChildren() {
     public Object render(TemplateContext context) {
         ObjectAppender.Controller builder = context.newObjectAppender(children.size());
         for (LNode node : children) {
+
+            // Since tags can be "empty", `node` can be null, in which case we simply continue.
+            // For example, the tag `{% # inline comment %}` is considered "empty" since there is nothing inside.
+            if (node == null) {
+                continue;
+            }
+
             Object value = node.render(context);
             if (value == null) {
                 continue;

src/test/java/liqp/TemplateTest.java

@@ -325,6 +325,34 @@ public Object apply(Object value, TemplateContext context, Object... params) {
         assertEquals("15.0", rendered);
     }
 
+    // https://github.com/bkiers/Liqp/issues/317
+    @Test
+    public void testInlineComment() {
+
+        String source =
+                "{% # this is a comment %}\n" +
+                "\n" +
+                "{% # for i in (1..3) -%}\n" +
+                "  i={{ i }}\n" +
+                "{% # endfor %}\n" +
+                "\n" +
+                "{%\n" +
+                "  ###############################\n" +
+                "  # This is a comment\n" +
+                "  # across multiple lines\n" +
+                "  ###############################\n" +
+                "%}";
+
+        String expected =
+                "\n" +
+                "\n" +
+                "i=\n" +
+                "\n" +
+                "\n";
+
+        assertThat(TemplateParser.DEFAULT.parse(source).render(), is(expected));
+    }
+
     private Map<String, Object> getDeepData() {
         Map<String, Object> data = new HashMap<>();
         Map<?,?> secondIndex = Collections.singletonMap("e", "ok");
@@ -355,5 +383,4 @@ public void testWalk() {
         String res = TemplateParser.DEFAULT.parse("{{foo.a}}").toStringTree();
         assertNotNull(res);
     }
-
 }

src/test/java/liqp/parser/v4/LiquidLexerTest.java

@@ -577,6 +577,7 @@ public void testOtherRaw() {
         assertThat(tokenise("{%raw%}?").get(2).getType(), is(LiquidLexer.OtherRaw));
     }
 
+    // https://github.com/bkiers/Liqp/issues/317
     @Test
     public void testInlineComment() {
 
@@ -599,6 +600,48 @@ public void testInlineComment() {
                 "  ###############################-\n"));
     }
 
+    // https://github.com/bkiers/Liqp/issues/317
+    @Test
+    public void testInlineCommentInTag() {
+        String source =
+                "{% # for i in (1..3) -%}\n" +
+                "{{ i }}\n" +
+                "{% # endfor %}";
+
+        List<Token> tokens = tokenise(source);
+
+        Object[][] expected = {
+                new Object[] { LiquidLexer.TagStart, "{%"},
+                new Object[] { LiquidLexer.WS2, " "},
+                new Object[] { LiquidLexer.CommentInTagId, "# for i in (1..3) "},
+                new Object[] { LiquidLexer.TagEnd, "-%}\n"},
+                new Object[] { LiquidLexer.OutStart, "{{"},
+                new Object[] { LiquidLexer.WS, " "},
+                new Object[] { LiquidLexer.Id, "i"},
+                new Object[] { LiquidLexer.WS, " "},
+                new Object[] { LiquidLexer.OutEnd, "}}"},
+                new Object[] { LiquidLexer.Other, "\n"},
+                new Object[] { LiquidLexer.TagStart, "{%"},
+                new Object[] { LiquidLexer.WS2, " "},
+                new Object[] { LiquidLexer.CommentInTagId, "# endfor "},
+                new Object[] { LiquidLexer.TagEnd, "%}"}
+        };
+
+        assertThat(tokens.size(), is(expected.length));
+
+        int index = 0;
+
+        for (Object[] test : expected) {
+
+            Token token = tokens.get(index);
+
+            assertThat(token.getType(), is(test[0]));
+            assertThat(token.getText(), is(test[1]));
+
+            index++;
+        }
+    }
+
     private static Token singleToken(String source) {
         return singleToken(source, false, true);
     }