Repository: lesshint/lesshint
Pull request #407 (2017-07-20 06:41:32)

Fix spaceBeforeBrace reporting on multi-line selectors
<!--
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?**

#231

**Is there anything in this PR that needs extra explaining or should something specific be focused on?**

This fixed a lot of other column/line position issues too. Found some invalid tests that I removed too. I also removed exceptions for `@import` nodes since they're no longer passed to this rule.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7d71ea3..12043b5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,7 @@
 # Changelog
+## 4.0.2 (2017-07-20)
+* Lockdown `postcss-values-parser` to `~` range. ([d198ed6](https://github.com/lesshint/lesshint/commit/d198ed6b6807464a0d9b443c4e592368bb4ede05))
+
 ## 4.0.1 (2017-07-10)
 * Fixed an issue where incomplete rules would crash `lesshint`. ([31b54c1](https://github.com/lesshint/lesshint/commit/31b54c1ef48840d457ddbfd04c9b08dd4fad936d))
 * Fixed faulty column reporting in `spaceAroundOperator`. ([87cb546](https://github.com/lesshint/lesshint/commit/87cb546697c5424800393b1bc6902c875fac3650))
diff --git a/docs/developer-guide/api/lesshint.md b/docs/developer-guide/api/lesshint.md
index 9db4d91..5acce58 100644
--- a/docs/developer-guide/api/lesshint.md
+++ b/docs/developer-guide/api/lesshint.md
@@ -71,6 +71,8 @@ Setup the options to use. This method must always be called before doing anythin
 
 The `config` argument is optional, but if a object is passed it'll be merged with the defaults.
 
+The merged, final, config object will be returned.
+
 ```js
 const config = {
     emptyRule: false
diff --git a/lib/lesshint.js b/lib/lesshint.js
index 367515a..92a1dec 100644
--- a/lib/lesshint.js
+++ b/lib/lesshint.js
@@ -136,6 +136,8 @@ class Lesshint {
         const defaultConfig = configLoader(__dirname + '/config/defaults.json');
 
         this.config = merge(defaultConfig, config);
+
+        return this.config;
     }
 
     hasAllowedExtension (file) {
diff --git a/lib/linters/space_before_brace.js b/lib/linters/space_before_brace.js
index 3dbd542..acfa2a3 100644
--- a/lib/linters/space_before_brace.js
+++ b/lib/linters/space_before_brace.js
@@ -20,11 +20,8 @@ module.exports = {
             throw new Error(`Invalid setting value for spaceBeforeBrace: ${ config.style }`);
         }
 
-        /**
-         * If the node is a bodiless rule, or it's an import statement, or we
-         * are dealing with an atrule without a body, bail.
-         */
-        if (node.empty || node.name === 'import' ||
+        // If the node is a bodiless rule or atrule, bail
+        if (node.empty ||
            /**
             * Hopefully node.empty will be implemented on AtRule nodes
             * in the future: https://github.com/webschik/postcss-less/issues/55
@@ -34,17 +31,13 @@ module.exports = {
         }
 
         if (!styles[config.style].test(node.raws.between)) {
-            let column = node.source.start.column;
-
-            if (node.type === 'rule') {
-                column += node.selector.length;
-            } else if (node.type === 'atrule') {
-                column += node.name.length + node.raws.afterName.length + node.params.length + 1;
-            }
+            const position = node.positionBy({
+                word: '{'
+            });
 
             return [{
-                column: column,
-                line: node.source.start.line,
+                column: position.column,
+                line: position.line,
                 message: this.message[config.style]
             }];
         }
diff --git a/lib/linters/url_format.js b/lib/linters/url_format.js
index f8322e2..b64e4c2 100644
--- a/lib/linters/url_format.js
+++ b/lib/linters/url_format.js
@@ -9,7 +9,7 @@ module.exports = {
     nodeTypes: ['decl'],
     message: 'URL "%s" should be %s.',
 
-    lint: function urlFormatLinter (config, decl) {
+    lint: function urlFormatLinter (config, node) {
         const style = {
             absolute: isAbsoluteURL,
             relative: (url) => {
@@ -25,7 +25,7 @@ module.exports = {
          * Traverse all child value-nodes
          * https://github.com/lesshint/lesshint/issues/225
          */
-        const ast = parseValue(decl.params || decl.value);
+        const ast = parseValue(node.params || node.value);
         const results = [];
 
         ast.first.nodes.forEach((child) => {
@@ -35,16 +35,16 @@ module.exports = {
 
             // Skip over the 'paren' node; the following node is the value node.
             const uri = child.first.next();
+            const value = uri.value.trim();
 
-            if (!style[config.style](uri.value)) {
-                const column = (decl.raws.between ? decl.raws.between.length : 0) +
-                         decl.source.start.column +
-                         decl.prop.length +
-                         uri.source.start.column - 1;
+            if (!style[config.style](value)) {
+                const position = node.positionBy({
+                    word: value
+                });
 
                 results.push({
-                    column: column,
-                    line: decl.source.start.line,
+                    column: position.column,
+                    line: position.line,
                     message: util.format(this.message, uri.value, config.style)
                 });
             }
diff --git a/lib/linters/url_quotes.js b/lib/linters/url_quotes.js
index db91e68..4e06314 100644
--- a/lib/linters/url_quotes.js
+++ b/lib/linters/url_quotes.js
@@ -57,14 +57,13 @@ module.exports = {
                  * Tracking: https://github.com/lesshint/postcss-values-parser/issues/1
                  */
                 if ((uri.type === 'word' && !(rSingle.test(value) || rDouble.test(value))) && uri.type !== 'string') {
-                    const column = (node.raws.between ? node.raws.between.length : 0) +
-                             node.source.start.column +
-                             (node.prop || node.name).length +
-                             uri.source.start.column - 1;
+                    const position = node.positionBy({
+                        word: value
+                    });
 
                     results.push({
-                        column: column,
-                        line: node.source.start.line,
+                        column: position.column,
+                        line: position.line,
                         message: this.message
                     });
                 }
diff --git a/package.json b/package.json
index a0f9586..f3ea9cb 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "lesshint",
   "description": "A tool to aid you in writing clean and consistent Less.",
-  "version": "4.0.1",
+  "version": "4.0.2",
   "main": "./lib/index.js",
   "author": {
     "name": "Jonathan Wilsson",
@@ -27,7 +27,7 @@
     "postcss": "^6.0.0",
     "postcss-less": "^1.0.0",
     "postcss-selector-parser": "^2.0.0",
-    "postcss-values-parser": "^1.2.1",
+    "postcss-values-parser": "~1.2.2",
     "rcfinder": "^0.1.8",
     "strip-json-comments": "^2.0.0"
   },


Test patch:
diff --git a/test/specs/lesshint.js b/test/specs/lesshint.js
index 8b1172b..5cc1404 100644
--- a/test/specs/lesshint.js
+++ b/test/specs/lesshint.js
@@ -273,6 +273,13 @@ describe('lesshint', function () {
 
             expect(lesshint.config).to.deep.equal(expected);
         });
+
+        it('should return the final config object', function () {
+            const expected = configLoader(path.resolve(process.cwd() + '/lib/config/defaults.json'));
+            const lesshint = new Lesshint();
+
+            expect(lesshint.configure(expected)).to.deep.equal(expected);
+        });
     });
 
     describe('getConfig', function () {
diff --git a/test/specs/linters/space_before_brace.js b/test/specs/linters/space_before_brace.js
index 6152c12..5669631 100644
--- a/test/specs/linters/space_before_brace.js
+++ b/test/specs/linters/space_before_brace.js
@@ -29,7 +29,7 @@ describe('lesshint', function () {
         it('should not allow one space when "style" is "no_space"', function () {
             const source = '.foo {}';
             const expected = [{
-                column: 5,
+                column: 6,
                 line: 1,
                 message: 'Opening curly brace should not be preceded by a space or new line.'
             }];
@@ -48,7 +48,7 @@ describe('lesshint', function () {
         it('should not allow multiple spaces when "style" is "no_space"', function () {
             const source = '.foo  {}';
             const expected = [{
-                column: 5,
+                column: 7,
                 line: 1,
                 message: 'Opening curly brace should not be preceded by a space or new line.'
             }];
@@ -67,8 +67,8 @@ describe('lesshint', function () {
         it('should not allow one new line when "style" is "no_space"', function () {
             const source = '.foo\n{}';
             const expected = [{
-                column: 5,
-                line: 1,
+                column: 1,
+                line: 2,
                 message: 'Opening curly brace should not be preceded by a space or new line.'
             }];
 
@@ -86,8 +86,8 @@ describe('lesshint', function () {
         it('should not allow multiple new lines when "style" is "no_space"', function () {
             const source = '.foo\n\n{}';
             const expected = [{
-                column: 5,
-                line: 1,
+                column: 1,
+                line: 3,
                 message: 'Opening curly brace should not be preceded by a space or new line.'
             }];
 
@@ -169,26 +169,7 @@ describe('lesshint', function () {
         it('should not allow multiple spaces when "style" is "one_space"', function () {
             const source = '.foo  {}';
             const expected = [{
-                column: 5,
-                line: 1,
-                message: 'Opening curly brace should be preceded by one space.'
-            }];
-
-            const options = {
-                style: 'one_space'
-            };
-
-            return spec.parse(source, function (ast) {
-                const result = spec.linter.lint(options, ast.root.first);
-
-                expect(result).to.deep.equal(expected);
-            });
-        });
-
-        it('should not allow multiple spaces when "style" is "one_space"', function () {
-            const source = '.foo  {}';
-            const expected = [{
-                column: 5,
+                column: 7,
                 line: 1,
                 message: 'Opening curly brace should be preceded by one space.'
             }];
@@ -207,8 +188,8 @@ describe('lesshint', function () {
         it('should not allow new line when "style" is "one_space"', function () {
             const source = '.foo, .bar\n{}';
             const expected = [{
-                column: 11,
-                line: 1,
+                column: 1,
+                line: 2,
                 message: 'Opening curly brace should be preceded by one space.'
             }];
 
@@ -226,7 +207,7 @@ describe('lesshint', function () {
         it('should not allow tab when "style" is "one_space"', function () {
             const source = '.foo, .bar\t{}';
             const expected = [{
-                column: 11,
+                column: 12,
                 line: 1,
                 message: 'Opening curly brace should be preceded by one space.'
             }];
@@ -245,7 +226,7 @@ describe('lesshint', function () {
         it('should not allow multiple spaces when multiple simple selectors are used and "style" is "one_space"', function () {
             const source = '.foo, .bar  {}';
             const expected = [{
-                column: 11,
+                column: 13,
                 line: 1,
                 message: 'Opening curly brace should be preceded by one space.'
             }];
@@ -316,8 +297,8 @@ describe('lesshint', function () {
         it('should not allow multiple new lines when "style" is "new_line"', function () {
             const source = '.foo\n\n{}';
             const expected = [{
-                column: 5,
-                line: 1,
+                column: 1,
+                line: 3,
                 message: 'Opening curly brace should be on its own line.'
             }];
 
@@ -335,8 +316,8 @@ describe('lesshint', function () {
         it('should not allow multiple new lines when a selector group is used and "style" is "new_line"', function () {
             const source = '.foo, .bar\n\n{}';
             const expected = [{
-                column: 11,
-                line: 1,
+                column: 1,
+                line: 3,
                 message: 'Opening curly brace should be on its own line.'
             }];
 
@@ -421,8 +402,8 @@ describe('lesshint', function () {
             });
         });
 
-        it('should not check imports', function () {
-            const source = '@import \'lib/colors\';';
+        it('should ignore nodes without a following block', function () {
+            const source = '.foo();';
             const options = {
                 style: 'one_space'
             };
@@ -434,8 +415,8 @@ describe('lesshint', function () {
             });
         });
 
-        it('should ignore nodes without a following block', function () {
-            const source = '.foo();';
+        it('should ignore atrule nodes without a following block', function () {
+            const source = '@foo();';
             const options = {
                 style: 'one_space'
             };
@@ -447,8 +428,15 @@ describe('lesshint', function () {
             });
         });
 
-        it('should ignore atrule nodes without a following block', function () {
-            const source = '@foo();';
+        it('should report the correct line on multi-line selectors (#231)', function () {
+            const source = '.foo,\n.bar  { color: red; }';
+
+            const expected = [{
+                column: 7,
+                line: 2,
+                message: 'Opening curly brace should be preceded by one space.'
+            }];
+
             const options = {
                 style: 'one_space'
             };
@@ -456,7 +444,7 @@ describe('lesshint', function () {
             return spec.parse(source, function (ast) {
                 const result = spec.linter.lint(options, ast.root.first);
 
-                expect(result).to.be.undefined;
+                expect(result).to.deep.equal(expected);
             });
         });
 
diff --git a/test/specs/linters/url_quotes.js b/test/specs/linters/url_quotes.js
index 2a5640b..950de75 100644
--- a/test/specs/linters/url_quotes.js
+++ b/test/specs/linters/url_quotes.js
@@ -156,7 +156,7 @@ describe('lesshint', function () {
         it('should not allow unquoted URLs strings surrounded by spaces (#22)', function () {
             const source = '.foo { background-image: url( img/image.jpg ); }';
             const expected = [{
-                column: 30,
+                column: 31,
                 line: 1,
                 message: 'URLs should be enclosed in quotes.'
             }];