[Sass] Better error messages for recursive mixins.

Note that this completely disallows recursive mixins,
something that I doubt will actually affect anyone.

doc-src/SASS_CHANGELOG.md

@@ -31,6 +31,26 @@ is compiled to:
 This is useful, since normally {file:SASS_REFERENCE.md#division-and-slash
 a slash with variables is treated as division}.
 
+### Recursive Mixins
+
+Mixins that include themselves will now print
+much more informative error messages.
+For example:
+
+    @mixin foo {@include bar}
+    @mixin bar {@include foo}
+    @include foo
+
+will print:
+
+    An @include loop has been found:
+        foo includes bar
+        bar includes foo
+
+Although it was previously possible to use recursive mixins
+without causing infinite looping, this is now disallowed,
+since there's no good reason to do it.
+
 ### Rails 3 Support
 
 Fix Sass configuration under Rails 3.

lib/sass/environment.rb

@@ -1,3 +1,5 @@
+require 'set'
+
 module Sass
   # The lexical environment for SassScript.
   # This keeps track of variable and mixin definitions.
@@ -24,6 +26,7 @@ module Sass
       @mixins = {}
       @parent = parent
       @stack = [] unless parent
+      @mixins_in_use = Set.new unless parent
       set_var("important", Script::String.new("!important")) unless @parent
     end
 
@@ -56,6 +59,7 @@ module Sass
       else
         stack.push(frame_info)
       end
+      mixins_in_use << stack.last[:mixin] if stack.last[:mixin] && !stack.last[:prepared]
     end
 
     # Like \{#push\_frame}, but next time a stack frame is pushed,
@@ -69,7 +73,8 @@ module Sass
     # Pop a stack frame from the mixin/include stack.
     def pop_frame
       stack.pop if stack.last && stack.last[:prepared]
-      stack.pop
+      popped = stack.pop
+      mixins_in_use.delete(popped[:mixin]) if popped && popped[:mixin]
     end
 
     # A list of stack frames in the mixin/include stack.
@@ -81,6 +86,13 @@ module Sass
       @stack ||= @parent.stack
     end
 
+    # A set of names of mixins currently present in the stack.
+    #
+    # @return [Set<String>] The mixin names.
+    def mixins_in_use
+      @mixins_in_use ||= @parent.mixins_in_use
+    end
+
     class << self
       private
 

lib/sass/error.rb

@@ -137,7 +137,10 @@ module Sass
     # @see #sass_backtrace
     # @return [String]
     def sass_backtrace_str(default_filename = "an unknown file")
-      "Syntax error: #{message}" +
+      lines = self.message.split("\n")
+      msg = lines[0] + lines[1..-1].
+        map {|l| "\n" + (" " * "Syntax error: ".size) + l}.join
+      "Syntax error: #{msg}" +
         Haml::Util.enum_with_index(sass_backtrace).map do |entry, i|
         "\n        #{i == 0 ? "on" : "from"} line #{entry[:line]}" +
           " of #{entry[:filename] || default_filename}" +

lib/sass/tree/mixin_node.rb

@@ -66,6 +66,8 @@ module Sass::Tree
     # @raise [Sass::SyntaxError] if an incorrect number of arguments was passed
     # @see Sass::Tree
     def perform!(environment)
+      handle_include_loop!(environment) if environment.mixins_in_use.include?(@name)
+
       original_env = environment
       original_env.push_frame(:filename => filename, :line => line)
       original_env.prepare_frame(:mixin => @name)
@@ -93,11 +95,29 @@ END
 
       self.children = mixin.tree.map {|c| c.perform(environment)}.flatten
     rescue Sass::SyntaxError => e
-      e.modify_backtrace(:mixin => @name, :line => @line)
-      e.add_backtrace(:line => @line)
+      if original_env # Don't add backtrace info if this is an @include loop
+        e.modify_backtrace(:mixin => @name, :line => @line)
+        e.add_backtrace(:line => @line)
+      end
       raise e
     ensure
-      original_env.pop_frame
+      original_env.pop_frame if original_env
+    end
+
+    private
+
+    def handle_include_loop!(environment)
+      msg = "An @include loop has been found:"
+      mixins = environment.stack.map {|s| s[:mixin]}.compact
+      if mixins.size == 2 && mixins[0] == mixins[1]
+        raise Sass::SyntaxError.new("#{msg} #{@name} includes itself")
+      end
+
+      mixins << @name
+      msg << "\n" << Haml::Util.enum_cons(mixins, 2).map do |m1, m2|
+        "    #{m1} includes #{m2}"
+      end.join("\n")
+      raise Sass::SyntaxError.new(msg)
     end
   end
 end

test/sass/engine_test.rb

@@ -342,6 +342,63 @@ SASS
     assert_hash_has(err.sass_backtrace[4], :filename => nil, :mixin => nil, :line => 1)
   end
 
+  def test_basic_mixin_loop_exception
+    render <<SASS
+@mixin foo
+  @include foo
+@include foo
+SASS
+    assert(false, "Exception not raised")
+  rescue Sass::SyntaxError => err
+    assert_equal("An @include loop has been found: foo includes itself", err.message)
+    assert_hash_has(err.sass_backtrace[0], :mixin => "foo", :line => 2)
+  end
+
+  def test_double_mixin_loop_exception
+    render <<SASS
+@mixin foo
+  @include bar
+@mixin bar
+  @include foo
+@include foo
+SASS
+    assert(false, "Exception not raised")
+  rescue Sass::SyntaxError => err
+    assert_equal(<<MESSAGE.rstrip, err.message)
+An @include loop has been found:
+    foo includes bar
+    bar includes foo
+MESSAGE
+    assert_hash_has(err.sass_backtrace[0], :mixin => "bar", :line => 4)
+    assert_hash_has(err.sass_backtrace[1], :mixin => "foo", :line => 2)
+  end
+
+  def test_deep_mixin_loop_exception
+    render <<SASS
+@mixin foo
+  @include bar
+
+@mixin bar
+  @include baz
+
+@mixin baz
+  @include foo
+
+@include foo
+SASS
+    assert(false, "Exception not raised")
+  rescue Sass::SyntaxError => err
+    assert_equal(<<MESSAGE.rstrip, err.message)
+An @include loop has been found:
+    foo includes bar
+    bar includes baz
+    baz includes foo
+MESSAGE
+    assert_hash_has(err.sass_backtrace[0], :mixin => "baz", :line => 8)
+    assert_hash_has(err.sass_backtrace[1], :mixin => "bar", :line => 5)
+    assert_hash_has(err.sass_backtrace[2], :mixin => "foo", :line => 2)
+  end
+
   def test_exception_css_with_offset
     opts = {:full_exception => true, :line => 362}
     render(("a\n  b: c\n" * 10) + "d\n  e:\n" + ("f\n  g: h\n" * 10), opts)