Merge pull request #490 from bgriffith/feature/fix-leading-negatives-…

…take-2 Fix issue with leading negatives and space around operator rule
sasstools · Jan 28, 2016 · 0156215 · 0156215
2 parents 42a1fdc + b1ebf3b
commit 0156215
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 10 deletions.
diff --git a/lib/rules/space-around-operator.js b/lib/rules/space-around-operator.js
@@ -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
@@ -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')
@@ -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;
         }
       }
@@ -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,

diff --git a/tests/rules/space-around-operator.js b/tests/rules/space-around-operator.js
@@ -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();
     });
   });
@@ -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();
     });
   });
@@ -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();
     });
   });
@@ -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();
     });
   });

diff --git a/tests/sass/space-around-operator.sass b/tests/sass/space-around-operator.sass
@@ -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)
diff --git a/tests/sass/space-around-operator.scss b/tests/sass/space-around-operator.scss
@@ -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);
+// }