From d4ecaff8b839e7629090e238dd6bd5bff6c9e74c Mon Sep 17 00:00:00 2001
From: Dylan Thacker-Smith <Dylan.Smith@shopify.com>
Date: Tue, 18 Mar 2014 17:38:43 -0400
Subject: [PATCH] Refactor to create tags with a parse class method instead of
 new.

By moving parse out of the initializer, we can call super at the start of
the initializers for subclasses, and avoid the nasty allocate hack.
---
 History.md                            |  1 +
 lib/liquid/block.rb                   |  2 +-
 lib/liquid/document.rb                |  7 +++----
 lib/liquid/tag.rb                     | 20 ++++++++++----------
 lib/liquid/tags/assign.rb             |  5 ++---
 lib/liquid/tags/capture.rb            |  5 ++---
 lib/liquid/tags/case.rb               |  5 ++---
 lib/liquid/tags/cycle.rb              |  4 ++--
 lib/liquid/tags/decrement.rb          |  5 ++---
 lib/liquid/tags/for.rb                |  4 ++--
 lib/liquid/tags/if.rb                 |  4 ++--
 lib/liquid/tags/include.rb            |  6 +++---
 lib/liquid/tags/increment.rb          |  4 ++--
 lib/liquid/tags/table_row.rb          |  5 ++---
 lib/liquid/template.rb                |  2 +-
 performance/shopify/comment_form.rb   |  6 +++---
 performance/shopify/paginate.rb       |  6 +++---
 test/liquid/tags/standard_tag_test.rb |  2 +-
 18 files changed, 44 insertions(+), 49 deletions(-)

diff --git a/History.md b/History.md
index 200b5eb..d99f7cb 100644
--- a/History.md
+++ b/History.md
@@ -3,6 +3,7 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
+* Tag#parse is called after initialize, which now takes options instead of tokens as the 3rd argument. See #321 [Dylan Thacker-Smith, dylanahsmith]
 * Raise `Liquid::ArgumentError` instead of `::ArgumentError` when filter has wrong number of arguments #309 [Bogdan Gusiev, bogdan]
 * Add a to_s default for liquid drops, see #306 [Adam Doeler, releod]
 * Add strip, lstrip, and rstrip to standard filters [Florian Weingarten, fw42]
diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb
index 2acfe20..bcac5a8 100644
--- a/lib/liquid/block.rb
+++ b/lib/liquid/block.rb
@@ -31,7 +31,7 @@ module Liquid
 
             # fetch the tag from registered blocks
             if tag = Template.tags[$1]
-              new_tag = tag.new_with_options($1, $2, tokens, @options || {})
+              new_tag = tag.parse($1, $2, tokens, @options)
               @blank &&= new_tag.blank?
               @nodelist << new_tag
               @children << new_tag
diff --git a/lib/liquid/document.rb b/lib/liquid/document.rb
index b802c85..1969a3a 100644
--- a/lib/liquid/document.rb
+++ b/lib/liquid/document.rb
@@ -1,9 +1,8 @@
 module Liquid
   class Document < Block
-    # we don't need markup to open this block
-    def initialize(tokens, options = {})
-      @options = options
-      parse(tokens)
+    def self.parse(tokens, options={})
+      # we don't need markup to open this block
+      super(nil, nil, tokens, options)
     end
 
     # There isn't a real delimiter
diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb
index 83dc730..2daf084 100644
--- a/lib/liquid/tag.rb
+++ b/lib/liquid/tag.rb
@@ -3,20 +3,20 @@ module Liquid
     attr_accessor :options
     attr_reader :nodelist, :warnings
 
-    def self.new_with_options(tag_name, markup, tokens, options)
-      # Forgive me Matz for I have sinned. I know this code is weird
-      # but it was necessary to maintain API compatibility.
-      new_tag = self.allocate
-      new_tag.options = options
-      new_tag.send(:initialize, tag_name, markup, tokens)
-      new_tag
+    class << self
+      def parse(tag_name, markup, tokens, options)
+        tag = new(tag_name, markup, options)
+        tag.parse(tokens)
+        tag
+      end
+
+      private :new
     end
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
       @tag_name   = tag_name
       @markup     = markup
-      @options    ||= {} # needs || because might be set before initialize
-      parse(tokens)
+      @options    = options
     end
 
     def parse(tokens)
diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb
index 4699a6a..a3bc86f 100644
--- a/lib/liquid/tags/assign.rb
+++ b/lib/liquid/tags/assign.rb
@@ -11,15 +11,14 @@ module Liquid
   class Assign < Tag
     Syntax = /(#{VariableSignature}+)\s*=\s*(.*)\s*/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       if markup =~ Syntax
         @to = $1
         @from = Variable.new($2)
       else
         raise SyntaxError.new options[:locale].t("errors.syntax.assign")
       end
-
-      super
     end
 
     def render(context)
diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb
index 34036be..9acb22b 100644
--- a/lib/liquid/tags/capture.rb
+++ b/lib/liquid/tags/capture.rb
@@ -14,14 +14,13 @@ module Liquid
   class Capture < Block
     Syntax = /(\w+)/
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       if markup =~ Syntax
         @to = $1
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.capture"))
       end
-
-      super
     end
 
     def render(context)
diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb
index 2b7179f..ed6e806 100644
--- a/lib/liquid/tags/case.rb
+++ b/lib/liquid/tags/case.rb
@@ -3,7 +3,8 @@ module Liquid
     Syntax     = /(#{QuotedFragment})/o
     WhenSyntax = /(#{QuotedFragment})(?:(?:\s+or\s+|\s*\,\s*)(#{QuotedFragment}.*))?/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       @blocks = []
 
       if markup =~ Syntax
@@ -11,8 +12,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.case"))
       end
-
-      super
     end
 
     def nodelist
diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb
index 02bca0b..ed42995 100644
--- a/lib/liquid/tags/cycle.rb
+++ b/lib/liquid/tags/cycle.rb
@@ -15,7 +15,8 @@ module Liquid
     SimpleSyntax = /\A#{QuotedFragment}+/o
     NamedSyntax  = /\A(#{QuotedFragment})\s*\:\s*(.*)/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       case markup
       when NamedSyntax
         @variables = variables_from_string($2)
@@ -26,7 +27,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.cycle"))
       end
-      super
     end
 
     def render(context)
diff --git a/lib/liquid/tags/decrement.rb b/lib/liquid/tags/decrement.rb
index 217d96d..e9b6606 100644
--- a/lib/liquid/tags/decrement.rb
+++ b/lib/liquid/tags/decrement.rb
@@ -19,10 +19,9 @@ module Liquid
   #    Hello: -3
   #
   class Decrement < Tag
-    def initialize(tag_name, markup, tokens)
-      @variable = markup.strip
-
+    def initialize(tag_name, markup, options)
       super
+      @variable = markup.strip
     end
 
     def render(context)
diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb
index 057658c..3cd9e97 100644
--- a/lib/liquid/tags/for.rb
+++ b/lib/liquid/tags/for.rb
@@ -46,10 +46,10 @@ module Liquid
   class For < Block
     Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       parse_with_selected_parser(markup)
       @nodelist = @for_block = []
-      super
     end
 
     def nodelist
diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb
index e6175e8..ad48bb5 100644
--- a/lib/liquid/tags/if.rb
+++ b/lib/liquid/tags/if.rb
@@ -14,10 +14,10 @@ module Liquid
     ExpressionsAndOperators = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o
     BOOLEAN_OPERATORS = %w(and or)
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       @blocks = []
       push_block('if', markup)
-      super
     end
 
     def nodelist
diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb
index d439fff..34b2b0b 100644
--- a/lib/liquid/tags/include.rb
+++ b/lib/liquid/tags/include.rb
@@ -17,7 +17,9 @@ module Liquid
   class Include < Tag
     Syntax = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
+
       if markup =~ Syntax
 
         @template_name = $1
@@ -31,8 +33,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.include"))
       end
-
-      super
     end
 
     def parse(tokens)
diff --git a/lib/liquid/tags/increment.rb b/lib/liquid/tags/increment.rb
index 5540061..251bd77 100644
--- a/lib/liquid/tags/increment.rb
+++ b/lib/liquid/tags/increment.rb
@@ -15,9 +15,9 @@ module Liquid
   #    Hello: 2
   #
   class Increment < Tag
-    def initialize(tag_name, markup, tokens)
-      @variable = markup.strip
+    def initialize(tag_name, markup, options)
       super
+      @variable = markup.strip
     end
 
     def render(context)
diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb
index 8ceab44..771042d 100644
--- a/lib/liquid/tags/table_row.rb
+++ b/lib/liquid/tags/table_row.rb
@@ -2,7 +2,8 @@ module Liquid
   class TableRow < Block
     Syntax = /(\w+)\s+in\s+(#{QuotedFragment}+)/o
 
-    def initialize(tag_name, markup, tokens)
+    def initialize(tag_name, markup, options)
+      super
       if markup =~ Syntax
         @variable_name = $1
         @collection_name = $2
@@ -13,8 +14,6 @@ module Liquid
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.table_row"))
       end
-
-      super
     end
 
     def render(context)
diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb
index 1f2bfd1..4ca7961 100644
--- a/lib/liquid/template.rb
+++ b/lib/liquid/template.rb
@@ -72,7 +72,7 @@ module Liquid
     # Parse source code.
     # Returns self for easy chaining
     def parse(source, options = {})
-      @root = Document.new(tokenize(source), DEFAULT_OPTIONS.merge(options))
+      @root = Document.parse(tokenize(source), DEFAULT_OPTIONS.merge(options))
       @warnings = nil
       self
     end
diff --git a/performance/shopify/comment_form.rb b/performance/shopify/comment_form.rb
index 2a62f66..30226dd 100644
--- a/performance/shopify/comment_form.rb
+++ b/performance/shopify/comment_form.rb
@@ -1,15 +1,15 @@
 class CommentForm < Liquid::Block
   Syntax = /(#{Liquid::VariableSignature}+)/
 
-  def initialize(tag_name, markup, tokens)
+  def initialize(tag_name, markup, options)
+    super
+
     if markup =~ Syntax
       @variable_name = $1
       @attributes = {}
     else
       raise SyntaxError.new("Syntax Error in 'comment_form' - Valid syntax: comment_form [article]")
     end
-
-    super
   end
 
   def render(context)
diff --git a/performance/shopify/paginate.rb b/performance/shopify/paginate.rb
index a5b5c9f..23c92c8 100644
--- a/performance/shopify/paginate.rb
+++ b/performance/shopify/paginate.rb
@@ -1,7 +1,9 @@
 class Paginate < Liquid::Block
   Syntax     = /(#{Liquid::QuotedFragment})\s*(by\s*(\d+))?/
 
-  def initialize(tag_name, markup, tokens)
+  def initialize(tag_name, markup, options)
+    super
+
     @nodelist = []
 
     if markup =~ Syntax
@@ -19,8 +21,6 @@ class Paginate < Liquid::Block
     else
       raise SyntaxError.new("Syntax Error in tag 'paginate' - Valid syntax: paginate [collection] by number")
     end
-
-    super
   end
 
   def render(context)
diff --git a/test/liquid/tags/standard_tag_test.rb b/test/liquid/tags/standard_tag_test.rb
index ddd365f..10cc5e3 100644
--- a/test/liquid/tags/standard_tag_test.rb
+++ b/test/liquid/tags/standard_tag_test.rb
@@ -4,7 +4,7 @@ class StandardTagTest < Test::Unit::TestCase
   include Liquid
 
   def test_tag
-    tag = Tag.new('tag', [], [])
+    tag = Tag.parse('tag', [], [], {})
     assert_equal 'liquid::tag', tag.name
     assert_equal '', tag.render(Context.new)
   end