Skip to content

Commit

Permalink
Fix SpaceAfterComma to not infinite loop on multline map literals
Browse files Browse the repository at this point in the history
Fixes sds#642
  • Loading branch information
sds committed Dec 1, 2015
1 parent a3c28b1 commit 4fb8d3f
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

* Add `placeholder`, `-moz-*`, `-ms-*`, and `-webkit-*` to `PseudoElement`
linter
* Fix `SpaceAfterComma` to not infinite loop on map literals as arguments
spanning multiple lines

## 0.43.0

Expand Down
2 changes: 2 additions & 0 deletions lib/scss_lint/linter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ def character_at(source_position, offset = 0)
actual_line = source_position.line - 1
actual_offset = source_position.offset + offset - 1

return nil if actual_offset < 0

engine.lines.size > actual_line && engine.lines[actual_line][actual_offset]
end

Expand Down
23 changes: 14 additions & 9 deletions lib/scss_lint/linter/space_after_comma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,18 @@ def check_commas_after_args(args, arg_type)
# Sometimes the line we're looking at doesn't even contain a comma!
next unless engine.lines[arg.line - 1].include?(',')

offset = find_comma_offset(arg)
comma_position = find_comma_position(arg)

# Check for space or newline after comma (we allow arguments to be split
# up over multiple lines).
spaces = 0
while (char = character_at(arg.source_range.end_pos, offset + 1)) == ' '
while (char = character_at(comma_position, spaces + 1)) == ' '
spaces += 1
offset += 1
end
next if char == "\n" || # Ignore trailing spaces
valid_spaces_after_comma?(spaces)

add_lint arg, "Commas in #{arg_type} should be followed by a single space"
add_lint comma_position, "Commas in #{arg_type} should be followed by a single space"
end
end

Expand All @@ -100,20 +99,26 @@ def check_commas_after_args(args, arg_type)
# source range. Thus we need to start at the indicated range, and check
# left and right of that range, gradually moving further outward until
# we find the comma.
def find_comma_offset(arg)
def find_comma_position(arg) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
offset = 0
pos = arg.source_range.end_pos

if character_at(arg.source_range.end_pos, offset) != ','
if character_at(pos, offset) != ','
loop do
offset += 1
break if character_at(arg.source_range.end_pos, offset) == ','
break if (right_char = character_at(pos, offset)) == ','
offset = -offset
break if character_at(arg.source_range.end_pos, offset) == ','
break if (left_char = character_at(pos, offset)) == ','
offset = -offset

next unless right_char.nil? && left_char.nil?
offset = 0
pos = Sass::Source::Position.new(pos.line + 1, 1)
break if character_at(pos, offset) == ','
end
end

offset
Sass::Source::Position.new(pos.line, pos.offset + offset)
end
end
end
12 changes: 12 additions & 0 deletions spec/scss_lint/linter/space_after_comma_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,18 @@
it { should_not report_lint }
end
end

context 'when map literal spans multiple lines' do
let(:scss) { <<-SCSS }
@include mixin('arg1', (
key1: 'arg2-key1-value',
key2: 'arg2-key2-value'
),$arg3: 'arg3-value', $arg4: 'arg4-value'
);
SCSS

it { should report_lint line: 4 }
end
end

context 'when more than one space is preferred' do
Expand Down

0 comments on commit 4fb8d3f

Please sign in to comment.