Repository: lesshint/lesshint
Pull request #180 (2016-05-22 15:05:18)

Fixes to column reporting
Fixed a bunch of issues where the column reporting was a bit off or wrong.

I also found a false positive in `spaceAfterPropertyColon` where it would report errors if there were spaces before the colon.


Patch:
diff --git a/lib/linters/attribute_quotes.js b/lib/linters/attribute_quotes.js
index 7b3b219..8efef67 100644
--- a/lib/linters/attribute_quotes.js
+++ b/lib/linters/attribute_quotes.js
@@ -20,8 +20,8 @@ module.exports = {
                 var column;
 
                 if (selector.operator && !selector.quoted) {
-                    column = selector.source.start.column + selector.attribute.length
-                             + selector.operator.length + '['.length;
+                    column = node.source.start.column + selector.source.start.column
+                             + selector.attribute.length + selector.operator.length;
 
                     results.push({
                         column: column,
diff --git a/lib/linters/decimal_zero.js b/lib/linters/decimal_zero.js
index 59ce4c7..2e0323e 100644
--- a/lib/linters/decimal_zero.js
+++ b/lib/linters/decimal_zero.js
@@ -65,7 +65,7 @@ module.exports = {
 
             if (output.type) {
                 results.push({
-                    column: node.prop.length + node.raws.between.length + child.source.start.column,
+                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
                     line: child.source.start.line,
                     message: util.format(self.message, number, output.inclusion, output.type)
                 });
diff --git a/lib/linters/hex_length.js b/lib/linters/hex_length.js
index d621240..81c6bd5 100644
--- a/lib/linters/hex_length.js
+++ b/lib/linters/hex_length.js
@@ -20,16 +20,16 @@ module.exports = {
 
         ast = parser(node.value).parse();
 
-        ast.first.walk(function (node) {
+        ast.first.walk(function (child) {
             var color;
             var canShorten = false;
             var valid = true;
 
-            if (node.type !== 'word' || !node.isColor) {
+            if (child.type !== 'word' || !child.isColor) {
                 return;
             }
 
-            color = node.value;
+            color = child.value;
 
             switch (config.style) {
                 case 'long':
@@ -57,6 +57,7 @@ module.exports = {
 
             if (!valid) {
                 results.push({
+                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
                     message: util.format(self.message, color, config.style)
                 });
             }
diff --git a/lib/linters/hex_notation.js b/lib/linters/hex_notation.js
index ddf1498..19d03a5 100644
--- a/lib/linters/hex_notation.js
+++ b/lib/linters/hex_notation.js
@@ -29,12 +29,12 @@ module.exports = {
             throw new Error('Invalid setting value for hexNotation: ' + config.style);
         }
 
-        ast.first.walk(function (node) {
-            if (node.type !== 'word' || !/^#/.test(node.value)) {
+        ast.first.walk(function (child) {
+            if (child.type !== 'word' || !/^#/.test(child.value)) {
                 return;
             }
 
-            color = node.value;
+            color = child.value;
 
             if (/^#\d+$/.test(color)) {
                 return;
@@ -42,6 +42,7 @@ module.exports = {
 
             if (!styles[config.style].test(color)) {
                 results.push({
+                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
                     message: util.format(self.message, color, config.style)
                 });
             }
diff --git a/lib/linters/hex_validation.js b/lib/linters/hex_validation.js
index 456b2c8..dcec5fb 100644
--- a/lib/linters/hex_validation.js
+++ b/lib/linters/hex_validation.js
@@ -21,14 +21,15 @@ module.exports = {
 
         ast = parser(node.value).parse();
 
-        ast.first.walk(function (node) {
-            if (node.type !== 'word') {
+        ast.first.walk(function (child) {
+            if (child.type !== 'word') {
                 return;
             }
 
-            if (rHex.test(node.value) && !rColor.test(node.value)) {
+            if (rHex.test(child.value) && !rColor.test(child.value)) {
                 results.push({
-                    message: util.format(self.message, node.value)
+                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
+                    message: util.format(self.message, child.value)
                 });
             }
         });
diff --git a/lib/linters/id_selector.js b/lib/linters/id_selector.js
index f7cd900..9115ac5 100644
--- a/lib/linters/id_selector.js
+++ b/lib/linters/id_selector.js
@@ -31,8 +31,8 @@ module.exports = {
                 }
 
                 results.push({
-                    column: id.source.start.column,
-                    line: id.source.start.line,
+                    column: node.source.start.column + id.source.start.column - 1,
+                    line: node.source.start.line,
                     message: self.message
                 });
             });
diff --git a/lib/linters/important_rule.js b/lib/linters/important_rule.js
index fd25809..c20f810 100644
--- a/lib/linters/important_rule.js
+++ b/lib/linters/important_rule.js
@@ -2,25 +2,22 @@
 
 module.exports = {
     name: 'importantRule',
-    nodeTypes: ['rule'],
+    nodeTypes: ['decl'],
     message: '!important should not be used.',
 
     lint: function importantRuleLinter (config, node) {
-        var results = [];
-        var self = this;
+        var pos = 1;
 
-        node.walkDecls(function (decl) {
-            if (decl.important) {
-                results.push({
-                    column: decl.source.start.column,
-                    line: decl.source.start.line,
-                    message: self.message
-                });
+        if (node.important) {
+            if (node.raws.important) {
+                pos = node.raws.important.indexOf('!');
             }
-        });
 
-        if (results.length) {
-            return results;
+            return [{
+                column: node.source.start.column + node.prop.length + node.raws.between.length + node.value.length + pos,
+                line: node.source.start.line,
+                message: this.message
+            }];
         }
     }
 };
diff --git a/lib/linters/property_units.js b/lib/linters/property_units.js
index 2619031..b1532b3 100644
--- a/lib/linters/property_units.js
+++ b/lib/linters/property_units.js
@@ -51,7 +51,7 @@ module.exports = {
             }
 
             results.push({
-                column: node.raws.between.length + property.length + value.source.start.column,
+                column: node.source.start.column + node.raws.between.length + property.length + value.source.start.column - 1,
                 line: value.source.start.line,
                 message: util.format(self.message.unit, unit, property)
             });
diff --git a/lib/linters/qualifying_element.js b/lib/linters/qualifying_element.js
index 6f83695..dffdbe1 100644
--- a/lib/linters/qualifying_element.js
+++ b/lib/linters/qualifying_element.js
@@ -55,7 +55,7 @@ module.exports = {
 
                     if (result) {
                         results.push({
-                            column: result.source.start.column,
+                            column: node.source.start.column + result.source.start.column - 1,
                             line: result.source.start.line,
                             message: util.format(self.message, result.type.charAt(0).toUpperCase() + result.type.substring(1))
                         });
diff --git a/lib/linters/selector_naming.js b/lib/linters/selector_naming.js
index d293812..549135d 100644
--- a/lib/linters/selector_naming.js
+++ b/lib/linters/selector_naming.js
@@ -32,8 +32,8 @@ module.exports = {
                     (config.disallowDash && name.indexOf('-') !== -1)
                 ) {
                     results.push({
-                        column: selector.source.start.column,
-                        line: selector.source.start.line,
+                        column: node.source.start.column + selector.source.start.column - 1,
+                        line: node.source.start.line,
                         message: util.format(self.message, name)
                     });
                 }
diff --git a/lib/linters/single_line_per_selector.js b/lib/linters/single_line_per_selector.js
index f5282e8..1cf9dfc 100644
--- a/lib/linters/single_line_per_selector.js
+++ b/lib/linters/single_line_per_selector.js
@@ -36,8 +36,8 @@ module.exports = {
 
                 if (!valid && tree.nodes.length > 1 && node.selector.indexOf('\n') === -1) {
                     results.push({
-                        column: thing.source.start.column,
-                        line: thing.source.start.line,
+                        column: node.source.start.column + thing.source.start.column - 1,
+                        line: node.source.start.line,
                         message: self.message
                     });
                 }
diff --git a/lib/linters/space_after_property_colon.js b/lib/linters/space_after_property_colon.js
index 567cf30..65dd1ab 100644
--- a/lib/linters/space_after_property_colon.js
+++ b/lib/linters/space_after_property_colon.js
@@ -15,18 +15,18 @@ module.exports = {
         // Find the colon (south of the spleen)
 
         var styles = {
-            'no_space': /^:$/,
-            'one_space': /^:\s$/,
-            'at_least_one_space': /^:\s{1,}$/
+            'no_space': /^\s*:$/,
+            'one_space': /^\s*:\s$/,
+            'at_least_one_space': /^\s*:\s{1,}$/
         };
 
         if (config.style && !styles[config.style]) {
-            throw new Error('Invalid setting value for urlFormat: ' + config.style);
+            throw new Error('Invalid setting value for spaceAfterPropertyColon: ' + config.style);
         }
 
         if (!styles[config.style].test(node.raws.between)) {
             return [{
-                column: node.source.start.column + node.prop.length + 1,
+                column: node.source.start.column + node.prop.length + node.raws.between.indexOf(':') + 1,
                 line: node.source.start.line,
                 message: util.format(this.message[config.style])
             }];
diff --git a/lib/linters/space_after_property_value.js b/lib/linters/space_after_property_value.js
index f52febb..73cdbad 100644
--- a/lib/linters/space_after_property_value.js
+++ b/lib/linters/space_after_property_value.js
@@ -15,7 +15,7 @@ module.exports = {
             case 'no_space':
                 if (node.raws.value && /.*\s$/.test(node.raws.value.raw)) {
                     results.push({
-                        column: node.source.end.column - 1,
+                        column: node.source.start.column + node.prop.length + node.raws.between.length + node.value.length,
                         line: node.source.end.line,
                         message: util.format(self.message, ' not', 'any')
                     });
@@ -25,7 +25,7 @@ module.exports = {
             case 'one_space':
                 if (!node.raws.value) {
                     results.push({
-                        column: node.source.end.column,
+                        column: node.source.start.column + node.prop.length + node.raws.between.length + node.value.length,
                         line: node.source.end.line,
                         message: util.format(self.message, '', 'one')
                     });
diff --git a/lib/linters/string_quotes.js b/lib/linters/string_quotes.js
index 1058b65..1a10182 100644
--- a/lib/linters/string_quotes.js
+++ b/lib/linters/string_quotes.js
@@ -40,7 +40,7 @@ module.exports = {
 
                     results.push({
                         column: column,
-                        line: decl.source.start.line,
+                        line: node.source.start.line,
                         message: util.format(self.message, config.style)
                     });
                 }
@@ -51,12 +51,12 @@ module.exports = {
                     var column;
 
                     if (selector.quoted && !quotes[config.style].test(selector.value)) {
-                        column = selector.source.start.column + selector.attribute.length
-                                 + selector.operator.length + '['.length;
+                        column = node.source.start.column + selector.source.start.column
+                                 + selector.attribute.length + selector.operator.length;
 
                         results.push({
                             column: column,
-                            line: selector.source.start.line,
+                            line: node.source.start.line,
                             message: util.format(self.message, config.style)
                         });
                     }
diff --git a/lib/linters/trailing_semicolon.js b/lib/linters/trailing_semicolon.js
index ced1dcf..01b48c2 100644
--- a/lib/linters/trailing_semicolon.js
+++ b/lib/linters/trailing_semicolon.js
@@ -6,32 +6,25 @@ module.exports = {
     message: 'All property declarations should end with a semicolon.',
 
     lint: function trailingSemicolonLinter (config, node) {
-        var others = 0;
+        var results = [];
+        var self = this;
 
-        if (node.ruleWithoutBody || (node.nodes && !node.nodes.length)) {
-            return;
-        }
-
-        if (!node.raws.semicolon) {
-            node.walk(function (n) {
-                if (n.type !== 'decl') {
-                    others++;
+        if (node.raws.semicolon === false) {
+            node.each(function (child, index) {
+                if (child.type !== 'decl' || index !== node.nodes.length - 1) {
+                    return;
                 }
+
+                results.push({
+                    column: child.source.end.column + 1,
+                    line: child.source.start.line,
+                    message: self.message
+                });
             });
+        }
 
-            /**
-             * If the node contains child nodes that aren't Declarations,
-             * then PostCSS will report raws.semicolon: false. in that case
-             * we should wait until lesshint walks to that Rule/AtRule, and
-             * let the linter handle that one.
-             */
-            if (others === 0) {
-                return [{
-                    column: node.source.start.column,
-                    line: node.source.start.line,
-                    message: this.message
-                }];
-            }
+        if (results.length) {
+            return results;
         }
     }
 };
diff --git a/lib/linters/zero_unit.js b/lib/linters/zero_unit.js
index d8980cb..331f285 100644
--- a/lib/linters/zero_unit.js
+++ b/lib/linters/zero_unit.js
@@ -71,7 +71,7 @@ module.exports = {
 
             if (!valid) {
                 results.push({
-                    column: node.prop.length + node.raws.between.length + child.source.start.column,
+                    column: node.source.start.column + node.prop.length + node.raws.between.length + child.source.start.column - 1,
                     line: node.source.start.line,
                     message: util.format(self.message, config.style === 'keep_unit' ? 'not ' : '')
                 });


Test patch:
diff --git a/test/specs/linters/hex_length.js b/test/specs/linters/hex_length.js
index e9b1b24..6c0afce 100644
--- a/test/specs/linters/hex_length.js
+++ b/test/specs/linters/hex_length.js
@@ -16,6 +16,7 @@ describe('lesshint', function () {
         it('should not allow short hand hex values when "style" is "long"', function () {
             var source = 'color: #ABC;';
             var expected = [{
+                column: 8,
                 message: '#ABC should be written in the long-form format.'
             }];
 
@@ -46,6 +47,7 @@ describe('lesshint', function () {
         it('should not allow longhand hex values when "style" is "short"', function () {
             var source = 'color: #AABBCC;';
             var expected = [{
+                column: 8,
                 message: '#AABBCC should be written in the short-form format.'
             }];
 
@@ -89,6 +91,7 @@ describe('lesshint', function () {
         it('should find hex values in background declarations', function () {
             var source = 'background: url(test.png) no-repeat #AABBCC;';
             var expected = [{
+                column: 37,
                 message: '#AABBCC should be written in the short-form format.'
             }];
 
@@ -106,6 +109,7 @@ describe('lesshint', function () {
         it('should not allow short hand hex values in variables when "style" is "long" (#28)', function () {
             var source = '@color: #ABC;';
             var expected = [{
+                column: 9,
                 message: '#ABC should be written in the long-form format.'
             }];
 
@@ -123,6 +127,7 @@ describe('lesshint', function () {
         it('should not allow longhand hex values in variables when "style" is "short" (#28)', function () {
             var source = '@color: #AABBCC;';
             var expected = [{
+                column: 9,
                 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 0515003..12d1e86 100644
--- a/test/specs/linters/hex_notation.js
+++ b/test/specs/linters/hex_notation.js
@@ -16,6 +16,7 @@ describe('lesshint', function () {
         it('should not allow uppercase hex values when "style" is "lowercase"', function () {
             var source = 'color: #AABBCC;';
             var expected = [{
+                column: 8,
                 message: '#AABBCC should be written in lowercase.'
             }];
 
@@ -46,6 +47,7 @@ describe('lesshint', function () {
         it('should not allow lowercase hex values when "style" is "uppercase"', function () {
             var source = 'color: #aabbcc;';
             var expected = [{
+                column: 8,
                 message: '#aabbcc should be written in uppercase.'
             }];
 
@@ -76,6 +78,7 @@ describe('lesshint', function () {
         it('should find hex values in background declarations', function () {
             var source = 'background: url(test.png) no-repeat #AABBCC;';
             var expected = [{
+                column: 37,
                 message: '#AABBCC should be written in lowercase.'
             }];
 
@@ -93,6 +96,7 @@ describe('lesshint', function () {
         it('should not allow uppercase hex values in variables when "style" is "lowercase" (#28)', function () {
             var source = '@color: #AABBCC;';
             var expected = [{
+                column: 9,
                 message: '#AABBCC should be written in lowercase.'
             }];
 
@@ -110,6 +114,7 @@ describe('lesshint', function () {
         it('should not allow lowercase hex values in variables when "style" is "uppercase" (#28)', function () {
             var source = '@color: #aabbcc;';
             var expected = [{
+                column: 9,
                 message: '#aabbcc should be written in uppercase.'
             }];
 
diff --git a/test/specs/linters/hex_validation.js b/test/specs/linters/hex_validation.js
index dac3342..eadb5f6 100644
--- a/test/specs/linters/hex_validation.js
+++ b/test/specs/linters/hex_validation.js
@@ -26,6 +26,7 @@ describe('lesshint', function () {
         it('should not allow invalid hex values', function () {
             var source = 'color: #AABBC;';
             var expected = [{
+                column: 8,
                 message: 'Hexadecimal color "#AABBC" should be either three or six characters long.'
             }];
 
@@ -39,6 +40,7 @@ describe('lesshint', function () {
         it('should find invalid hex values in background declarations', function () {
             var source = 'background: url(test.png) no-repeat #AABBC;';
             var expected = [{
+                column: 37,
                 message: 'Hexadecimal color "#AABBC" should be either three or six characters long.'
             }];
 
@@ -52,6 +54,7 @@ describe('lesshint', function () {
         it('should not allow invalid hex values in variable declarations (#28)', function () {
             var source = '@color: #AABBC;';
             var expected = [{
+                column: 9,
                 message: 'Hexadecimal color "#AABBC" should be either three or six characters long.'
             }];
 
diff --git a/test/specs/linters/important_rule.js b/test/specs/linters/important_rule.js
index 935eddd..1207d12 100644
--- a/test/specs/linters/important_rule.js
+++ b/test/specs/linters/important_rule.js
@@ -6,7 +6,7 @@ var spec = require('../util.js').setup();
 describe('lesshint', function () {
     describe('#importantRule()', function () {
         it('should have the proper node types', function () {
-            var source = '.foo { color: red; }';
+            var source = 'color: red;';
 
             return spec.parse(source, function (ast) {
                 expect(spec.linter.nodeTypes).to.include(ast.root.first.type);
@@ -14,7 +14,7 @@ describe('lesshint', function () {
         });
 
         it('should not do anything when there is no !important present', function () {
-            var source = '.foo { color: red; }';
+            var source = 'color: red;';
 
             return spec.parse(source, function (ast) {
                 var result = spec.linter.lint({}, ast.root.first);
@@ -24,9 +24,9 @@ describe('lesshint', function () {
         });
 
         it('should not allow !important', function () {
-            var source = '.foo { color: red !important; }';
+            var source = 'color: red !important;';
             var expected = [{
-                column: 8,
+                column: 12,
                 line: 1,
                 message: '!important should not be used.'
             }];
diff --git a/test/specs/linters/qualifying_element.js b/test/specs/linters/qualifying_element.js
index 4748fac..359d89f 100644
--- a/test/specs/linters/qualifying_element.js
+++ b/test/specs/linters/qualifying_element.js
@@ -125,7 +125,7 @@ describe('lesshint', function () {
         it('should check parent selectors. #106', function () {
             var source = 'a { &.active { color: red; } }';
             var expected = [{
-                column: 2,
+                column: 6,
                 line: 1,
                 message: 'Class selectors should not include a qualifying element.'
             }];
diff --git a/test/specs/linters/space_after_property_colon.js b/test/specs/linters/space_after_property_colon.js
index b6d8703..acf45a6 100644
--- a/test/specs/linters/space_after_property_colon.js
+++ b/test/specs/linters/space_after_property_colon.js
@@ -160,6 +160,19 @@ describe('lesshint', function () {
             });
         });
 
+        it('should not care about spaces before the colon', function () {
+            var source = '.foo { color : red; }';
+            var options = {
+                style: 'one_space'
+            };
+
+            return spec.parse(source, function (ast) {
+                var result = spec.linter.lint(options, ast.root.first.first);
+
+                expect(result).to.be.undefined;
+            });
+        });
+
         it('should throw on invalid "style" value', function () {
             var source = '.foo { color:red; }';
             var options = {
diff --git a/test/specs/linters/trailing_semicolon.js b/test/specs/linters/trailing_semicolon.js
index c9076f1..f726c4e 100644
--- a/test/specs/linters/trailing_semicolon.js
+++ b/test/specs/linters/trailing_semicolon.js
@@ -16,7 +16,7 @@ describe('lesshint', function () {
         it('should not allow missing semicolons', function () {
             var source = '.foo { color: red }';
             var expected = [{
-                column: 1,
+                column: 18,
                 line: 1,
                 message: 'All property declarations should end with a semicolon.'
             }];