1
0
Fork 0
mirror of https://github.com/haml/haml.git synced 2022-11-09 12:33:31 -05:00

[Sass] Improve error handling.

Unify and clarify the way Sass errors are handled (see notes in Sass::Error).
This fixes some bugs with filenames and/or line numbers being mis-recorded for imports.
This commit is contained in:
Nathan Weizenbaum 2009-09-13 15:52:59 -07:00
parent 580b6a932b
commit 616a00cea6
10 changed files with 170 additions and 99 deletions

View file

@ -3,6 +3,15 @@
* Table of contents
{:toc}
## 2.4.0
### Error Backtraces
Numerous bugs were fixed with the backtraces given for Sass errors,
especially when importing files.
All imports will now show up in the Ruby backtrace,
with the proper filename and line number.
## [2.2.3](http://github.com/nex3/haml/commit/2.2.3)
Sass 2.2.3 prints line numbers for warnings about selectors

View file

@ -159,7 +159,9 @@ module Sass
append_children(root, tree(tabulate(@template)).first, true)
root.options = @options
root
rescue SyntaxError => e; e.add_metadata(@options[:filename], @line)
rescue SyntaxError => e
e.modify_backtrace(:filename => @options[:filename], :line => @line)
raise e
end
private
@ -179,10 +181,11 @@ module Sass
unless line_tab_str.empty?
tab_str ||= line_tab_str
raise SyntaxError.new("Indenting at the beginning of the document is illegal.", index) if first
if tab_str.include?(?\s) && tab_str.include?(?\t)
raise SyntaxError.new("Indentation can't use both tabs and spaces.", index)
end
raise SyntaxError.new("Indenting at the beginning of the document is illegal.",
:line => index) if first
raise SyntaxError.new("Indentation can't use both tabs and spaces.",
:line => index) if tab_str.include?(?\s) && tab_str.include?(?\t)
end
first &&= !tab_str.nil?
if tab_str.nil?
@ -196,10 +199,14 @@ module Sass
end
line_tabs = line_tab_str.scan(tab_str).size
raise SyntaxError.new(<<END.strip.gsub("\n", ' '), index) if tab_str * line_tabs != line_tab_str
if tab_str * line_tabs != line_tab_str
message = <<END.strip.gsub("\n", ' ')
Inconsistent indentation: #{Haml::Shared.human_indentation line_tab_str, true} used for indentation,
but the rest of the document was indented using #{Haml::Shared.human_indentation tab_str}.
END
raise SyntaxError.new(message, :line => index)
end
lines << Line.new(line.strip, line_tabs, index, tab_str.size, @options[:filename], [])
end
lines
@ -212,9 +219,8 @@ END
nodes = []
while (line = arr[i]) && line.tabs >= base
if line.tabs > base
if line.tabs > base + 1
raise SyntaxError.new("The line was indented #{line.tabs - base} levels deeper than the previous line.", line.index)
end
raise SyntaxError.new("The line was indented #{line.tabs - base} levels deeper than the previous line.",
:line => line.index) if line.tabs > base + 1
nodes.last.children, i = tree(arr, i)
else
@ -252,7 +258,8 @@ END
child = build_tree(parent, line, root)
if child.is_a?(Tree::RuleNode) && child.continued?
raise SyntaxError.new("Rules can't end in commas.", child.line) unless child.children.empty?
raise SyntaxError.new("Rules can't end in commas.",
:line => child.line) unless child.children.empty?
if continued_rule
continued_rule.add_rules child
else
@ -262,7 +269,8 @@ END
end
if continued_rule
raise SyntaxError.new("Rules can't end in commas.", continued_rule.line) unless child.is_a?(Tree::RuleNode)
raise SyntaxError.new("Rules can't end in commas.",
:line => continued_rule.line) unless child.is_a?(Tree::RuleNode)
continued_rule.add_rules child
continued_rule.children = child.children
continued_rule, child = nil, continued_rule
@ -272,7 +280,8 @@ END
validate_and_append_child(parent, child, line, root)
end
raise SyntaxError.new("Rules can't end in commas.", continued_rule.line) if continued_rule
raise SyntaxError.new("Rules can't end in commas.",
:line => continued_rule.line) if continued_rule
parent
end
@ -281,9 +290,11 @@ END
unless root
case child
when Tree::MixinDefNode
raise SyntaxError.new("Mixins may only be defined at the root of a document.", line.index)
raise SyntaxError.new("Mixins may only be defined at the root of a document.",
:line => line.index)
when Tree::ImportNode
raise SyntaxError.new("Import directives may only be used at the root of a document.", line.index)
raise SyntaxError.new("Import directives may only be used at the root of a document.",
:line => line.index)
end
end
@ -349,9 +360,9 @@ LONG
def parse_property(line, property_regx)
name, eq, value = line.text.scan(property_regx)[0]
if name.nil? || value.nil?
raise SyntaxError.new("Invalid property: \"#{line.text}\".", @line)
end
raise SyntaxError.new("Invalid property: \"#{line.text}\".",
:line => @line) if name.nil? || value.nil?
expr = if (eq.strip[0] == SCRIPT_CHAR)
parse_script(value, :offset => line.offset + line.text.index(value))
else
@ -362,8 +373,10 @@ LONG
def parse_variable(line)
name, op, value = line.text.scan(Script::MATCH)[0]
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath variable declarations.", @line + 1) unless line.children.empty?
raise SyntaxError.new("Invalid variable: \"#{line.text}\".", @line) unless name && value
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath variable declarations.",
:line => @line + 1) unless line.children.empty?
raise SyntaxError.new("Invalid variable: \"#{line.text}\".",
:line => @line) unless name && value
Tree::VariableNode.new(name, parse_script(value, :offset => line.offset + line.text.index(value)), op == '||=')
end
@ -383,7 +396,8 @@ LONG
# If value begins with url( or ",
# it's a CSS @import rule and we don't want to touch it.
if directive == "import" && value !~ /^(url\(|")/
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath import directives.", @line + 1) unless line.children.empty?
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath import directives.",
:line => @line + 1) unless line.children.empty?
value.split(/,\s*/).map {|f| Tree::ImportNode.new(f)}
elsif directive == "for"
parse_for(line, root, value)
@ -397,7 +411,8 @@ LONG
Tree::IfNode.new(parse_script(value, :offset => offset))
elsif directive == "debug"
raise SyntaxError.new("Invalid debug directive '@debug': expected expression.") unless value
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath debug directives.", @line + 1) unless line.children.empty?
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath debug directives.",
:line => @line + 1) unless line.children.empty?
offset = line.offset + line.text.index(value).to_i
Tree::DebugNode.new(parse_script(value, :offset => offset))
else
@ -416,9 +431,9 @@ LONG
else
expected = "'to <expr>' or 'through <expr>'"
end
raise SyntaxError.new("Invalid for directive '@for #{text}': expected #{expected}.", @line)
raise SyntaxError.new("Invalid for directive '@for #{text}': expected #{expected}.")
end
raise SyntaxError.new("Invalid variable \"#{var}\".", @line) unless var =~ Script::VALIDATE
raise SyntaxError.new("Invalid variable \"#{var}\".") unless var =~ Script::VALIDATE
parsed_from = parse_script(from_expr, :offset => line.offset + line.text.index(from_expr))
parsed_to = parse_script(to_expr, :offset => line.offset + line.text.index(to_expr))
@ -431,7 +446,7 @@ LONG
if text
if text !~ /^if\s+(.+)/
raise SyntaxError.new("Invalid else directive '@else #{text}': expected 'if <expr>'.", @line)
raise SyntaxError.new("Invalid else directive '@else #{text}': expected 'if <expr>'.")
end
expr = parse_script($1, :offset => line.offset + line.text.index($1))
end
@ -444,7 +459,7 @@ LONG
def parse_mixin_definition(line)
name, arg_string = line.text.scan(/^=\s*([^(]+)(.*)$/).first
raise SyntaxError.new("Invalid mixin \"#{line.text[1..-1]}\".", @line) if name.nil?
raise SyntaxError.new("Invalid mixin \"#{line.text[1..-1]}\".") if name.nil?
offset = line.offset + line.text.size - arg_string.size
args = Script::Parser.new(arg_string.strip, @line, offset).parse_mixin_definition_arglist
@ -454,11 +469,12 @@ LONG
def parse_mixin_include(line, root)
name, arg_string = line.text.scan(/^\+\s*([^(]+)(.*)$/).first
raise SyntaxError.new("Invalid mixin include \"#{line.text}\".", @line) if name.nil?
raise SyntaxError.new("Invalid mixin include \"#{line.text}\".") if name.nil?
offset = line.offset + line.text.size - arg_string.size
args = Script::Parser.new(arg_string.strip, @line, offset).parse_mixin_include_arglist
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath mixin directives.", @line + 1) unless line.children.empty?
raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath mixin directives.",
:line => @line + 1) unless line.children.empty?
Tree::MixinNode.new(name, args)
end

View file

@ -4,51 +4,98 @@ module Sass
# and the Sass file that was being parsed (if applicable).
#
# All Sass errors are raised as {Sass::SyntaxError}s.
#
# When dealing with SyntaxErrors,
# it's important to provide filename and line number information.
# This will be used in various error reports to users, including backtraces;
# see \{#sass\_backtrace} for details.
#
# Some of this information is usually provided as part of the constructor.
# New backtrace entries can be added with \{#add\_backtrace},
# which is called when an exception is raised between files (e.g. with `@import`).
#
# Often, a chunk of code will all have similar backtrace information -
# the same filename or even line.
# It may also be useful to have a default line number set.
# In those situations, the default values can be used
# by omitting the information on the original exception,
# and then calling \{#modify\_backtrace} in a wrapper `rescue`.
# When doing this, be sure that all exceptions ultimately end up
# with the information filled in.
class SyntaxError < StandardError
# The line of the Sass template on which the error occurred.
# The backtrace of the error within Sass files.
# This is an array of hashes containing information for a single entry.
# The hashes have the following keys:
#
# @return [Fixnum]
attr_accessor :sass_line
# `:filename`
# : The name of the file in which the exception was raised,
# or `nil` if no filename is available.
#
# `:line`
# : The line of the file on which the error occurred. Never nil.
#
# This information is also included in standard backtrace format
# in the output of \{#backtrace}.
#
# @return [Aray<Hash<Symbol, Object>>]
attr_accessor :sass_backtrace
# The name of the file that was being parsed when the exception was raised.
# @param msg [String] The error message
# @param attrs [Hash<Symbol, Object>] The information in the backtrace entry.
# See \{#sass\_backtrace}
def initialize(msg, attrs = {})
@message = msg
@sass_backtrace = []
add_backtrace(attrs)
end
# The name of the file in which the exception was raised.
# This could be `nil` if no filename is available.
#
# @return [String]
attr_reader :sass_filename
# @param msg [String] The error message
# @param lineno [Fixnum] See \{#sass\_line}
def initialize(msg, lineno = nil)
@message = msg
@sass_line = lineno
def sass_filename
sass_backtrace.first[:filename]
end
# Add information about the filename and line on which the error was raised,
# and re-raises the exception.
# The line of the Sass template on which the error occurred.
#
# @param filename [String] See \{#sass\_filename}
# @param line [Fixnum] See \{#sass\_line}
# @raise [Sass::SyntaxError] self
def add_metadata(filename, line)
self.sass_line ||= line
add_backtrace_entry(filename) unless sass_filename
raise self
# @return [Fixnum]
def sass_line
sass_backtrace.first[:line]
end
# Adds a properly formatted entry to the exception's backtrace.
# Adds an entry to the exception's Sass backtrace.
#
# @param filename [String] The file in which the error occurred,
# if applicable (defaults to "(sass)")
def add_backtrace_entry(filename) # :nodoc:
@sass_filename ||= filename
self.backtrace ||= []
self.backtrace.unshift "#{@sass_filename || '(sass)'}:#{@sass_line}"
# @param attrs [Hash<Symbol, Object>] The information in the backtrace entry.
# See \{#sass\_backtrace}
def add_backtrace(attrs)
sass_backtrace << attrs.reject {|k, v| v.nil?}
end
# Modify the top Sass backtrace entry (that is, the last one)
# to have the given attributes.
# If that entry already has one of the given attributes set,
# that takes precendence.
#
# @param attrs [Hash<Symbol, Object>] The information to add to the backtrace entry.
# See \{#sass\_backtrace}
def modify_backtrace(attrs)
sass_backtrace[-1] = attrs.reject {|k, v| v.nil?}.merge(sass_backtrace.last)
end
# @return [String] The error message
def to_s
@message
end
# Returns the standard exception backtrace,
# including the Sass backtrace.
#
# @return [Array<String>]
def backtrace
return nil if super.nil?
sass_backtrace.map {|h| "#{h[:filename] || "(sass)"}:#{h[:line]}"} + super
end
end
# The class for Sass errors that are raised due to invalid unit conversions

View file

@ -13,7 +13,8 @@ module Sass
# @param filename [String] The path to the Sass file
# @param options [Hash<Symbol, Object>] The options hash.
# Only the {file:SASS_REFERENCE.md#cache-option `:cache_location`} option is used
# @raise [Sass::SyntaxError] if there's an error in the document
# @raise [Sass::SyntaxError] if there's an error in the document.
# The caller has responsibility for setting backtrace information, if necessary
def tree_for(filename, options)
options = Sass::Engine::DEFAULT_OPTIONS.merge(options)
text = File.read(filename)
@ -30,15 +31,8 @@ module Sass
engine = Sass::Engine.new(text, options.merge(:filename => filename))
begin
root = engine.to_tree
rescue Sass::SyntaxError => err
err.add_backtrace_entry(filename)
raise err
end
root = engine.to_tree
try_to_write_sassc(root, compiled_filename, sha, options) if options[:cache]
root
end

View file

@ -47,12 +47,8 @@ module Sass
def self.parse(value, line, offset, filename = nil)
Parser.parse(value, line, offset, filename)
rescue Sass::SyntaxError => e
if e.message == "SassScript error"
e.instance_eval do
@message += ": #{value.dump}."
end
end
e.sass_line = line
e.message << ": #{value.inspect}." if e.message == "SassScript error"
e.modify_backtrace(:line => line, :filename => filename)
raise e
end
end

View file

@ -16,7 +16,8 @@ module Sass
def to_s(*args)
@to_s ||= (style == :compressed ? super().strip : super())
rescue Sass::SyntaxError => e
e.add_backtrace_entry(@filename)
e.modify_backtrace(:filename => @imported_filename)
e.add_backtrace(:filename => @filename, :line => @line)
raise e
end
@ -34,7 +35,8 @@ module Sass
self.children = Sass::Files.tree_for(full_filename, @options).children
self.children = perform_children(environment)
rescue Sass::SyntaxError => e
e.add_backtrace_entry(@filename)
e.modify_backtrace(:filename => @imported_filename)
e.add_backtrace(:filename => @filename, :line => @line)
raise e
end
@ -50,7 +52,7 @@ module Sass
begin
full_filename = Sass::Files.find_file_to_import(@imported_filename, import_paths)
rescue Exception => e
raise SyntaxError.new(e.message, self.line)
raise SyntaxError.new(e.message, :line => self.line, :filename => @filename)
end
if full_filename =~ /\.css$/

View file

@ -66,7 +66,7 @@ module Sass
# @see #invalid_child?
def <<(child)
if msg = invalid_child?(child)
raise Sass::SyntaxError.new(msg, child.line)
raise Sass::SyntaxError.new(msg, :line => child.line)
end
@children << child
end
@ -122,18 +122,19 @@ module Sass
def to_s
result = String.new
children.each do |child|
if child.is_a? PropNode
raise Sass::SyntaxError.new('Properties aren\'t allowed at the root of a document.', child.line)
else
next if child.invisible?
child_str = child.to_s(1)
result << child_str + (style == :compressed ? '' : "\n")
end
raise Sass::SyntaxError.new('Properties aren\'t allowed at the root of a document.',
:line => child.line) if child.is_a? PropNode
next if child.invisible?
child_str = child.to_s(1)
result << child_str + (style == :compressed ? '' : "\n")
end
result.rstrip!
return "" if result.empty?
return result + "\n"
rescue Sass::SyntaxError => e; e.add_metadata(filename, line)
rescue Sass::SyntaxError => e
e.modify_backtrace(:filename => filename)
raise e
end
# Runs the dynamic Sass code:
@ -154,7 +155,9 @@ module Sass
def perform(environment)
environment.options = @options if self.class == Tree::Node
_perform(environment)
rescue Sass::SyntaxError => e; e.add_metadata(filename, line)
rescue Sass::SyntaxError => e
e.modify_backtrace(:filename => filename, :line => line)
raise e
end
# The output style. See {file:SASS_REFERENCE.md#sass_options the Sass options documentation}.
@ -228,7 +231,7 @@ module Sass
def balance(*args)
res = Haml::Shared.balance(*args)
return res if res
raise Sass::SyntaxError.new("Unbalanced brackets.", line)
raise Sass::SyntaxError.new("Unbalanced brackets.", :line => line)
end
# Returns an error message if the given child node is invalid,

View file

@ -41,22 +41,18 @@ module Sass::Tree
# @return [String] The resulting CSS
# @raise [Sass::SyntaxError] if the property uses invalid syntax
def to_s(tabs, parent_name = nil)
if @options[:property_syntax] == :old && @prop_syntax == :new
raise Sass::SyntaxError.new("Illegal property syntax: can't use new syntax when :property_syntax => :old is set.", @line)
elsif @options[:property_syntax] == :new && @prop_syntax == :old
raise Sass::SyntaxError.new("Illegal property syntax: can't use old syntax when :property_syntax => :new is set.", @line)
end
raise Sass::SyntaxError.new("Illegal property syntax: can't use new syntax when :property_syntax => :old is set.",
:line => @line) if @options[:property_syntax] == :old && @prop_syntax == :new
raise Sass::SyntaxError.new("Illegal property syntax: can't use old syntax when :property_syntax => :new is set.",
:line => @line) if @options[:property_syntax] == :new && @prop_syntax == :old
raise Sass::SyntaxError.new("Invalid property: #{declaration.dump} (no \";\" required at end-of-line).",
:line => @line) if value[-1] == ?;
raise Sass::SyntaxError.new("Invalid property: #{declaration.dump} (no value).",
:line => @line) if value.empty? && children.empty?
if value[-1] == ?;
raise Sass::SyntaxError.new("Invalid property: #{declaration.dump} (no \";\" required at end-of-line).", @line)
end
real_name = name
real_name = "#{parent_name}-#{real_name}" if parent_name
if value.empty? && children.empty?
raise Sass::SyntaxError.new("Invalid property: #{declaration.dump} (no value).", @line)
end
join_string = case style
when :compact; ' '
when :compressed; ''

View file

@ -162,9 +162,8 @@ module Sass::Tree
if super_rules.nil?
return @parsed_rules.map do |line|
line.map do |rule|
if rule.include?(:parent)
raise Sass::SyntaxError.new("Base-level rules cannot contain the parent-selector-referencing character '#{PARENT}'.", self.line)
end
raise Sass::SyntaxError.new("Base-level rules cannot contain the parent-selector-referencing character '#{PARENT}'.",
:line => @line) if rule.include?(:parent)
rule.join
end.compact

View file

@ -186,6 +186,15 @@ SASS
rescue Sass::SyntaxError => err
assert_equal(2, err.sass_line)
assert_match(/bork#{i}\.sass$/, err.sass_filename)
assert_equal(err.sass_filename, err.sass_backtrace.first[:filename])
assert_equal(err.sass_line, err.sass_backtrace.first[:line])
assert_nil(err.sass_backtrace[1][:filename])
assert_equal(1, err.sass_backtrace[1][:line])
assert_match(/bork#{i}\.sass:2$/, err.backtrace.first)
assert_equal("(sass):1", err.backtrace[1])
else
assert(false, "Exception not raised for imported template: bork#{i}")
end