diff --git a/doc-src/SASS_CHANGELOG.md b/doc-src/SASS_CHANGELOG.md index 4816e0fd..0fd37020 100644 --- a/doc-src/SASS_CHANGELOG.md +++ b/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. diff --git a/lib/sass/environment.rb b/lib/sass/environment.rb index 811a9817..3d1eb3f6 100644 --- a/lib/sass/environment.rb +++ b/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] The mixin names. + def mixins_in_use + @mixins_in_use ||= @parent.mixins_in_use + end + class << self private diff --git a/lib/sass/error.rb b/lib/sass/error.rb index 419ee4cb..9c9b5e4f 100644 --- a/lib/sass/error.rb +++ b/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}" + diff --git a/lib/sass/tree/mixin_node.rb b/lib/sass/tree/mixin_node.rb index 15e1751a..2f6c8494 100644 --- a/lib/sass/tree/mixin_node.rb +++ b/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 diff --git a/test/sass/engine_test.rb b/test/sass/engine_test.rb index a7c1e771..40e63d83 100755 --- a/test/sass/engine_test.rb +++ b/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 < 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 < err + assert_equal(< "bar", :line => 4) + assert_hash_has(err.sass_backtrace[1], :mixin => "foo", :line => 2) + end + + def test_deep_mixin_loop_exception + render < err + assert_equal(< "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)