Update {% doc %} to no longer support nested tags (as {% comment %} does)

lib/liquid/locales/en.yml

@@ -8,6 +8,7 @@
       case_invalid_when: "Syntax Error in tag 'case' - Valid when condition: {% when [condition] [or condition2...] %}"
       case_invalid_else: "Syntax Error in tag 'case' - Valid else condition: {% else %} (no parameters) "
       cycle: "Syntax Error in 'cycle' - Valid syntax: cycle [name :] var [, var2, var3 ...]"
+      doc_invalid_nested: "Syntax Error in 'doc' - Nested doc tags are not allowed"
       for: "Syntax Error in 'for loop' - Valid syntax: for [item] in [collection]"
       for_invalid_in: "For loops require an 'in' clause"
       for_invalid_attribute: "Invalid attribute in for loop. Valid attributes are limit and offset"

lib/liquid/tags/comment.rb

@@ -15,8 +15,6 @@ module Liquid
   #   {% endcomment %}
   # @liquid_syntax_keyword content The content of the comment.
   class Comment < Block
-    TAG_NAME = "comment"
-
     def render_to_output_buffer(_context, output)
       output
     end
@@ -36,10 +34,7 @@ module Liquid
       end
 
       parse_context.depth += 1
-      tag_depth = 1
-
-      begin_tag = self.class::TAG_NAME
-      end_tag = "end#{self.class::TAG_NAME}"
+      comment_tag_depth = 1
 
       begin
         # Consume tokens without creating child nodes.
@@ -62,13 +57,13 @@ module Liquid
           case tag_name
           when "raw"
             parse_raw_tag_body(tokenizer)
-          when begin_tag
-            tag_depth += 1
-          when end_tag
-            tag_depth -= 1
+          when "comment"
+            comment_tag_depth += 1
+          when "endcomment"
+            comment_tag_depth -= 1
           end
 
-          if tag_depth.zero?
+          if comment_tag_depth.zero?
             parse_context.trim_whitespace = (token[-3] == WhitespaceControl) unless tokenizer.for_liquid_tag
             return false
           end

lib/liquid/tags/doc.rb

@@ -24,7 +24,48 @@ module Liquid
   #     {% render 'message', foo: 'Hello', bar: 'World' %}
   #   {% enddoc %}
   #   {{ foo }}, {{ bar }}!
-  class Doc < Comment
-    TAG_NAME = "doc"
+  class Doc < Block
+    def render_to_output_buffer(_context, output)
+      output
+    end
+
+    def unknown_tag(_tag, _markup, _tokens)
+    end
+
+    def blank?
+      true
+    end
+
+    def parse_body(body, tokenizer)
+      while (token = tokenizer.send(:shift))
+        tag_name = if tokenizer.for_liquid_tag
+          next if token.empty? || token.match?(BlockBody::WhitespaceOrNothing)
+
+          tag_name_match = BlockBody::LiquidTagToken.match(token)
+
+          next if tag_name_match.nil?
+
+          tag_name_match[1]
+        else
+          token =~ BlockBody::FullToken
+          Regexp.last_match(2)
+        end
+
+        raise_nested_doc_error if tag_name == "doc"
+
+        if tag_name == "enddoc"
+          parse_context.trim_whitespace = (token[-3] == WhitespaceControl) unless tokenizer.for_liquid_tag
+          return false
+        end
+      end
+
+      raise_tag_never_closed(block_name)
+    end
+
+    private
+
+    def raise_nested_doc_error
+      raise SyntaxError, parse_context.locale.t("errors.syntax.doc_invalid_nested")
+    end
   end
 end

test/unit/tags/doc_tag_unit_test.rb

@@ -3,8 +3,38 @@
 require 'test_helper'
 
 class DocTagUnitTest < Minitest::Test
-  def test_doc_inside_liquid_tag
-    assert_template_result('', <<~LIQUID.chomp)
+  def test_doc_tag
+    template = <<~LIQUID.chomp
+      {% doc %}
+        Renders loading-spinner.
+
+        @param {string} foo - some foo
+        @param {string} [bar] - optional bar
+
+        @example
+        {% render 'loading-spinner', foo: 'foo' %}
+        {% render 'loading-spinner', foo: 'foo', bar: 'bar' %}
+      {% enddoc %}
+    LIQUID
+
+    assert_template_result('', template)
+  end
+
+  def test_doc_tag_inside_liquid_tag
+    template = <<~LIQUID.chomp
+      {% liquid
+        doc
+          Assigns foo to 1.
+        enddoc
+        assign foo = 1
+      %}
+    LIQUID
+
+    assert_template_result('', template)
+  end
+
+  def test_doc_tag_inside_liquid_tag_with_control_flow_nodes
+    template = <<~LIQUID.chomp
       {% liquid
         if 1 != 1
           doc
@@ -14,10 +44,12 @@ class DocTagUnitTest < Minitest::Test
         endif
       %}
     LIQUID
+
+    assert_template_result('', template)
   end
 
-  def test_does_not_parse_nodes_inside_a_doc
-    assert_template_result('', <<~LIQUID.chomp)
+  def test_doc_tag_ignores_liquid_nodes
+    template = <<~LIQUID.chomp
       {% doc %}
         {% if true %}
         {% if ... %}
@@ -29,89 +61,92 @@ class DocTagUnitTest < Minitest::Test
         {% endcase %}
       {% enddoc %}
     LIQUID
+
+    assert_template_result('', template)
   end
 
-  def test_allows_unclosed_tags
-    assert_template_result('', <<~LIQUID.chomp)
+  def test_doc_tag_ignores_unclosed_liquid_tags
+    template = <<~LIQUID.chomp
       {% doc %}
         {% if true %}
       {% enddoc %}
     LIQUID
+
+    assert_template_result('', template)
   end
 
-  def test_open_tags_in_doc
-    assert_template_result('', <<~LIQUID.chomp)
+  def test_doc_tag_ignores_nested_doc_tags
+    template = <<~LIQUID.chomp
       {% doc %}
         {% assign a = 123 {% doc %}
       {% enddoc %}
     LIQUID
 
-    assert_raises(Liquid::SyntaxError) do
-      assert_template_result('', <<~LIQUID.chomp)
+    assert_template_result('', template)
+  end
+
+  def test_doc_tag_does_not_allow_nested_docs
+    error = assert_raises(Liquid::SyntaxError) do
+      template = <<~LIQUID.chomp
+        {% doc %}
+          {% doc %}
+            {% doc %}
+        {% enddoc %}
+      LIQUID
+
+      Liquid::Template.parse(template)
+    end
+
+    exp_error = "Liquid syntax error: Syntax Error in 'doc' - Nested doc tags are not allowed"
+    act_error = error.message
+
+    assert_equal(exp_error, act_error)
+  end
+
+  def test_doc_tag_ignores_nested_raw_tags
+    template = <<~LIQUID.chomp
+      {% doc %}
+        {% raw %}
+      {% enddoc %}
+    LIQUID
+
+    assert_template_result('', template)
+  end
+
+  def test_doc_tag_raises_an_error_for_unclosed_assign
+    error = assert_raises(Liquid::SyntaxError) do
+      template = <<~LIQUID.chomp
         {% doc %}
           {% assign foo = "1"
         {% enddoc %}
       LIQUID
+
+      Liquid::Template.parse(template)
     end
 
-    assert_raises(Liquid::SyntaxError) do
-      assert_template_result('', <<~LIQUID.chomp)
-        {% doc %}
-          {% doc %}
-            {% invalid
-          {% enddoc %}
-        {% enddoc %}
-      LIQUID
-    end
+    exp_error = "Liquid syntax error: 'doc' tag was never closed"
+    act_error = error.message
 
-    assert_raises(Liquid::SyntaxError) do
-      assert_template_result('', <<~LIQUID.chomp)
+    assert_equal(exp_error, act_error)
+  end
+
+  def test_doc_tag_raises_an_error_for_malformed_syntax
+    error = assert_raises(Liquid::SyntaxError) do
+      template = <<~LIQUID.chomp
         {% doc %}
         {% {{ {%- enddoc %}
       LIQUID
+
+      Liquid::Template.parse(template)
     end
+
+    exp_error = "Liquid syntax error: 'doc' tag was never closed"
+    act_error = error.message
+
+    assert_equal(exp_error, act_error)
   end
 
-  def test_child_doc_tags_need_to_be_closed
-    assert_template_result('', <<~LIQUID.chomp)
-      {% doc %}
-        {% doc %}
-          {% doc %}{% enddoc %}
-        {% enddoc %}
-      {% enddoc %}
-    LIQUID
-
-    assert_raises(Liquid::SyntaxError) do
-      assert_template_result('', <<~LIQUID.chomp)
-        {% doc %}
-          {% doc %}
-            {% doc %}
-          {% enddoc %}
-        {% enddoc %}
-      LIQUID
-    end
-  end
-
-  def test_child_raw_tags_need_to_be_closed
-    assert_template_result('', <<~LIQUID.chomp)
-      {% doc %}
-        {% raw %}
-          {% enddoc %}
-        {% endraw %}
-      {% enddoc %}
-    LIQUID
-
-    assert_raises(Liquid::SyntaxError) do
-      Liquid::Template.parse(<<~LIQUID.chomp)
-        {% doc %}
-          {% raw %}
-          {% enddoc %}
-        {% enddoc %}
-      LIQUID
-    end
-  end
-
-  def test_error_line_number_is_correct
+  def test_doc_tag_preserves_error_line_numbers
     template = Liquid::Template.parse(<<~LIQUID.chomp, line_numbers: true)
       {% doc %}
         {% if true %}
@@ -119,73 +154,21 @@ class DocTagUnitTest < Minitest::Test
       {{ errors.standard_error }}
     LIQUID
 
-    output = template.render('errors' => ErrorDrop.new)
     expected = <<~TEXT.chomp
 
       Liquid error (line 4): standard error
     TEXT
 
-    assert_equal(expected, output)
+    assert_equal(expected, template.render('errors' => ErrorDrop.new))
   end
 
-  def test_doc_tag_delimiter_with_extra_strings
-    assert_template_result(
-      '',
-      <<~LIQUID.chomp,
-        {% doc %}
-          {% doc %}
-          {% enddoc
-          {% if true %}
-          {% endif %}
-        {% enddoc %}
-      LIQUID
-    )
-  end
-
-  def test_nested_doc_tag_with_extra_strings
-    assert_template_result(
-      '',
-      <<~LIQUID.chomp,
-        {% doc %}
-          {% doc
-            {% assign foo = 1 %}
-          {% enddoc
-          {% assign foo = 1 %}
-        {% enddoc %}
-      LIQUID
-    )
-  end
-
-  def test_ignores_delimiter_with_extra_strings
-    assert_template_result('', <<~LIQUID.chomp)
-      {% if true %}
-        {% doc %}
-          {% docEXTRA %}wut{% enddocEXTRA %}xyz
-        {% enddoc %}
-      {% endif %}
-    LIQUID
-  end
-
-  def test_delimiter_can_have_extra_strings
-    assert_template_result('', "{% doc %}123{% enddoc xyz %}")
-    assert_template_result('', "{% doc %}123{% enddoc\txyz %}")
-    assert_template_result('', "{% doc %}123{% enddoc\nxyz %}")
-    assert_template_result('', "{% doc %}123{% enddoc\n   xyz  enddoc %}")
-    assert_template_result('', "{%doc}{% assign a = 1 %}{%enddoc}{% endif %}")
-  end
-
-  def test_with_whitespace_control
+  def test_doc_tag_whitespace_control
+    # Basic whitespace control
     assert_template_result("Hello!", "      {%- doc -%}123{%- enddoc -%}Hello!")
     assert_template_result("Hello!", "{%- doc -%}123{%- enddoc -%}     Hello!")
     assert_template_result("Hello!", "      {%- doc -%}123{%- enddoc -%}     Hello!")
 
-    assert_template_result("Hello!", <<~LIQUID.chomp)
-      {%- doc %}Whitespace control!{% enddoc -%}
-      Hello!
-    LIQUID
-  end
-
-  def test_dont_override_liquid_tag_whitespace_control
+    # Whitespace control within liquid tags
     assert_template_result("Hello!World!", <<~LIQUID.chomp)
       Hello!
       {%- liquid
@@ -195,5 +178,27 @@ class DocTagUnitTest < Minitest::Test
       -%}
       World!
     LIQUID
+
+    # Multiline whitespace control
+    assert_template_result("Hello!", <<~LIQUID.chomp)
+      {%- doc %}Whitespace control!{% enddoc -%}
+      Hello!
+    LIQUID
+  end
+
+  def test_doc_tag_delimiter_handling
+    assert_template_result('', <<~LIQUID.chomp)
+      {% if true %}
+        {% doc %}
+          {% docEXTRA %}wut{% enddocEXTRA %}xyz
+        {% enddoc %}
+      {% endif %}
+    LIQUID
+
+    assert_template_result('', "{% doc %}123{% enddoc xyz %}")
+    assert_template_result('', "{% doc %}123{% enddoc\txyz %}")
+    assert_template_result('', "{% doc %}123{% enddoc\nxyz %}")
+    assert_template_result('', "{% doc %}123{% enddoc\n   xyz  enddoc %}")
+    assert_template_result('', "{%doc}{% assign a = 1 %}{%enddoc}{% endif %}")
   end
 end