Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with leading negatives and space around operator rule #490

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/rules/space-around-operator.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var checkSpacing = function (node, i, parent, parser, result) {
//////////////////////////

if (parent && operator && next) {

// The parent ofthe exceptions are always going to be of type value
if (parent.is('value')) {
// 1. We should allow valid CSS use of leading - operator when
Expand All @@ -55,7 +56,6 @@ var checkSpacing = function (node, i, parent, parser, result) {
// 2. We should allow valid CSS use of / operators when defining
// font size/line-height
if (previous) {

// The first value is the font-size and must have a unit, therefore
// a node of type dimension
if (operator === '/' && previous.is('dimension')
Expand All @@ -65,9 +65,10 @@ var checkSpacing = function (node, i, parent, parser, result) {
return false;
}
}
// If we have a negative value without a preceeding value then we should
// return false
else {

// 3. If there is nothing leading the minus, it's a negative value so we
// shouldn't have a following space
if (!previous && operator === '-' && !next.is('space')) {
return false;
}
}
Expand All @@ -91,8 +92,8 @@ var checkSpacing = function (node, i, parent, parser, result) {
}
else {
if (
((previous.end.line >= previous.start.line) && (previous.end.column > previous.start.column))
|| (next.end.line >= next.start.line) && (next.end.column > next.start.column)
(previous && (previous.end.line >= previous.start.line) && (previous.end.column > previous.start.column))
|| (next && (next.end.line >= next.start.line) && (next.end.column > next.start.column))
) {
result = helpers.addUnique(result, {
'ruleId': parser.rule.name,
Expand Down
8 changes: 4 additions & 4 deletions tests/rules/space-around-operator.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('space around operator - scss', function () {
lint.test(file, {
'space-around-operator': 1
}, function (data) {
lint.assert.equal(83, data.warningCount);
lint.assert.equal(84, data.warningCount);
done();
});
});
Expand All @@ -26,7 +26,7 @@ describe('space around operator - scss', function () {
}
]
}, function (data) {
lint.assert.equal(77, data.warningCount);
lint.assert.equal(78, data.warningCount);
done();
});
});
Expand All @@ -42,7 +42,7 @@ describe('space around operator - sass', function () {
lint.test(file, {
'space-around-operator': 1
}, function (data) {
lint.assert.equal(83, data.warningCount);
lint.assert.equal(84, data.warningCount);
done();
});
});
Expand All @@ -56,7 +56,7 @@ describe('space around operator - sass', function () {
}
]
}, function (data) {
lint.assert.equal(77, data.warningCount);
lint.assert.equal(78, data.warningCount);
done();
});
});
Expand Down
16 changes: 16 additions & 0 deletions tests/sass/space-around-operator.sass
Original file line number Diff line number Diff line change
Expand Up @@ -450,3 +450,19 @@ h1

.foo
z-index: -1

// ========================
// Interesting user cases
// ========================

// Should ignore the this
.foo
$val: -($foo * 2)

// Should flag this if rule is true
.foo
$val: 1-($foo * 2)

// Should flag this if rule is false
.foo
$val: 1 - ($foo * 2)
24 changes: 24 additions & 0 deletions tests/sass/space-around-operator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -456,3 +456,27 @@ $qux: (2 * 1); // FIXME: Gonzales - FIXED IN BETA
.foo {
z-index: -1;
}

// ========================
// Interesting user cases
// ========================

// Should ignore the this
.foo {
$val: -($foo * 2);
}

// Should flag this if rule is true
.foo {
$val: 1-($foo * 2);
}

// Should flag this if rule is false
.foo {
$val: 1 - ($foo * 2);
}

// Invalid Sass so compile would fail - Issue #483
// .foo {
// $val: - ($foo * 2);
// }