Repository: lesshint/lesshint
Pull request #443 (2017-12-01 22:42:44)

Rewrite singleLinePerSelector
This comes with a slight behaviour change. It'll no longer report the first selector on a line, only the following ones.

<!--
If you're submitting a PR for a new feature, please open an issue about it first so we can discuss it.
If you're submitting a PR for something else, for example a documentation fix, go right ahead!
-->

**Which issue, if any, does this resolve?**
#346

**Is there anything in this PR that needs extra explaining or should something specific be focused on?**
No, I think it should be pretty straight forward.

I don't think this should be merged until we're ready for a new major version, though.


Patch:
diff --git a/lib/linters/single_line_per_selector.js b/lib/linters/single_line_per_selector.js
index 10020a1..de4a2bf 100644
--- a/lib/linters/single_line_per_selector.js
+++ b/lib/linters/single_line_per_selector.js
@@ -8,46 +8,48 @@ module.exports = {
     message: 'Each selector should be on its own line.',
 
     lint: function singleLinePerSelectorLinter (config, node) {
-        const tree = parseSelector(node);
+        const selectors = parseSelector(node.selector);
         const results = [];
 
-        let valid = true;
-
-        tree.each((selector) => {
-            let reported = false;
-
-            selector.each((thing) => {
-                // This selector is already reported, bail
-                if (reported) {
-                    return;
-                }
-
-                const value = thing.toString().trim();
+        // Just one selector, so nothing to check
+        if (selectors.length === 1) {
+            return;
+        }
 
-                switch (config.style) {
-                    case '18f':
-                        if (value && value.length >= 5) {
-                            valid = false;
+        let longest = 0;
 
-                            return;
-                        }
+        selectors.each((selector) => {
+            selector.each((child) => {
+                const value = child.toString().trim();
+                const length = value.length;
 
-                        break;
-                    default:
-                        valid = false;
+                if (length > longest) {
+                    longest = length;
+                }
 
-                        break;
+                /**
+                 * Ignore 18f style selectors (length less than 5 chars)
+                 * but only when all selectors are less than 5 chars
+                 */
+                if (config.style === '18f' && length < 5 && longest < 5) {
+                    return true;
                 }
 
-                if (!valid && tree.nodes.length > 1 && node.selector.indexOf('\n') === -1) {
-                    reported = true;
+                if (child.spaces.before && child.spaces.before.indexOf('\n') === -1) {
+                    const position = node.positionBy({
+                        word: selector.toString().trim()
+                    });
 
                     results.push({
-                        column: node.source.start.column + thing.source.start.column - 1,
-                        line: node.source.start.line,
+                        column: position.column,
+                        line: position.line,
                         message: this.message
                     });
+
+                    return false;
                 }
+
+                return true;
             });
         });
 


Test patch:
diff --git a/test/specs/linters/single_line_per_selector.js b/test/specs/linters/single_line_per_selector.js
index 92528e5..b896425 100644
--- a/test/specs/linters/single_line_per_selector.js
+++ b/test/specs/linters/single_line_per_selector.js
@@ -26,11 +26,6 @@ describe('lesshint', function () {
         it('should not allow multiple selectors on the same line', function () {
             const source = '.foo, .bar {}';
             const expected = [
-                {
-                    column: 1,
-                    line: 1,
-                    message: 'Each selector should be on its own line.'
-                },
                 {
                     column: 7,
                     line: 1,
@@ -69,7 +64,7 @@ describe('lesshint', function () {
         });
 
         it('should not allow selectors with long names on the same line when "style" is "18f"', function () {
-            const source = '.foobar, .bar {}';
+            const source = '.foobar, .bar, {}';
             const options = {
                 style: '18f'
             };
@@ -97,20 +92,14 @@ describe('lesshint', function () {
             });
         });
 
-        it('should not report the same selector multiple times. #239', function () {
-            const source = '.foo .bar, .bar .foo {}';
-            const expected = [
-                {
-                    column: 1,
-                    line: 1,
-                    message: 'Each selector should be on its own line.'
-                },
-                {
-                    column: 12,
-                    line: 1,
-                    message: 'Each selector should be on its own line.'
-                }
-            ];
+        it('should warn on selectors without an accompanying newline. #346', function () {
+            const source = '.aaa, .bbb,\n.ccc {}';
+
+            const expected = [{
+                column: 7,
+                line: 1,
+                message: 'Each selector should be on its own line.'
+            }];
 
             return spec.parse(source, function (ast) {
                 const result = spec.linter.lint({}, ast.root.first);