Skip to content

Commit

Permalink
Add SpaceAroundOperator linter
Browse files Browse the repository at this point in the history
  • Loading branch information
srawlins authored and sds committed Jul 23, 2015
1 parent f99c7d0 commit c44e3e5
Show file tree
Hide file tree
Showing 4 changed files with 357 additions and 0 deletions.
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ linters:
SpaceAfterVariableName:
enabled: true

SpaceAroundOperator:
enabled: true
style: one_space # or 'no_space'

SpaceBeforeBrace:
enabled: true
style: space # or 'new_line'
Expand Down
32 changes: 32 additions & 0 deletions lib/scss_lint/linter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Below is a list of linters supported by `scss-lint`, ordered alphabetically.
* [SpaceAfterPropertyColon](#spaceafterpropertycolon)
* [SpaceAfterPropertyName](#spaceafterpropertyname)
* [SpaceAfterVariableName](#spaceaftervariablename)
* [SpaceAroundOperator](#spacearoundoperator)
* [SpaceBeforeBrace](#spacebeforebrace)
* [SpaceBetweenParens](#spacebetweenparens)
* [StringQuotes](#stringquotes)
Expand Down Expand Up @@ -1293,6 +1294,37 @@ $my-var : 0;
$my-var: 0;
```

## SpaceAroundOperator

Operators should be formatted with a single space on both sides of an infix
operator. These include `+`, `-`, `*`, `/`, `%`, `==`, `!=`, `>`, `>=`, `<`,
and `<=`.

**Bad: no space around operator**
```scss
margin: 5px+5px;
```

**Bad: more than one space around operator**
```scss
margin: 5px + 5px;
```

**Good**
```scss
margin: 5px + 5px;
```

Note that this linter only applies to actual, evaluated operators. So values
like `nth-child(2n+1)`, `10px/12px`, and `my-font` will not be linted, as they
are valid CSS.

The `style` option allows you to specify a different preferred style.

Configuration Option | Description
---------------------|---------------------------------------------------------
`style` | `one_space`, `no_space` (default **one_space**)

## SpaceBeforeBrace

Opening braces should be preceded by a single space.
Expand Down
81 changes: 81 additions & 0 deletions lib/scss_lint/linter/space_around_operator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
module SCSSLint
# Checks for space around operators on values.
class Linter::SpaceAroundOperator < Linter
include LinterRegistry

def visit_script_operation(node) # rubocop:disable Metrics/AbcSize
source = source_from_range(node.source_range).chop
left_range = node.operand1.source_range
right_range = node.operand2.source_range

# We need to #chop at the end because an operation's operand1 _always_
# includes one character past the actual operand (which is either a
# whitespace character, or the first character of the operation).
left_source = source_from_range(left_range).chop
right_source = source_from_range(right_range).chop
operator_source = source_between(left_range, right_range)
left_source, operator_source = adjust_left_boundary(left_source, operator_source)

match = operator_source.match(/
(?<left_space>\s*)
(?<operator>\S+)
(?<right_space>\s*)
/x)

if config['style'] == 'one_space'
if match[:left_space] != ' ' || match[:right_space] != ' '
add_lint(node, SPACE_MSG % [source, left_source, match[:operator], right_source])
end
elsif match[:left_space] != '' || match[:right_space] != ''
add_lint(node, NO_SPACE_MSG % [source, left_source, match[:operator], right_source])
end

yield
end

private

SPACE_MSG = '`%s` should be written with a single space on each side of ' \
'the operator: `%s %s %s`'
NO_SPACE_MSG = '`%s` should be written without spaces around the ' \
'operator: `%s%s%s`'

def source_between(range1, range2)
# We don't want to add 1 to range1.end_pos.offset for the same reason as
# the #chop comment above.
between_start = Sass::Source::Position.new(
range1.end_pos.line,
range1.end_pos.offset,
)
between_end = Sass::Source::Position.new(
range2.start_pos.line,
range2.start_pos.offset - 1,
)

source_from_range(Sass::Source::Range.new(between_start,
between_end,
range1.file,
range1.importer))
end

def adjust_left_boundary(left, operator)
# If the left operand is wrapped in parentheses, any right parens end up
# in the operator source. Here, we move them into the left operand
# source, which is awkward in any messaging, but it works.
if match = operator.match(/^(\s*\))+/)
left += match[0]
operator = operator[match.end(0)..-1]
end

# If the left operand is a nested operation, Sass includes any whitespace
# before the (outer) operator in the left operator's source_range's
# end_pos, which is not the case with simple, non-operation operands.
if match = left.match(/\s+$/)
left = left[0..match.begin(0)]
operator = match[0] + operator
end

[left, operator]
end
end
end
240 changes: 240 additions & 0 deletions spec/scss_lint/linter/space_around_operator_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
require 'spec_helper'

describe SCSSLint::Linter::SpaceAroundOperator do
let(:linter_config) { { 'style' => style } }

context 'when one space is preferred' do
let(:style) { 'one_space' }

context 'when no properties exist' do
let(:scss) { <<-SCSS }
p {
}
SCSS

it { should_not report_lint }
end

context 'when values without infix operators exist' do
let(:scss) { <<-SCSS }
p {
margin: 5px;
}
SCSS

it { should_not report_lint }
end

context 'when values with properly-spaced infix operators exist' do
let(:scss) { <<-SCSS }
$x: 2px + 2px;
p {
margin: 5px + 5px;
}
SCSS

it { should_not report_lint }
end

context 'when numeric values with infix operators exist' do
let(:scss) { <<-SCSS }
p {
margin: 5px+5px;
margin: 5px + 5px;
margin: 4px*2;
margin: 20px%3;
font-family: sans-+serif;
}
$x: 10px+10px;
$x: 20px-10px;
SCSS

it { should report_lint line: 2 }
it { should report_lint line: 3 }
it { should report_lint line: 4 }
it { should report_lint line: 5 }
it { should report_lint line: 6 }
it { should report_lint line: 9 }
it { should report_lint line: 10 }
end

context 'when numeric values with infix operators and newlines exist' do
let(:scss) { <<-SCSS }
p {
margin: 7px+
7px;
padding: 9px
+ 9px;
}
SCSS

it { should report_lint line: 2 }
it { should report_lint line: 4 }
end

context 'when numeric values with multiple infix operators exist' do
let(:scss) { <<-SCSS }
p {
margin: 5px*2+8px;
}
SCSS

it { should report_lint line: 2, count: 2 }
end

context 'when if nodes with comparison infix operators exist' do
let(:scss) { <<-SCSS }
p {
@if 3==2 {
margin: 5px;
}
@if 5>4 {
margin: 5px;
}
}
SCSS

it { should report_lint line: 2 }
it { should report_lint line: 6 }
end

context 'when values containing multiple operators exist' do
let(:scss) { <<-SCSS }
p {
margin: 2px+8px 18px+12px;
}
SCSS

it { should report_lint line: 2, count: 2 }
end

context 'when a function call contains a value with infix operators' do
let(:scss) { <<-SCSS }
p {
margin: some-function(2em+1em);
}
SCSS

it { should report_lint line: 2 }
end

context 'when mixin include contains a value with infix operators' do
let(:scss) { <<-SCSS }
p {
@include some-mixin(4em-2em);
}
SCSS

it { should report_lint line: 2 }
end

context 'when string contains an infix operator' do
let(:scss) { <<-SCSS }
p {
content: func("4em-2em");
}
SCSS

it { should_not report_lint }
end

context 'when string contains an interpolated infix operator' do
let(:scss) { <<-SCSS }
p {
content: "There are \#{11+1} months."
}
SCSS

it { should report_lint line: 2 }
end

context 'when values with non-evaluated operations exist' do
let(:scss) { <<-SCSS }
$my-variable: 10px;
p {
font: 12px/10px;
margin: 2em-1em;
padding: $my-variable;
font-size: em(16px / $base-font-size);
}
SCSS

it { should report_lint line: 5 }
end

context 'when values with nested operators and proper space exist' do
let(:scss) { <<-SCSS }
p {
top: ($number) * -2;
top: (($number)) * ((2));
top: (($number ) ) * ( ( 2 ) );
top: ($number
) * ( 2 );
top: 2px + 3px + 4px;
top: (2px + 3px) + 4px;
top: ( 2px + 3px ) + 4px;
top: 2px + (3px + 4px);
top: 2px + ( 3px + 4px );
top: (2px) + (3px) + (4px);
top: ( 2px ) + ( 3px ) + ( 4px );
}
@if $var == 1 or $var == 2 {
}
SCSS

it { should_not report_lint }
end

context 'when values with proper division operations exist' do
let(:scss) { <<-SCSS }
$x: 20px;
p {
width: $x/2;
width: round(1.5)/2;
width: (50px/2);
width: 10px + 20px/2;
}
SCSS

it { should report_lint line: 3 }
it { should report_lint line: 4 }
it { should report_lint line: 5 }
it { should report_lint line: 6 }
end
end
context 'when one space is preferred' do
let(:style) { 'no_space' }

context 'when values with single-spaced infix operators exist' do
let(:scss) { <<-SCSS }
$x: 2px + 2px;
p {
margin: 5px + 5px;
width: 5px + 5px;
}
SCSS

it { should report_lint line: 1 }
it { should report_lint line: 4 }
it { should report_lint line: 5 }
end

context 'when values with no-spaced infix operators exist' do
let(:scss) { <<-SCSS }
$x: 2px+2px;
p {
margin: 5px+5px;
}
SCSS

it { should_not report_lint }
end
end
end

0 comments on commit c44e3e5

Please sign in to comment.