Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix Warnings #695

Closed
wants to merge 16 commits into from

4 participants

@tenderlove

Hi,

I've read previous pull requests that tried to eliminate warnings. However, I think the Sass codebase contains some great examples on why running with -w is a good thing.

I will enumerate the examples here and link to commits that deal with the issue.

Unused variable warnings

Sass assigns values to variables which are unused. In some cases, the code has no side effects and can be eliminated. I was able to delete code:

Calls to deprecated code

Sass calls methods on URI that will eventually be deleted. I fixed that here.

Redefined methods

Sass defines tests that never run because the methods were redefined. I changed the test name here, here, and could even remove completely duplicated code here.

Uninitialized instance variables

Uninitialized instance variables led me to find an instance variable that is probably misspelled. @throw_err is never set anywhere, and the positive branch of that if statement is never executed in the tests.

Circular requires

Circular requires could possibly lead to a deadlock, so I fixed those.

Shadowed variable warnings

Shadowed variables could refer to variables in the outside scope, so I fixed those here and here.

Setting external encodings

I eliminated a warning around setting the external encoding. Does this mean that Sass requires people to set the external_encoding to UTF-8? I suspect it's possible to eliminate this requirement.

Literals in conditionals

This is probably my most dubious commit. I know team Sass uses literals in conditionals as coding style, but it causes warnings (since it will always evaluate the same way). I changed the literals to constants as a compromise between readability and eliminating warnings.

Duplicate character class warnings

I left these. I think it's a bug in Ruby, so I filed a ticket.

Anyway, all of these are great examples of why I like to run -w on my code. I realize Team Sass may not care for using -w, so I've purposely not squashed these commits in the hope that some (if not all) of them are merged to mainline. Please feel free to cherry pick what you do want, and leave what you don't.

Also, I really really want to say thanks for making the tests super easy to get running. It's not often I can clone a repo and run rake and it does the right thing. THANK YOU! :smile:

:heart::heart::heart::heart::heart::heart::heart::heart::heart::heart::heart::heart::heart::heart:

@nex3
Owner

You make a pretty compelling case for the utility of some of these warnings. That said, I still don't like the idea of warnings that make the code less readable, like the warnings for !:foo and to some degree uninitialized instance variables (although the @throw_err issue is worrisome).

I'll go through the individual commits and leave review comments. Once those are addressed, I'll merge in everything other than the literal-in-conditional and uninitialized-ivars. I'll fix the @throw_err thing myself.

Thanks for the PR, @tenderlove.

@nex3

I'd like to have the way these subclasses are required be consistent across all of them. I'd rather see them all required here as they are now, and the back-requires in the individual files be removed.

I can change the requires to the other direction, but I think that files should require what they need. The String class needs the Literal class, so it should require the Literal class, not the other way around.

Owner

I like that philosophy in general, but it doesn't really work out if circular requires aren't allowed. Literal needs Bool, and Bool needs Literal. A semantic use of require there would be circular.

Given that a fully semantic model doesn't work, the "require from superclass" model at least guarantees that the superclass will be defined when the subclass is required.

Ok. I've changed it in d2ee3a6

A pattern that retains semantic properties but avoids circular requires:

require 'sass/script/literal' unless defined? Sass::Script::Literal
@nex3

As per the comment, this can be removed entirely now.

Sounds good, I'll rm this code! :D

@nex3

There's no need for this, or the :nodoc: comment. Half the methods in Sass::Util are hacks around compatibility-issues.

Sounds good, I'll delete the comment.

@nex3

Can we cache the parser? Something like $parser ||= URI.const_defined?(:DEFAULT_PARSER) ? URI::DEFAULT_PARSER : URI?

Sure. I'll cache the value.

@nex3

test_summarized_selectors_with_element

Thanks!

@nex3

The second one of these can just be test_nested_extender_with_early_child_selector.

OK, I'll fix it.

@nex3
Owner

This is in test code; we set the encoding so that tests that care about that sort of thing will behave consistently in different environments.

Gotcha, I'll remove the comment. Should the tests that require utf8 as the external encoding be specially marked?

Owner

In an ideal world, yeah, probably, but it's often hard to tell which tests rely on it implicitly.

@nex3

Why this change? Generally we make all the attributes on these classes mutable.

There is a children= method defined later in the file

Owner

Oh, sure.

@tenderlove

You make a pretty compelling case for the utility of some of these warnings. That said, I still don't like the idea of > warnings that make the code less readable, like the warnings for !:foo and to some degree uninitialized instance variables (although the @throw_err issue is worrisome).

I prefer my libraries to run warning free so that app developers are free to make the choice to use -w or not, but I definitely see where you're coming from. -w helps me track down bugs (like some of these) and it's nice to have the option to use it.

I'll go through the individual commits and leave review comments. Once those are addressed, I'll merge in everything other than the literal-in-conditional and uninitialized-ivars. I'll fix the @throw_err thing myself.

Awesome, thank you! :smile:

Thanks for the PR, @tenderlove.

No problem, I'm glad to help out. I've pushed a commit that fixes all the stuff you commented on except for the circular require issue. If you feel strongly about that one, then I'll change it as well.

Thanks for your time!

@nex3 nex3 referenced this pull request from a commit
@nex3 nex3 Merge branch 'warnings'
Closes #695
a3ffe87
@nex3 nex3 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 89 additions and 69 deletions.
  1. +1 −0  Rakefile
  2. +1 −1  lib/sass/callbacks.rb
  3. +1 −0  lib/sass/css.rb
  4. +5 −4 lib/sass/engine.rb
  5. +3 −1 lib/sass/environment.rb
  6. +0 −2  lib/sass/script/bool.rb
  7. +0 −2  lib/sass/script/color.rb
  8. +1 −0  lib/sass/script/lexer.rb
  9. +0 −2  lib/sass/script/null.rb
  10. +0 −2  lib/sass/script/number.rb
  11. +3 −1 lib/sass/script/operation.rb
  12. +7 −2 lib/sass/script/parser.rb
  13. +0 −2  lib/sass/script/string.rb
  14. +8 −2 lib/sass/scss/parser.rb
  15. +3 −1 lib/sass/scss/static_parser.rb
  16. +1 −1  lib/sass/selector/sequence.rb
  17. +3 −3 lib/sass/selector/simple_sequence.rb
  18. +3 −1 lib/sass/tree/node.rb
  19. +2 −2 lib/sass/tree/rule_node.rb
  20. +2 −0  lib/sass/tree/visitors/check_nesting.rb
  21. +1 −0  lib/sass/tree/visitors/convert.rb
  22. +4 −3 lib/sass/tree/visitors/cssize.rb
  23. +3 −1 lib/sass/tree/visitors/perform.rb
  24. +7 −2 lib/sass/tree/visitors/to_css.rb
  25. +10 −0 lib/sass/util.rb
  26. +0 −4 lib/sass/version.rb
  27. +1 −1  test/sass/conversion_test.rb
  28. +2 −16 test/sass/extend_test.rb
  29. +4 −4 test/sass/importer_test.rb
  30. +1 −1  test/sass/scss/css_test.rb
  31. +5 −6 test/sass/source_map_test.rb
  32. +1 −1  test/sass/util_test.rb
  33. +6 −1 test/test_helper.rb
View
1  Rakefile
@@ -17,6 +17,7 @@ Rake::TestTask.new do |t|
test_files.exclude(scope('test/plugins/*'))
t.test_files = test_files
t.verbose = true
+ t.warning = true
end
# ----- Packaging -----
View
2  lib/sass/callbacks.rb
@@ -55,7 +55,7 @@ def on_#{name}(&block)
end
def run_#{name}(*args)
- return unless @_sass_callbacks
+ return unless defined?(@_sass_callbacks) && @_sass_callbacks
return unless @_sass_callbacks[#{name.inspect}]
@_sass_callbacks[#{name.inspect}].each {|c| c[*args]}
end
View
1  lib/sass/css.rb
@@ -31,6 +31,7 @@ def initialize(template, options = {})
template = template.read
end
+ @checked_encoding = nil
@options = options.dup
# Backwards compatibility
@options[:old] = true if @options[:alternate] == false
View
9 lib/sass/engine.rb
@@ -252,6 +252,9 @@ def self.for_file(filename, options)
def initialize(template, options={})
@options = self.class.normalize_options(options)
@template = template
+ @filename = nil
+ @checked_encoding = nil
+ @line = nil
end
# Render the template to CSS.
@@ -353,7 +356,7 @@ def _render_with_sourcemap(sourcemap_uri)
rendered << "\n" if rendered[-1] != ?\n
rendered << "\n" unless compressed
rendered << "/*@ sourceMappingURL="
- rendered << URI.encode(sourcemap_uri)
+ rendered << Sass::Util.escape_uri(sourcemap_uri)
rendered << " */"
rendered = encode_and_set_charset(rendered)
return rendered, sourcemap
@@ -673,9 +676,7 @@ def parse_property_or_rule(line)
property
else
res.pop if comment
- scanner_start_pos = scanner.pos
interp_parsed = parse_interp(scanner.rest)
- scanned_size = scanner.pos - scanner_start_pos
selector_range = Sass::Source::Range.new(
ident_range.start_pos,
Sass::Source::Position.new(@line, to_parser_offset(line.offset) + line.text.length),
@@ -911,7 +912,7 @@ def parse_import(line, value, offset)
def parse_import_arg(scanner, offset)
return if scanner.eos?
- if match_length = scanner.match?(/url\(/i)
+ if scanner.match?(/url\(/i)
script_parser = Sass::Script::Parser.new(scanner, @line, to_parser_offset(offset), @options)
str = script_parser.parse_string
View
4 lib/sass/environment.rb
@@ -27,6 +27,8 @@ class Environment
# @param parent [Environment] See \{#parent}
def initialize(parent = nil, options = nil)
@parent = parent
+ @caller = nil
+ @content = nil
@options = options || (parent && parent.options) || {}
end
@@ -62,7 +64,7 @@ def #{name}(name)
end
def _#{name}(name)
- (@#{name}s && @#{name}s[name]) || @parent && @parent._#{name}(name)
+ (defined?(@#{name}s) && @#{name}s && @#{name}s[name]) || @parent && @parent._#{name}(name)
end
protected :_#{name}
View
2  lib/sass/script/bool.rb
@@ -1,5 +1,3 @@
-require 'sass/script/literal'
-
module Sass::Script
# A SassScript object representing a boolean (true or false) value.
class Bool < Literal
View
2  lib/sass/script/color.rb
@@ -1,5 +1,3 @@
-require 'sass/script/literal'
-
module Sass::Script
# A SassScript object representing a CSS color.
#
View
1  lib/sass/script/lexer.rb
@@ -138,6 +138,7 @@ def initialize(str, line, offset, options)
@options = options
@interpolation_stack = []
@prev = nil
+ @tok = nil
end
# Moves the lexer forward one token.
View
2  lib/sass/script/null.rb
@@ -1,5 +1,3 @@
-require 'sass/script/literal'
-
module Sass::Script
# A SassScript object representing a null value.
class Null < Literal
View
2  lib/sass/script/number.rb
@@ -1,5 +1,3 @@
-require 'sass/script/literal'
-
module Sass::Script
# A SassScript object representing a number.
# SassScript numbers can have decimal values,
View
4 lib/sass/script/operation.rb
@@ -1,7 +1,10 @@
require 'set'
+require 'sass/script/literal'
require 'sass/script/string'
require 'sass/script/number'
require 'sass/script/color'
+require 'sass/script/bool'
+require 'sass/script/null'
require 'sass/script/functions'
require 'sass/script/unary_operation'
require 'sass/script/interpolation'
@@ -35,7 +38,6 @@ def inspect
# @see Node#to_sass
def to_sass(opts = {})
- pred = Sass::Script::Parser.precedence_of(@operator)
o1 = operand_to_sass @operand1, :left, opts
o2 = operand_to_sass @operand2, :right, opts
sep =
View
9 lib/sass/script/parser.rb
@@ -29,6 +29,8 @@ def offset
def initialize(str, line, offset, options = {})
@options = options
@lexer = lexer_class.new(str, line, offset, options)
+ @stop_at = nil
+ @in_parens = nil
end
# Parses a SassScript expression within an interpolated segment (`#{}`).
@@ -282,12 +284,15 @@ def expr
production :equals, :interpolation, :single_eq
+ WA = true # :nodoc:
+ WB = true # :nodoc:
+
def try_op_before_interp(op, prev = nil)
return unless @lexer.peek && @lexer.peek.type == :begin_interpolation
wb = @lexer.whitespace?(op)
str = Script::String.new(Lexer::OPERATORS_REVERSE[op.type])
str.line = @lexer.line
- interp = Script::Interpolation.new(prev, str, nil, wb, !:wa, :originally_text)
+ interp = Script::Interpolation.new(prev, str, nil, wb, !WA, :originally_text)
interp.line = @lexer.line
interpolation(interp)
end
@@ -301,7 +306,7 @@ def try_ops_after_interp(ops, name, prev = nil)
str = Script::String.new(Lexer::OPERATORS_REVERSE[op.type])
str.line = @lexer.line
start_pos = source_position
- interp = Script::Interpolation.new(prev, str, assert_expr(name), !:wb, wa, :originally_text)
+ interp = Script::Interpolation.new(prev, str, assert_expr(name), !WB, wa, :originally_text)
interp.line = @lexer.line
interp.source_range = range(start_pos)
return interp
View
2  lib/sass/script/string.rb
@@ -1,5 +1,3 @@
-require 'sass/script/literal'
-
module Sass::Script
# A SassScript object representing a CSS string *or* a CSS identifier.
class String < Literal
View
10 lib/sass/scss/parser.rb
@@ -28,6 +28,10 @@ def initialize(str, filename, importer, line = 1, offset = 1)
@line = line
@offset = offset
@strs = []
+ @throw_error = nil
+ @throw_err = nil
+ @use_property_exception = nil
+ @expected = nil
end
# Parses an SCSS document.
@@ -459,9 +463,11 @@ def _moz_document_directive(start_pos)
directive_body(res.flatten, start_pos)
end
+ ALLOW_VAR = true # :nodoc:
+
def moz_document_function
return unless val = interp_uri || _interp_string(:url_prefix) ||
- _interp_string(:domain) || function(!:allow_var) || interpolation
+ _interp_string(:domain) || function(!ALLOW_VAR) || interpolation
ss
val
end
@@ -696,7 +702,7 @@ def simple_selector_sequence
# http://www.w3.org/TR/css3-animations/#keyframes-
start_pos = source_position
- return expr(!:allow_var) unless e = element_name || id_selector ||
+ return expr(!ALLOW_VAR) unless e = element_name || id_selector ||
class_selector || placeholder_selector || attrib || pseudo ||
parent_selector || interpolation_selector
res = [e]
View
4 lib/sass/scss/static_parser.rb
@@ -26,9 +26,11 @@ def parse_selector
private
+ ALLOW_VAR = true # :nodoc:
+
def moz_document_function
return unless val = tok(URI) || tok(URL_PREFIX) || tok(DOMAIN) ||
- function(!:allow_var)
+ function(!ALLOW_VAR)
ss
[val]
end
View
2  lib/sass/selector/sequence.rb
@@ -255,7 +255,7 @@ def merge_final_ops(seq1, seq2, res = [])
# is a supersequence of the other, use that, otherwise give up.
lcs = Sass::Util.lcs(ops1, ops2)
return unless lcs == ops1 || lcs == ops2
- res.unshift *(ops1.size > ops2.size ? ops1 : ops2).reverse
+ res.unshift(*(ops1.size > ops2.size ? ops1 : ops2).reverse)
return res
end
View
6 lib/sass/selector/simple_sequence.rb
@@ -135,9 +135,9 @@ def do_extend(extends, parent_directives, seen = Set.new)
# by the time extension and unification happen,
# this exception will only ever be raised as a result of programmer error
def unify(sels, other_subject)
- return unless sseq = members.inject(sels) do |sseq, sel|
- return unless sseq
- sel.unify(sseq)
+ return unless sseq = members.inject(sels) do |member, sel|
+ return unless member
+ sel.unify(member)
end
SimpleSequence.new(sseq, other_subject || subject?)
end
View
4 lib/sass/tree/node.rb
@@ -32,7 +32,7 @@ class Node
# The child nodes of this node.
#
# @return [Array<Tree::Node>]
- attr_accessor :children
+ attr_reader :children
# Whether or not this node has child nodes.
# This may be true even when \{#children} is empty,
@@ -64,6 +64,8 @@ class Node
def initialize
@children = []
+ @filename = nil
+ @options = nil
end
# Sets the options hash for the node and all its children.
View
4 lib/sass/tree/rule_node.rb
@@ -1,5 +1,4 @@
require 'pathname'
-require 'uri'
module Sass::Tree
# A static node reprenting a CSS rule.
@@ -69,6 +68,7 @@ def initialize(rule, selector_source_range = nil)
@rule = Sass::Util.strip_string_array(merged)
@selector_source_range = selector_source_range
@tabs = 0
+ @parsed_rules = nil
try_to_parse_non_interpolated_rules
super()
end
@@ -115,7 +115,7 @@ def continued?
#
# @return [{#to_s => #to_s}]
def debug_info
- {:filename => filename && ("file://" + URI.escape(File.expand_path(filename))),
+ {:filename => filename && ("file://" + Sass::Util.escape_uri(File.expand_path(filename))),
:line => self.line}
end
View
2  lib/sass/tree/visitors/check_nesting.rb
@@ -4,6 +4,8 @@ class Sass::Tree::Visitors::CheckNesting < Sass::Tree::Visitors::Base
def initialize
@parents = []
+ @parent = nil
+ @current_mixin_def = nil
end
def visit(node)
View
1  lib/sass/tree/visitors/convert.rb
@@ -16,6 +16,7 @@ def initialize(options, format)
@options = options
@format = format
@tabs = 0
+ @is_else = nil
# 2 spaces by default
@tab_chars = @options[:indent] || " "
end
View
7 lib/sass/tree/visitors/cssize.rb
@@ -14,6 +14,7 @@ def self.visit(root); super; end
def initialize
@parent_directives = []
@extends = Sass::Util::SubsetMap.new
+ @parent = nil
end
# If an exception is raised, this adds proper metadata to the backtrace.
@@ -124,12 +125,12 @@ def visit_extend(node)
end
sel = sseq.members
- parent.resolved_rules.members.each do |seq|
- if !seq.members.last.is_a?(Sass::Selector::SimpleSequence)
+ parent.resolved_rules.members.each do |member|
+ if !member.members.last.is_a?(Sass::Selector::SimpleSequence)
raise Sass::SyntaxError.new("#{seq} can't extend: invalid selector")
end
- @extends[sel] = Extend.new(seq, sel, node, @parent_directives.dup, :not_found)
+ @extends[sel] = Extend.new(member, sel, node, @parent_directives.dup, :not_found)
end
end
View
4 lib/sass/tree/visitors/perform.rb
@@ -191,11 +191,13 @@ def visit_for(node)
end
end
+ HAS_CONTENT = true # :nodoc:
+
# Loads the function into the environment.
def visit_function(node)
env = Sass::Environment.new(@environment, node.options)
@environment.set_local_function(node.name,
- Sass::Callable.new(node.name, node.args, node.splat, env, node.children, !:has_content, "function"))
+ Sass::Callable.new(node.name, node.args, node.splat, env, node.children, !HAS_CONTENT, "function"))
[]
end
View
9 lib/sass/tree/visitors/to_css.rb
@@ -13,7 +13,13 @@ def initialize(build_source_mapping = false)
@line = 1
@offset = 1
@result = ""
- @source_mapping = Sass::Source::Map.new if build_source_mapping
+ @lstrip = false
+ @in_directive = nil
+ if build_source_mapping
+ @source_mapping = Sass::Source::Map.new
+ else
+ @source_mapping = nil
+ end
end
# Runs the visitor on `node`.
@@ -274,7 +280,6 @@ def visit_rule(node)
joined_rules.gsub!(/\s*\n\s*/, "#{line_separator}#{per_rule_indent}")
old_spaces = ' ' * @tabs
- spaces = ' ' * (@tabs + 1)
if node.style != :compressed
if node.options[:debug_info] && !@in_directive
visit(debug_info_rule(node.debug_info, node.options))
View
10 lib/sass/util.rb
@@ -3,6 +3,7 @@
require 'enumerator'
require 'stringio'
require 'rbconfig'
+require 'uri'
require 'sass/root'
require 'sass/util/subset_map'
@@ -962,6 +963,15 @@ def method_missing(name, *args, &block)
end
end
+
+ URI_ESCAPE = URI.const_defined?(:DEFAULT_PARSER) ?
+ URI::DEFAULT_PARSER :
+ URI
+
+ def escape_uri(uri)
+ URI_ESCAPE.escape uri
+ end
+
private
# Calculates the memoization table for the Least Common Subsequence algorithm.
View
4 lib/sass/version.rb
@@ -1,8 +1,4 @@
require 'date'
-
-# This is necessary for loading Sass when Haml is required in Rails 3.
-# Once the split is complete, we can remove it.
-require File.dirname(__FILE__) + '/../sass'
require 'sass/util'
module Sass
View
2  test/sass/conversion_test.rb
@@ -1347,7 +1347,7 @@ def test_dasherize
SASS
end
- def test_loud_comment_conversion
+ def test_loud_comment_conversion
assert_renders(<<SASS, <<SCSS)
/*! \#{"interpolated"}
SASS
View
18 test/sass/extend_test.rb
@@ -489,7 +489,7 @@ def test_nested_extender_with_child_selector_unifies
SCSS
end
- def test_nested_extender_with_early_child_selectors_doesnt_subseq_them
+ def test_nested_extender_with_early_child_selector
assert_equal <<CSS, render(<<SCSS)
.foo .bar, .foo .bip > .baz {
a: b; }
@@ -1061,7 +1061,7 @@ def test_extend_with_subject_fails_with_conflicting_subject
x! .bar {a: b}
y! .bap {@extend .bar}
SCSS
-end
+ end
def test_extend_warns_when_extendee_doesnt_exist
assert_raise_message(Sass::SyntaxError, <<ERR) {render(<<SCSS)}
@@ -1098,20 +1098,6 @@ def test_extend_succeeds_when_one_extension_fails_but_others_dont
SCSS
end
- def test_extend_succeeds_when_one_extension_fails_but_others_dont
- assert_equal(<<CSS, render(<<SCSS))
-a.bar {
- a: b; }
-
-.bar, b.foo {
- c: d; }
-CSS
-a.bar {a: b}
-.bar {c: d}
-b.foo {@extend .bar}
-SCSS
- end
-
def test_optional_extend_succeeds_when_extendee_doesnt_exist
assert_equal("", render(<<SCSS))
.foo {@extend .bar !optional}
View
8 test/sass/importer_test.rb
@@ -200,7 +200,7 @@ def test_source_map_with_only_css_uri_supports_public_url_imports
}
SCSS
- rendered, sourcemap = engine.render_with_sourcemap('sourcemap_uri')
+ _, sourcemap = engine.render_with_sourcemap('sourcemap_uri')
assert_equal <<JSON.strip, sourcemap.to_json(:css_uri => 'css_uri')
{
"version": "3",
@@ -223,7 +223,7 @@ def test_source_map_with_only_css_uri_doesnt_support_filesystem_importer
.foo {a: b}
SCSS
- rendered, sourcemap = engine.render_with_sourcemap('http://1.example.com/style.map')
+ _, sourcemap = engine.render_with_sourcemap('http://1.example.com/style.map')
assert_warning(<<WARNING) {sourcemap.to_json(:css_uri => 'css_uri')}
WARNING: Couldn't determine public URL for "#{filename_for_test(:scss)}" while generating sourcemap.
@@ -244,7 +244,7 @@ def test_source_map_with_css_uri_and_css_path_doesnt_support_filesystem_importer
.foo {a: b}
SCSS
- rendered, sourcemap = engine.render_with_sourcemap('http://1.example.com/style.map')
+ _, sourcemap = engine.render_with_sourcemap('http://1.example.com/style.map')
assert_warning(<<WARNING) {sourcemap.to_json(:css_uri => 'css_uri', :css_path => 'css_path')}
WARNING: Couldn't determine public URL for "#{filename_for_test(:scss)}" while generating sourcemap.
@@ -293,7 +293,7 @@ def test_source_map_with_css_path_and_sourcemap_path_supports_file_system_import
.foo {a: b}
SCSS
- rendered, sourcemap = engine.render_with_sourcemap('http://map.example.com/map/style.map')
+ _, sourcemap = engine.render_with_sourcemap('http://map.example.com/map/style.map')
css_path = 'static/style.css'
sourcemap_path = 'map/style.map'
assert_equal <<JSON.strip, sourcemap.to_json(:css_path => css_path, :sourcemap_path => sourcemap_path)
View
2  test/sass/scss/css_test.rb
@@ -651,7 +651,7 @@ def test_supports
## Selectors
# Taken from http://dev.w3.org/csswg/selectors4/#overview
- def test_summarized_selectors
+ def test_summarized_selectors_with_element
assert_selector_parses('*')
assert_selector_parses('E')
assert_selector_parses('E:not(s)')
View
11 test/sass/source_map_test.rb
@@ -5,7 +5,7 @@
class SourcemapTest < Test::Unit::TestCase
def test_to_json_requires_args
- rendered, sourcemap = render_with_sourcemap('')
+ _, sourcemap = render_with_sourcemap('')
assert_raise(ArgumentError) {sourcemap.to_json({})}
assert_raise(ArgumentError) {sourcemap.to_json({:css_path => 'foo'})}
assert_raise(ArgumentError) {sourcemap.to_json({:sourcemap_path => 'foo'})}
@@ -474,7 +474,7 @@ def test_while_sourcemap_scss
CSS
end
-def test_while_sourcemap_sass
+ def test_while_sourcemap_sass
assert_parses_with_mapping <<'SASS', <<'CSS', :syntax => :sass
$i: 6
@while $i > 0
@@ -723,7 +723,6 @@ def build_ranges(text, file_name = nil)
start_positions = {}
text.split("\n").each_with_index do |line_text, line|
line += 1 # lines shoud be 1-based
- match_start = 0
while match = line_text.match(ANNOTATION_REGEX)
closing = !match[1].empty?
name = match[2]
@@ -750,13 +749,13 @@ def build_mapping_from_annotations(source, css, source_file_name)
source_ranges = build_ranges(source, source_file_name)
target_ranges = build_ranges(css)
map = Sass::Source::Map.new
- mappings = Sass::Util.flatten(source_ranges.map do |(name, sources)|
+ Sass::Util.flatten(source_ranges.map do |(name, sources)|
assert(sources.length == 1, "#{sources.length} source ranges encountered for annotation #{name}")
assert(target_ranges[name], "No target ranges for annotation #{name}")
target_ranges[name].map {|target_range| [sources.first, target_range]}
end, 1).
- sort_by {|(source, target)| [target.start_pos.line, target.start_pos.offset]}.
- each {|(source, target)| map.add(source, target)}
+ sort_by {|(_, target)| [target.start_pos.line, target.start_pos.offset]}.
+ each {|(s2, target)| map.add(s2, target)}
map
end
View
2  test/sass/util_test.rb
@@ -324,7 +324,7 @@ def assert_json_string(source, target)
def test_json_value_of
assert_json_value 0, "0"
- assert_json_value -42, "-42"
+ assert_json_value(-42, "-42")
assert_json_value 42, "42"
assert_json_value true, "true"
assert_json_value false, "false"
View
7 test/test_helper.rb
@@ -7,7 +7,12 @@
require 'mathn' if ENV['MATHN'] == 'true'
Sass::RAILS_LOADED = true unless defined?(Sass::RAILS_LOADED)
-Encoding.default_external = 'UTF-8' if defined?(Encoding)
+
+if defined?(Encoding)
+ $-w, w = false, $-w
+ Encoding.default_external = 'UTF-8'
+ $-w = w
+end
module Sass::Script::Functions
def option(name)
Something went wrong with that request. Please try again.