From ea9addc3857324b940e1096bcbfa42e339baa09f Mon Sep 17 00:00:00 2001
From: Nathan Weizenbaum <nex342@gmail.com>
Date: Wed, 15 Oct 2008 20:00:28 -0700
Subject: [PATCH] Scope variables at indentation level.

---
 lib/sass/engine.rb              | 33 +++++++++++----------------------
 lib/sass/environment.rb         | 33 +++++++++++++++++++++++++++++++++
 lib/sass/script/variable.rb     |  2 +-
 lib/sass/tree/directive_node.rb |  3 ---
 lib/sass/tree/for_node.rb       |  7 +++++--
 lib/sass/tree/if_node.rb        |  1 +
 lib/sass/tree/mixin_def_node.rb | 18 ++++++++++++++++++
 lib/sass/tree/mixin_node.rb     | 32 ++++++++++++++++++++------------
 lib/sass/tree/node.rb           |  2 +-
 lib/sass/tree/variable_node.rb  |  8 ++++----
 lib/sass/tree/while_node.rb     |  2 +-
 test/sass/engine_test.rb        | 27 +++++++++++++++++++++++++++
 12 files changed, 122 insertions(+), 46 deletions(-)
 create mode 100644 lib/sass/environment.rb
 create mode 100644 lib/sass/tree/mixin_def_node.rb

diff --git a/lib/sass/engine.rb b/lib/sass/engine.rb
index 519e335e..e1c466a0 100644
--- a/lib/sass/engine.rb
+++ b/lib/sass/engine.rb
@@ -1,21 +1,26 @@
 require 'enumerator'
 require 'strscan'
 require 'sass/tree/node'
-require 'sass/tree/value_node'
 require 'sass/tree/rule_node'
 require 'sass/tree/comment_node'
 require 'sass/tree/attr_node'
 require 'sass/tree/directive_node'
 require 'sass/tree/variable_node'
+require 'sass/tree/mixin_def_node'
 require 'sass/tree/mixin_node'
 require 'sass/tree/if_node'
 require 'sass/tree/while_node'
 require 'sass/tree/for_node'
+require 'sass/environment'
 require 'sass/script'
 require 'sass/error'
 require 'haml/shared'
 
 module Sass
+  # :stopdoc:
+  Mixin = Struct.new(:name, :args, :environment, :tree)
+  # :startdoc:
+
   # This is the class where all the parsing and processing of the Sass
   # template is done. It can be directly used by the user by creating a
   # new instance and calling <tt>render</tt> to render the template. For example:
@@ -26,7 +31,6 @@ module Sass
   #   puts output
   class Engine
     Line = Struct.new(:text, :tabs, :index, :filename, :children)
-    Mixin = Struct.new(:name, :args, :tree)
 
     # The character that begins a CSS attribute.
     ATTRIBUTE_CHAR  = ?:
@@ -88,8 +92,8 @@ module Sass
         :load_paths => ['.']
       }.merge! options
       @template = template
-      @environment = {"important" => Script::String.new("!important")}
-      @mixins = {}
+      @environment = Environment.new
+      @environment.set_var("important", Script::String.new("!important"))
     end
 
     # Processes the template and returns the result as a string.
@@ -113,10 +117,6 @@ module Sass
       @environment
     end
 
-    def mixins
-      @mixins
-    end
-
     def render_to_tree
       root = Tree::Node.new(@options)
       append_children(root, tree(tabulate(@template)).first, true)
@@ -224,7 +224,7 @@ END
     def validate_and_append_child(parent, child, line, root)
       unless root
         case child
-        when :mixin
+        when Tree::MixinDefNode
           raise SyntaxError.new("Mixins may only be defined at the root of a document.", line.index)
         when Tree::DirectiveNode
           raise SyntaxError.new("Import directives may only be used at the root of a document.", line.index)
@@ -391,9 +391,7 @@ END
         default = Script.parse(default, @line) if default
         { :name => arg[1..-1], :default_value => default }
       end
-      mixin = @mixins[name] = Mixin.new(name, args, [])
-      append_children(mixin.tree, line.children, false)
-      :mixin
+      Tree::MixinDefNode.new(name, args, @options)
     end
 
     def parse_mixin_include(line, root)
@@ -401,15 +399,9 @@ END
       args = parse_mixin_arguments(arg_string)
       raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath mixin directives.", @line + 1) unless line.children.empty?
       raise SyntaxError.new("Invalid mixin include \"#{line.text}\".", @line) if name.nil? || args.nil?
-      raise SyntaxError.new("Undefined mixin '#{name}'.", @line) unless mixin = @mixins[name]
-
       args.each {|a| raise SyntaxError.new("Mixin arguments can't be empty.", @line) if a.empty?}
-      raise SyntaxError.new(<<END.gsub("\n", "")) if mixin.args.size < args.size
-Mixin #{name} takes #{mixin.args.size} argument#{'s' if mixin.args.size != 1}
- but #{args.size} #{args.size == 1 ? 'was' : 'were'} passed.
-END
 
-      Tree::MixinNode.new(mixin, args.map {|s| Script.parse(s, @line)}, @options)
+      Tree::MixinNode.new(name, args.map {|s| Script.parse(s, @line)}, @options)
     end
 
     def import_paths
@@ -446,8 +438,6 @@ END
             engine = Sass::Engine.new(file.read, new_options)
           end
 
-          engine.mixins.merge! @mixins
-
           begin
             root = engine.render_to_tree
           rescue Sass::SyntaxError => err
@@ -455,7 +445,6 @@ END
             raise err
           end
           nodes += root.children
-          @mixins = engine.mixins
         end
       end
 
diff --git a/lib/sass/environment.rb b/lib/sass/environment.rb
new file mode 100644
index 00000000..6e46711e
--- /dev/null
+++ b/lib/sass/environment.rb
@@ -0,0 +1,33 @@
+module Sass
+  class Environment
+    attr_reader :parent
+
+    def initialize(parent = nil)
+      @vars = {}
+      @mixins = {}
+      @parent = parent
+    end
+
+    def self.inherited_hash(name)
+      class_eval <<RUBY, __FILE__, __LINE__ + 1
+        def #{name}(name)
+          @#{name}s[name] || @parent && @parent.#{name}(name)
+        end
+
+        def set_#{name}(name, value)
+          if @parent && @parent.#{name}(name)
+            @parent.set_#{name}(name, value)
+          else
+            @#{name}s[name] = value
+          end
+        end
+
+        def set_local_#{name}(name, value)
+          @#{name}s[name] = value
+        end
+RUBY
+    end
+    inherited_hash :var
+    inherited_hash :mixin
+  end
+end
diff --git a/lib/sass/script/variable.rb b/lib/sass/script/variable.rb
index 33e157c0..718742cf 100644
--- a/lib/sass/script/variable.rb
+++ b/lib/sass/script/variable.rb
@@ -12,7 +12,7 @@ module Sass
       end
 
       def perform(environment)
-        (val = environment[name]) && (return val)
+        (val = environment.var(name)) && (return val)
         raise SyntaxError.new("Undefined variable: \"!#{name}\".")
       end
     end
diff --git a/lib/sass/tree/directive_node.rb b/lib/sass/tree/directive_node.rb
index 4d0d3aca..002b0e74 100644
--- a/lib/sass/tree/directive_node.rb
+++ b/lib/sass/tree/directive_node.rb
@@ -1,6 +1,3 @@
-require 'sass/tree/node'
-require 'sass/tree/value_node'
-
 module Sass::Tree
   class DirectiveNode < Node
     attr_accessor :value
diff --git a/lib/sass/tree/for_node.rb b/lib/sass/tree/for_node.rb
index e9f2feb9..09dd1df2 100644
--- a/lib/sass/tree/for_node.rb
+++ b/lib/sass/tree/for_node.rb
@@ -18,8 +18,11 @@ module Sass::Tree
       range = Range.new(from, to, @exclusive)
 
       children = []
-      sub_env = environment.dup
-      range.each {|i| children += perform_children(sub_env.merge(@var => Sass::Script::Number.new(i)))}
+      range.each do |i|
+        environment = Sass::Environment.new(environment)
+        environment.set_local_var(@var, Sass::Script::Number.new(i))
+        children += perform_children(environment)
+      end
       children
     end
   end
diff --git a/lib/sass/tree/if_node.rb b/lib/sass/tree/if_node.rb
index c070e987..2ba98503 100644
--- a/lib/sass/tree/if_node.rb
+++ b/lib/sass/tree/if_node.rb
@@ -18,6 +18,7 @@ module Sass::Tree
     protected
 
     def _perform(environment)
+      environment = Sass::Environment.new(environment)
       return perform_children(environment) if @expr.nil? || @expr.perform(environment).to_bool
       return @else.perform(environment) if @else
       []
diff --git a/lib/sass/tree/mixin_def_node.rb b/lib/sass/tree/mixin_def_node.rb
new file mode 100644
index 00000000..473d209e
--- /dev/null
+++ b/lib/sass/tree/mixin_def_node.rb
@@ -0,0 +1,18 @@
+module Sass
+  module Tree
+    class MixinDefNode < Node
+      def initialize(name, args, options)
+        @name = name
+        @args = args
+        super(options)
+      end
+
+      private
+
+      def _perform(environment)
+        environment.set_mixin(@name, Sass::Mixin.new(@name, @args, environment, children))
+        []
+      end
+    end
+  end
+end
diff --git a/lib/sass/tree/mixin_node.rb b/lib/sass/tree/mixin_node.rb
index 9d181c7f..921183fa 100644
--- a/lib/sass/tree/mixin_node.rb
+++ b/lib/sass/tree/mixin_node.rb
@@ -2,25 +2,33 @@ require 'sass/tree/node'
 
 module Sass::Tree
   class MixinNode < Node
-    def initialize(mixin, args, options)
-      @mixin = mixin
+    def initialize(name, args, options)
+      @name = name
       @args = args
       super(options)
-      self.children = @mixin.tree
     end
 
     protected
 
     def _perform(environment)
-      perform_children(@mixin.args.zip(@args).inject(environment.dup) do |env, (arg, value)|
-          env[arg[:name]] = if value
-                              value.perform(environment)
-                            elsif arg[:default_value]
-                              arg[:default_value].perform(env)
-                            end
-          raise Sass::SyntaxError.new("Mixin #{@mixin.name} is missing parameter !#{arg[:name]}.") unless env[arg[:name]]
-          env
-        end)
+      raise Sass::SyntaxError.new("Undefined mixin '#{@name}'.", @line) unless mixin = environment.mixin(@name)
+
+      raise Sass::SyntaxError.new(<<END.gsub("\n", "")) if mixin.args.size < @args.size
+Mixin #{@name} takes #{mixin.args.size} argument#{'s' if mixin.args.size != 1}
+ but #{@args.size} #{@args.size == 1 ? 'was' : 'were'} passed.
+END
+
+      environment = mixin.args.zip(@args).inject(Sass::Environment.new(mixin.environment)) do |env, (arg, value)|
+        env.set_local_var(arg[:name],
+          if value
+            value.perform(environment)
+          elsif arg[:default_value]
+            arg[:default_value].perform(env)
+          end)
+        raise Sass::SyntaxError.new("Mixin #{@name} is missing parameter !#{arg[:name]}.") unless env.var(arg[:name])
+        env
+      end
+      mixin.tree.map {|c| c.perform(environment)}.flatten
     end
   end
 end
diff --git a/lib/sass/tree/node.rb b/lib/sass/tree/node.rb
index 0b59107b..26c02081 100644
--- a/lib/sass/tree/node.rb
+++ b/lib/sass/tree/node.rb
@@ -51,7 +51,7 @@ module Sass
       end
 
       def perform!(environment)
-        self.children = perform_children(environment)
+        self.children = perform_children(Environment.new(environment))
       end
 
       def perform_children(environment)
diff --git a/lib/sass/tree/variable_node.rb b/lib/sass/tree/variable_node.rb
index bda6ca2b..b425f8fd 100644
--- a/lib/sass/tree/variable_node.rb
+++ b/lib/sass/tree/variable_node.rb
@@ -11,10 +11,10 @@ module Sass
       protected
 
       def _perform(environment)
-        if @guarded
-          environment[@name] ||= @expr.perform(environment)
-        else
-          environment[@name] = @expr.perform(environment)
+        if @guarded && environment.var(@name).nil?
+          environment.set_var(@name, @expr.perform(environment))
+        elsif !@guarded
+          environment.set_var(@name, @expr.perform(environment))
         end
 
         []
diff --git a/lib/sass/tree/while_node.rb b/lib/sass/tree/while_node.rb
index a22b1df8..a43af4a7 100644
--- a/lib/sass/tree/while_node.rb
+++ b/lib/sass/tree/while_node.rb
@@ -12,7 +12,7 @@ module Sass::Tree
     def _perform(environment)
       children = []
       while @expr.perform(environment).to_bool
-        children += perform_children(environment)
+        children += perform_children(Sass::Environment.new(environment))
       end
       children
     end
diff --git a/test/sass/engine_test.rb b/test/sass/engine_test.rb
index 41bab5a6..e98dbaff 100755
--- a/test/sass/engine_test.rb
+++ b/test/sass/engine_test.rb
@@ -76,6 +76,8 @@ class SassEngineTest < Test::Unit::TestCase
     "@else\n  a\n    b: c" => ["@else must come after @if.", 1],
     "@if false\n@else foo" => "Invalid @else directive '@else foo': expected 'if <expr>'.",
     "@if false\n@else if " => "Invalid @else directive '@else if': expected 'if <expr>'.",
+    "a\n  !b = 12\nc\n  d = !b" => 'Undefined variable: "!b".',
+    "=foo\n  !b = 12\nc\n  +foo\n  d = !b" => 'Undefined variable: "!b".',
 
     # Regression tests
     "a\n  b:\n    c\n    d" => ["Illegal nesting: Only attributes may be nested beneath attributes.", 3],
@@ -672,6 +674,31 @@ a
 SASS
   end
 
+  def test_variable_scope
+    assert_equal(<<CSS, render(<<SASS))
+a {
+  b-1: c;
+  b-2: c;
+  d: 12; }
+
+b {
+  d: 17; }
+CSS
+!i = 12
+a
+  @for !i from 1 through 2
+    b-\#{!i}: c
+  d = !i
+
+=foo
+  !i = 17
+
+b
+  +foo
+  d = !i
+SASS
+  end
+
   def test_argument_error
     assert_raise(Sass::SyntaxError) { render("a\n  b = hsl(1)") }
   end