Repository: lesshint/lesshint
Pull request #415 (2017-07-21 07:26:58)

Refactor variableValue position reporting
<!--
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?**
No issue.

**Is there anything in this PR that needs extra explaining or should something specific be focused on?**
No, should be straight forward.


Patch:
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/border_zero.js b/lib/linters/border_zero.js
index 06ed6e5..90e9ad2 100644
--- a/lib/linters/border_zero.js
+++ b/lib/linters/border_zero.js
@@ -16,8 +16,10 @@ module.exports = {
             return;
         }
 
+        const value = node.value;
+
         // Bail if it's an actual border
-        if (node.value !== '0' && node.value !== 'none') {
+        if (value !== '0' && value !== 'none') {
             return;
         }
 
@@ -25,27 +27,29 @@ module.exports = {
 
         switch (config.style) {
             case 'none':
-                if (node.value === '0') {
+                if (value === '0') {
                     message = this.message.none;
                 }
 
                 break;
             case 'zero':
-                if (node.value === 'none') {
+                if (value === 'none') {
                     message = this.message.zero;
                 }
 
                 break;
             default:
-                throw new Error(
-                    'Invalid setting value for borderZero: ' + config.style
-                );
+                throw new Error(`Invalid setting value for borderZero: ${ config.style }`);
         }
 
         if (message) {
+            const position = node.positionBy({
+                word: value
+            });
+
             return [{
-                column: node.source.start.column + node.prop.length + node.raws.between.length,
-                line: node.source.start.line,
+                column: position.column,
+                line: position.line,
                 message: message
             }];
         }
diff --git a/lib/linters/hex_length.js b/lib/linters/hex_length.js
index f679d26..dd6ec67 100644
--- a/lib/linters/hex_length.js
+++ b/lib/linters/hex_length.js
@@ -49,8 +49,13 @@ module.exports = {
             }
 
             if (!valid) {
+                const position = node.positionBy({
+                    word: color
+                });
+
                 results.push({
-                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
+                    column: position.column,
+                    line: position.line,
                     message: util.format(this.message, color, config.style)
                 });
             }
diff --git a/lib/linters/hex_notation.js b/lib/linters/hex_notation.js
index 1a0dc01..39ec075 100644
--- a/lib/linters/hex_notation.js
+++ b/lib/linters/hex_notation.js
@@ -39,8 +39,13 @@ module.exports = {
             }
 
             if (!styles[config.style].test(color)) {
+                const position = node.positionBy({
+                    word: color
+                });
+
                 results.push({
-                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
+                    column: position.column,
+                    line: position.line,
                     message: util.format(this.message, color, config.style)
                 });
             }
diff --git a/lib/linters/hex_validation.js b/lib/linters/hex_validation.js
index 0eb1f35..dba78db 100644
--- a/lib/linters/hex_validation.js
+++ b/lib/linters/hex_validation.js
@@ -24,8 +24,13 @@ module.exports = {
             }
 
             if (child.isHex && !child.isColor) {
+                const position = node.positionBy({
+                    word: child.value
+                });
+
                 results.push({
-                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
+                    column: position.column,
+                    line: position.line,
                     message: util.format(this.message, child.value)
                 });
             }
diff --git a/lib/linters/space_after_property_value.js b/lib/linters/space_after_property_value.js
index 8236631..4454034 100644
--- a/lib/linters/space_after_property_value.js
+++ b/lib/linters/space_after_property_value.js
@@ -8,15 +8,19 @@ module.exports = {
     message: 'Semicolon after property value should%s be preceded by %s space.',
 
     lint: function spaceAfterPropertyValueLinter (config, node) {
-        const nodeEnd = node.source.end || {};
+        const nodeString = node.toString().trimRight();
         const results = [];
 
         switch (config.style) {
             case 'no_space':
                 if (/.*\s$/.test(node.raws.important) || node.raws.value && /.*\s$/.test(node.raws.value.raw)) {
+                    const position = node.positionBy({
+                        index: nodeString.length
+                    });
+
                     results.push({
-                        column: node.source.start.column + node.prop.length + node.raws.between.length + node.value.length,
-                        line: nodeEnd.line,
+                        column: position.column,
+                        line: position.line,
                         message: util.format(this.message, ' not', 'any')
                     });
                 }
@@ -24,9 +28,13 @@ module.exports = {
                 break;
             case 'one_space':
                 if (!/.*\s$/.test(node.raws.important) && !node.raws.value) {
+                    const position = node.positionBy({
+                        index: nodeString.length
+                    });
+
                     results.push({
-                        column: node.source.start.column + node.prop.length + node.raws.between.length + node.value.length,
-                        line: nodeEnd.line,
+                        column: position.column,
+                        line: position.line,
                         message: util.format(this.message, '', 'one')
                     });
                 }
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/universal_selector.js b/lib/linters/universal_selector.js
index 36648c5..c58ef8d 100644
--- a/lib/linters/universal_selector.js
+++ b/lib/linters/universal_selector.js
@@ -12,10 +12,14 @@ module.exports = {
         const results = [];
 
         tree.each((selector) => {
-            selector.walkUniversals((uni) => {
+            selector.walkUniversals((universal) => {
+                const position = node.positionBy({
+                    index: universal.sourceIndex
+                });
+
                 results.push({
-                    column: node.source.start.column + uni.source.start.column - 1,
-                    line: node.source.start.line,
+                    column: position.column,
+                    line: position.line,
                     message: this.message
                 });
             });
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/lib/linters/variable_value.js b/lib/linters/variable_value.js
index 6a308e8..467b3c6 100644
--- a/lib/linters/variable_value.js
+++ b/lib/linters/variable_value.js
@@ -11,11 +11,13 @@ module.exports = {
     lint: function variableValueLinter (config, node) {
         if (config.never && config.never.indexOf(node.prop) !== -1) {
             if (isVariable(node.value)) {
+                const position = node.positionBy({
+                    word: node.value
+                });
+
                 return [{
-                    column: node.source.start.column +
-                            node.prop.length +
-                            node.raws.between.length,
-                    line: node.source.start.line,
+                    column: position.column,
+                    line: position.line,
                     message: util.format(this.message, 'Variable', node.prop)
                 }];
             }
@@ -23,11 +25,13 @@ module.exports = {
 
         if (config.always && config.always.indexOf(node.prop) !== -1) {
             if (!isVariable(node.value)) {
+                const position = node.positionBy({
+                    word: node.value
+                });
+
                 return [{
-                    column: node.source.start.column +
-                            node.prop.length +
-                            node.raws.between.length,
-                    line: node.source.start.line,
+                    column: position.column,
+                    line: position.line,
                     message: util.format(this.message, 'Non variable', node.prop)
                 }];
             }


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/hex_length.js b/test/specs/linters/hex_length.js
index 64552fc..22e4a2d 100644
--- a/test/specs/linters/hex_length.js
+++ b/test/specs/linters/hex_length.js
@@ -17,6 +17,7 @@ describe('lesshint', function () {
             const source = 'color: #ABC;';
             const expected = [{
                 column: 8,
+                line: 1,
                 message: '#ABC should be written in the long-form format.'
             }];
 
@@ -48,6 +49,7 @@ describe('lesshint', function () {
             const source = 'color: #AABBCC;';
             const expected = [{
                 column: 8,
+                line: 1,
                 message: '#AABBCC should be written in the short-form format.'
             }];
 
@@ -92,6 +94,7 @@ describe('lesshint', function () {
             const source = 'background: url(test.png) no-repeat #AABBCC;';
             const expected = [{
                 column: 37,
+                line: 1,
                 message: '#AABBCC should be written in the short-form format.'
             }];
 
@@ -110,6 +113,7 @@ describe('lesshint', function () {
             const source = '@color: #ABC;';
             const expected = [{
                 column: 9,
+                line: 1,
                 message: '#ABC should be written in the long-form format.'
             }];
 
@@ -128,6 +132,7 @@ describe('lesshint', function () {
             const source = '@color: #AABBCC;';
             const expected = [{
                 column: 9,
+                line: 1,
                 message: '#AABBCC should be written in the short-form format.'
             }];
 
diff --git a/test/specs/linters/hex_notation.js b/test/specs/linters/hex_notation.js
index e13fa2b..c8770e7 100644
--- a/test/specs/linters/hex_notation.js
+++ b/test/specs/linters/hex_notation.js
@@ -17,6 +17,7 @@ describe('lesshint', function () {
             const source = 'color: #AABBCC;';
             const expected = [{
                 column: 8,
+                line: 1,
                 message: '#AABBCC should be written in lowercase.'
             }];
 
@@ -48,6 +49,7 @@ describe('lesshint', function () {
             const source = 'color: #aabbcc;';
             const expected = [{
                 column: 8,
+                line: 1,
                 message: '#aabbcc should be written in uppercase.'
             }];
 
@@ -79,6 +81,7 @@ describe('lesshint', function () {
             const source = 'background: url(test.png) no-repeat #AABBCC;';
             const expected = [{
                 column: 37,
+                line: 1,
                 message: '#AABBCC should be written in lowercase.'
             }];
 
@@ -97,6 +100,7 @@ describe('lesshint', function () {
             const source = '@color: #AABBCC;';
             const expected = [{
                 column: 9,
+                line: 1,
                 message: '#AABBCC should be written in lowercase.'
             }];
 
@@ -115,6 +119,7 @@ describe('lesshint', function () {
             const source = '@color: #aabbcc;';
             const expected = [{
                 column: 9,
+                line: 1,
                 message: '#aabbcc should be written in uppercase.'
             }];
 
diff --git a/test/specs/linters/hex_validation.js b/test/specs/linters/hex_validation.js
index 86f0940..c9e9ac0 100644
--- a/test/specs/linters/hex_validation.js
+++ b/test/specs/linters/hex_validation.js
@@ -27,6 +27,7 @@ describe('lesshint', function () {
             const source = 'color: #AABBC;';
             const expected = [{
                 column: 8,
+                line: 1,
                 message: 'Hexadecimal color "#AABBC" should be either three or six characters long.'
             }];
 
@@ -41,6 +42,7 @@ describe('lesshint', function () {
             const source = 'background: url(test.png) no-repeat #AABBC;';
             const expected = [{
                 column: 37,
+                line: 1,
                 message: 'Hexadecimal color "#AABBC" should be either three or six characters long.'
             }];
 
@@ -55,6 +57,7 @@ describe('lesshint', function () {
             const source = '@color: #AABBC;';
             const expected = [{
                 column: 9,
+                line: 1,
                 message: 'Hexadecimal color "#AABBC" should be either three or six characters long.'
             }];
 
diff --git a/test/specs/linters/space_after_property_value.js b/test/specs/linters/space_after_property_value.js
index c023596..07ba96a 100644
--- a/test/specs/linters/space_after_property_value.js
+++ b/test/specs/linters/space_after_property_value.js
@@ -61,7 +61,7 @@ describe('lesshint', function () {
         it('should not allow any space after "!important" when "style" is "no_space"', function () {
             const source = '.foo { color: red !important ; }';
             const expected = [{
-                column: 18,
+                column: 29,
                 line: 1,
                 message: 'Semicolon after property value should not be preceded by any space.'
             }];
@@ -125,7 +125,7 @@ describe('lesshint', function () {
         it('should not allow a missing space after "!important" when "style" is "one_space"', function () {
             const source = '.foo { color: red !important; }';
             const expected = [{
-                column: 18,
+                column: 29,
                 line: 1,
                 message: 'Semicolon after property value should be preceded by one space.'
             }];
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.'
             }];