Last Linter refactor touches

lesshint · Mar 18, 2017 · 94b0b09 · 94b0b09
1 parent cd912c8
commit 94b0b09
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 19 deletions.
diff --git a/lib/lesshint.js b/lib/lesshint.js
@@ -101,11 +101,11 @@ class Lesshint {
 
     checkString (input, checkPath) {
         let result;
-        let linter;
+
+        checkPath = checkPath || '';
 
         try {
-            checkPath = checkPath || '';
-            linter = new Linter(input, checkPath, this.config);
+            const linter = new Linter(input, checkPath, this.config);
 
             result = linter.lint();
         } catch (e) {

diff --git a/lib/linter.js b/lib/linter.js
@@ -132,24 +132,23 @@ class Linter {
                         return;
                     }
 
-                    // Each piece of lint (linter result) can override
-                    // the default properties of the result we're adding
-                    // to the collection
+                    /**
+                     * Each piece of lint (linter result) can override
+                     * the default properties of the result we're adding
+                     * to the collection
+                     */
                     piece = merge({
                         column: column,
                         file: this.filename,
                         fullPath: this.fullPath,
                         line: line,
                         linter: linter.name,
                         message: 'Unknown linting error. Source: ' + linter.name,
+                        position: column + this.linesRaw.slice(0, line - 1).join('').length - 1,
                         severity: options.severity || this.severity.warning,
                         source: line ? this.lines[line - 1] : ''
                     }, piece);
 
-                    piece.position = column
-                        + this.linesRaw.slice(0, line - 1).join('').length
-                        - 1;
-
                     this.results.push(piece);
                 });
             }
@@ -196,15 +195,13 @@ class Linter {
 
     lint () {
         try {
-            // Check top-level root node
             this.checkNode(this.ast.root);
 
-            // Walk through and check each node within (but not including) root
             this.ast.root.walk((node) => {
                 this.checkNode(node);
             });
         } catch (e) {
-            // Just rethrow the exception and let someone else handle it
+            // Just rethrow any exceptions and let someone else handle it
             throw e;
         }
 

diff --git a/test/plugins/noArrayLinter.js b/test/plugins/noArrayLinter.js
@@ -0,0 +1,11 @@
+'use strict';
+
+module.exports = {
+    name: 'noArray',
+    nodeTypes: ['comment'],
+    message: 'Not an array.',
+
+    lint: function noArrayLinter () {
+        return this.message;
+    }
+};
diff --git a/test/specs/linter.js b/test/specs/linter.js
@@ -414,6 +414,20 @@ describe('linter', function () {
             expect(result).to.have.length(0);
         });
 
+        it('should throw if a linter does not return an array', function () {
+            const source = '// foo\n';
+            const config = {
+                linters: ['../test/plugins/noArrayLinter'],
+                noArray: true
+            };
+
+            expect(function () {
+                const linter = new Linter(source, 'test.less', config);
+
+                linter.lint();
+            }).to.throw('Linter "noArray" must return an array.');
+        });
+
         it('should load a custom linter (as a require path)', function () {
             const source = '// boo!\n';
             const testPath = path.resolve(process.cwd(), 'test.less');
@@ -466,4 +480,16 @@ describe('linter', function () {
             expect(result).to.deep.equal(expected);
         });
     });
+
+    describe('configure', function () {
+        it('should throw on invalid "linters" value', function () {
+            const config = {
+                linters: 'invalid'
+            };
+
+            expect(function () {
+                new Linter('', '', config);
+            }).to.throw('Linters should be an array of file paths and/or linters');
+        });
+    });
 });
diff --git a/test/specs/util.js b/test/specs/util.js
@@ -14,11 +14,11 @@ module.exports = {
         delete require.cache[__filename];
 
         const filename = path.join('../../lib/linters/', path.basename(module.parent.filename));
-        const linterObj = new Linter('', '', {});
-        let linter;
+        const linter = new Linter('', '', {});
+        let rule;
 
         try {
-            linter = require(filename);
+            rule = require(filename);
         } catch (e) {
             return {};
         }
@@ -27,10 +27,10 @@ module.exports = {
         const rPseudo = /::?[^ ,:.]+/g;
 
         return {
-            linter: linter,
-            parser: linterObj.parse,
+            linter: rule,
+            parser: linter.parse,
             parse: function (source, callback) {
-                return linterObj.parse(source).then(function (ast) {
+                return linter.parse(source).then(function (ast) {
                     /**
                      * If we're dealing with a regular Rule (or other) node, which isn't
                      * an actual Mixin or AtRule, and its selector contains a pseudo