Repository: lesshint/lesshint
Pull request #190 (2016-06-08 20:25:00)

Ensure singleLinePerProperty checks each rule on its own
Props @wareczek (idea taken from an old comment that was deleted for some reason).

Ref #177.

wareczek: I figured out that this idea was buggy is some case... but now don't remember it :D
wareczek: Fails on:
```css
.modal-content {
    font-weight: normal;
    height: 20px;

    .close {
        background-color: red;
    }
}
```
jwilsson: Aha, thanks @wareczek!

Do you wanna take another look at it?
jwilsson: I have a patch ready, just wanted to get your opinion on it, the difference from now is that it'll emit two errors for these kind of lines (one for each property):

```less
.modal-content {
  font-weight: normal; height: 20px;
}
```

I think that's OK. If everyone agrees, I'll update the tests to reflect it (since some of them expect one error now where there will be emitted two instead).
wareczek: It's ok :)

Patch:
diff --git a/lib/linters/single_line_per_property.js b/lib/linters/single_line_per_property.js
index 0d87aae..c2767ec 100644
--- a/lib/linters/single_line_per_property.js
+++ b/lib/linters/single_line_per_property.js
@@ -1,10 +1,19 @@
 'use strict';
 
+var hasNewlineBefore = function (node) {
+    return /\n/.test(node.raws.before);
+};
+
 var isValidAfter = function (node, index) {
     var next = node.parent.nodes[index + 1];
 
-    if (!next || (next && next.type === 'comment')) {
-        // Only valid if followed by comment
+    // Nothing after, valid
+    if (!next) {
+        return true;
+    }
+
+    // Followed by comment or next node is on a new line, valid
+    if (next.type === 'comment' || hasNewlineBefore(next)) {
         return true;
     }
 
@@ -32,8 +41,8 @@ module.exports = {
                 return;
             }
 
-            validBefore = /\n/.test(child.raws.before);
-            validAfter = isValidAfter(node, index);
+            validBefore = hasNewlineBefore(child);
+            validAfter = isValidAfter(child, index);
 
             // Check if the closing bracket is on the same line as the last property
             if (index === node.nodes.length - 1 &&
diff --git a/lib/linters/url_quotes.js b/lib/linters/url_quotes.js
index 459eecc..8d60fe3 100644
--- a/lib/linters/url_quotes.js
+++ b/lib/linters/url_quotes.js
@@ -1,19 +1,27 @@
 'use strict';
 
+var parser = require('postcss-values-parser');
+
 module.exports = {
     name: 'urlQuotes',
     nodeTypes: ['decl', 'atrule'],
     message: 'URLs should be enclosed in quotes.',
 
     lint: function urlQuotesLinter (config, node) {
-
-        var parser = require('postcss-values-parser');
-        var ast = parser(node.params || node.value).parse();
-        var uri = ast.first.first;
-        var column;
-        var value;
         var rSingle = /^\'(.+)\'$/;
         var rDouble = /^\"(.+)\"$/;
+        var column;
+        var value;
+        var ast;
+        var uri;
+
+        // Nothing to check, bail
+        if (!node.params && !node.value) {
+            return;
+        }
+
+        ast = parser(node.params || node.value).parse();
+        uri = ast.first.first;
 
         if (uri.type !== 'func' || uri.value !== 'url') {
             return;
diff --git a/lib/linters/zero_unit.js b/lib/linters/zero_unit.js
index 331f285..c89fd70 100644
--- a/lib/linters/zero_unit.js
+++ b/lib/linters/zero_unit.js
@@ -34,7 +34,7 @@ module.exports = {
 
         ast = parser(node.value).parse();
 
-        ast.walk(function (child) {
+        ast.first.each(function (child) {
             var unit = child.unit;
             var valid = true;
 


Test patch:
diff --git a/test/specs/linters/single_line_per_property.js b/test/specs/linters/single_line_per_property.js
index 6da0f34..23ef320 100644
--- a/test/specs/linters/single_line_per_property.js
+++ b/test/specs/linters/single_line_per_property.js
@@ -25,11 +25,18 @@ describe('lesshint', function () {
 
         it('should not allow multiple properties on the same line', function () {
             var source = '.foo {\n color: red; margin-right: 10px; \n}';
-            var expected = [{
-                column: 14,
-                line: 2,
-                message: 'Each property should be on its own line.'
-            }];
+            var expected = [
+                {
+                    column: 2,
+                    line: 2,
+                    message: 'Each property should be on its own line.'
+                },
+                {
+                    column: 14,
+                    line: 2,
+                    message: 'Each property should be on its own line.'
+                }
+            ];
 
             return spec.parse(source, function (ast) {
                 var result = spec.linter.lint({}, ast.root.first);
@@ -215,11 +222,18 @@ describe('lesshint', function () {
 
         it('should not allow single-line, inline comment after multiple definitions in same line', function () {
             var source = '.foo {\n color: red; margin-right: 10px; // inline comment\n}';
-            var expected = [{
-                column: 14,
-                line: 2,
-                message: 'Each property should be on its own line.'
-            }];
+            var expected = [
+                {
+                    column: 2,
+                    line: 2,
+                    message: 'Each property should be on its own line.'
+                },
+                {
+                    column: 14,
+                    line: 2,
+                    message: 'Each property should be on its own line.'
+                }
+            ];
 
             return spec.parse(source, function (ast) {
                 var result = spec.linter.lint({}, ast.root.first);
@@ -230,11 +244,18 @@ describe('lesshint', function () {
 
         it('should not allow one-line, multi-line comment after multiple definitions in same line', function () {
             var source = '.foo {\n color: red; margin-right: 10px; /* inline comment */\n}';
-            var expected = [{
-                column: 14,
-                line: 2,
-                message: 'Each property should be on its own line.'
-            }];
+            var expected = [
+                {
+                    column: 2,
+                    line: 2,
+                    message: 'Each property should be on its own line.'
+                },
+                {
+                    column: 14,
+                    line: 2,
+                    message: 'Each property should be on its own line.'
+                }
+            ];
 
             return spec.parse(source, function (ast) {
                 var result = spec.linter.lint({}, ast.root.first);
@@ -292,5 +313,22 @@ describe('lesshint', function () {
                 expect(result).to.be.undefined;
             });
         });
+
+        it('should check each rule on its own', function () {
+            var source = '';
+
+            source += '.foo {\n';
+            source += '    margin-bottom: 0;\n';
+            source += '}\n';
+            source += '.bar {\n';
+            source += '    height: auto;\n';
+            source += '}';
+
+            return spec.parse(source, function (ast) {
+                var result = spec.linter.lint({}, ast.root.first);
+
+                expect(result).to.be.undefined;
+            });
+        });
     });
 });
diff --git a/test/specs/linters/zero_unit.js b/test/specs/linters/zero_unit.js
index 3e76d51..8a9c594 100644
--- a/test/specs/linters/zero_unit.js
+++ b/test/specs/linters/zero_unit.js
@@ -144,6 +144,19 @@ describe('lesshint', function () {
             });
         });
 
+        it('should not check function arguments', function () {
+            var source = 'color: rgb(0, 0, 0);';
+            var options = {
+                style: 'no_unit'
+            };
+
+            return spec.parse(source, function (ast) {
+                var result = spec.linter.lint(options, ast.root.first);
+
+                expect(result).to.be.undefined;
+            });
+        });
+
         it('should throw on invalid "style" value', function () {
             var source = 'margin-right: 0;';
             var options = {