Repository: sasstools/sass-lint
Pull request #458 (2015-12-20 23:20:42)

Update to error codes
As discussed in #221 when errors are detected from the CLI there is an inconsistent error code being thrown.

This doesn't fully solve the issues raised there as I believe we should be supporting a range of different error codes but this for now solves the main problem that's been reported.

Took the opportunity to improve comments and structure within the main index file too.

If any errors are detected now the CLI will always exit with a error code of 1 even if errors are supressed with the `--quiet` flag, warnings will still return a code of 0.

@BPScott you may want to double check this meets your requirements?

`DCO 1.1 Signed-off-by: Dan Purdy danjpurdy@gmail.com`


Patch:
diff --git a/bin/sass-lint.js b/bin/sass-lint.js
index afaf34c..33bea9d 100755
--- a/bin/sass-lint.js
+++ b/bin/sass-lint.js
@@ -7,7 +7,8 @@ var program = require('commander'),
 
 var configPath,
     ignores,
-    configOptions = {};
+    configOptions = {},
+    exitCode = 0;
 
 var detectPattern = function (pattern) {
   var detects;
@@ -18,6 +19,10 @@ var detectPattern = function (pattern) {
     lint.outputResults(detects, configOptions, configPath);
   }
 
+  if (lint.errorCount(detects).count) {
+    exitCode = 1;
+  }
+
   if (program.exit) {
     lint.failOnError(detects);
   }
@@ -86,3 +91,7 @@ else {
     detectPattern(path);
   });
 }
+
+process.on('exit', function () {
+  process.exit(exitCode); // eslint-disable-line no-process-exit
+});
diff --git a/index.js b/index.js
index f9a194f..787f074 100644
--- a/index.js
+++ b/index.js
@@ -6,31 +6,94 @@ var slConfig = require('./lib/config'),
     slRules = require('./lib/rules'),
     glob = require('glob'),
     path = require('path'),
-    jsonFormatter = require('eslint/lib/formatters/json'),
     fs = require('fs-extra');
 
-
 var sassLint = function (config) {
   config = require('./lib/config')(config);
   return;
 };
 
+/**
+ * Takes any user specified options and a configPath
+ * which returns a compiled config object
+ *
+ * @param {object} config user specified rules/options passed in
+ * @param {string} configPath path to a config file
+ * @returns {object} the compiled config object
+ */
 sassLint.getConfig = function (config, configPath) {
   return slConfig(config, configPath);
 };
 
-sassLint.resultCount = function (results) {
-  var flagCount = 0,
-      jsonResults = JSON.parse(jsonFormatter(results));
+/**
+ * Parses our results object to count errors and return
+ * paths to files with detected errors.
+ *
+ * @param {object} results our results object
+ * @returns {object} errors object containing the error count and paths for files incl. errors
+ */
+sassLint.errorCount = function (results) {
+  var errors = {
+    count: 0,
+    files: []
+  };
+
+  results.forEach(function (result) {
+    if (result.errorCount) {
+      errors.count += result.errorCount;
+      errors.files.push(result.filePath);
+    }
+  });
 
+  return errors;
+};
 
-  for (var i = 0; i < jsonResults.length; i++) {
-    flagCount += (jsonResults[i].warningCount + jsonResults[i].errorCount);
-  }
+/**
+ * Parses our results object to count warnings and return
+ * paths to files with detected warnings.
+ *
+ * @param {object} results our results object
+ * @returns {object} warnings object containing the error count and paths for files incl. warnings
+ */
+sassLint.warningCount = function (results) {
+  var warnings = {
+    count: 0,
+    files: []
+  };
 
-  return flagCount;
+  results.forEach(function (result) {
+    if (result.warningCount) {
+      warnings.count += result.warningCount;
+      warnings.files.push(result.filePath);
+    }
+  });
+
+  return warnings;
 };
 
+/**
+ * Parses our results object to count warnings and errors and return
+ * a cumulative count of both
+ *
+ * @param {object} results our results object
+ * @returns {int} the cumulative count of errors and warnings detected
+ */
+sassLint.resultCount = function (results) {
+  var warnings = this.warningCount(results),
+      errors = this.errorCount(results);
+
+  return warnings.count + errors.count;
+};
+
+/**
+ * Runs each rule against our AST tree and returns our main object of detected
+ * errors, warnings, messages and filenames.
+ *
+ * @param {object} file file object from fs.readFileSync
+ * @param {object} options user specified rules/options passed in
+ * @param {string} configPath path to a config file
+ * @returns {object} an object containing error & warning counts plus lint messages for each parsed file
+ */
 sassLint.lintText = function (file, options, configPath) {
   var rules = slRules(this.getConfig(options, configPath)),
       ast = {},
@@ -80,6 +143,16 @@ sassLint.lintText = function (file, options, configPath) {
   };
 };
 
+/**
+ * Takes a glob pattern or target string and creates an array of files as targets for
+ * linting taking into account any user specified ignores. For each resulting file sassLint.lintText
+ * is called which returns an object of results for that file which we push to our results object.
+ *
+ * @param {string} files a glob pattern or single file path as a lint target
+ * @param {object} options user specified rules/options passed in
+ * @param {string} configPath path to a config file
+ * @returns {object} results object containing all results
+ */
 sassLint.lintFiles = function (files, options, configPath) {
   var that = this,
       results = [],
@@ -114,7 +187,14 @@ sassLint.lintFiles = function (files, options, configPath) {
   return results;
 };
 
-
+/**
+ * Handles formatting of results using EsLint formatters
+ *
+ * @param {object} results our results object
+ * @param {object} options user specified rules/options passed in
+ * @param {string} configPath path to a config file
+ * @returns {object} results our results object in the user specified format
+ */
 sassLint.format = function (results, options, configPath) {
   var config = this.getConfig(options, configPath),
       format = config.options.formatter.toLowerCase();
@@ -124,6 +204,15 @@ sassLint.format = function (results, options, configPath) {
   return formatted(results);
 };
 
+/**
+ * Handles outputting results whether this be straight to the console/stdout or to a file.
+ * Passes results to the format function to ensure results are output in the chosen format
+ *
+ * @param {object} results our results object
+ * @param {object} options user specified rules/options passed in
+ * @param {string} configPath path to a config file
+ * @returns {object} results our results object
+ */
 sassLint.outputResults = function (results, options, configPath) {
   var config = this.getConfig(options, configPath);
 
@@ -147,16 +236,18 @@ sassLint.outputResults = function (results, options, configPath) {
   return results;
 };
 
+/**
+ * Throws an error if there are any errors detected. The error includes a count of all errors
+ * and a list of all files that include errors.
+ *
+ * @param {object} results our results object
+ * @returns {void}
+ */
 sassLint.failOnError = function (results) {
-  var result,
-      i;
+  var errorCount = this.errorCount(results);
 
-  for (i = 0; i < results.length; i++) {
-    result = results[i];
-
-    if (result.errorCount > 0) {
-      throw new Error(result.errorCount + ' errors detected in ' + result.filePath);
-    }
+  if (errorCount.count > 0) {
+    throw new Error(errorCount.count + ' errors were detected in \n- ' + errorCount.files.join('\n- '));
   }
 };
 


Test patch:
diff --git a/tests/cli.js b/tests/cli.js
index 68bb893..a22f96c 100644
--- a/tests/cli.js
+++ b/tests/cli.js
@@ -308,16 +308,27 @@ describe('cli', function () {
 
   });
 
+  it('should exit with exit code 1 when quiet', function (done) {
+    var command = 'sass-lint -c tests/yml/.error-output.yml tests/sass/cli-error.scss --verbose --no-exit';
+
+    childProcess.exec(command, function (err) {
+      if (err.code === 1) {
+        return done();
+      }
+
+      return done(new Error('Error code not 1'));
+    });
+  });
+
+  /**
+   * We disabled eslints handle callback err rule here as we are deliberately throwing errors that we don't care about
+   */
   it('parse errors should report as a lint error', function (done) {
     var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
 
-    childProcess.exec(command, function (err, stdout) {
+    childProcess.exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err
       var result = JSON.parse(stdout)[0];
 
-      if (err !== null) {
-        return done(new Error('Parse error failure'));
-      }
-
       assert.equal(1, result.errorCount);
       done();
     });
@@ -326,15 +337,11 @@ describe('cli', function () {
   it('parse errors should report as severity 2', function (done) {
     var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
 
-    childProcess.exec(command, function (err, stdout) {
+    childProcess.exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err
       var result = JSON.parse(stdout)[0],
           messages = result.messages[0],
           severity = 2;
 
-      if (err !== null) {
-        return done(new Error('Parse error failure'));
-      }
-
       assert.equal(severity, messages.severity);
       done();
     });
@@ -343,15 +350,11 @@ describe('cli', function () {
   it('parse errors should report the correct message', function (done) {
     var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
 
-    childProcess.exec(command, function (err, stdout) {
+    childProcess.exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err
       var result = JSON.parse(stdout)[0],
           message = result.messages[0].message,
           expected = 'Please check validity of the block starting from line #5';
 
-      if (err !== null) {
-        return done(new Error('Parse error failure'));
-      }
-
       assert.equal(expected, message);
       done();
     });
@@ -360,15 +363,11 @@ describe('cli', function () {
   it('parse errors rule Id should be \'Fatal\'', function (done) {
     var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
 
-    childProcess.exec(command, function (err, stdout) {
+    childProcess.exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err
       var result = JSON.parse(stdout)[0],
           messages = result.messages[0],
           ruleId = 'Fatal';
 
-      if (err !== null) {
-        return done(new Error('Parse error failure'));
-      }
-
       assert.equal(ruleId, messages.ruleId);
       done();
     });
diff --git a/tests/main.js b/tests/main.js
index 02a7af7..4aed580 100644
--- a/tests/main.js
+++ b/tests/main.js
@@ -12,6 +12,48 @@ var lintFile = function lintFile (file, options, cb) {
   cb(results[0]);
 };
 
+var resultsObj = [{
+  filePath: 'app/scss/echo-base/defaults/utilities/_mixins.scss',
+  warningCount: 3,
+  errorCount: 0,
+  messages: [{
+    ruleId: 'no-vendor-prefixes',
+    line: 120,
+    column: 8,
+    message: 'Vendor prefixes should not be used',
+    severity: 1
+  }, {
+    ruleId: 'no-vendor-prefixes',
+    line: 130,
+    column: 8,
+    message: 'Vendor prefixes should not be used',
+    severity: 1
+  }, {
+    ruleId: 'no-vendor-prefixes',
+    line: 140,
+    column: 8,
+    message: 'Vendor prefixes should not be used',
+    severity: 1
+  }]
+}, {
+  filePath: 'app/scss/main.scss',
+  warningCount: 0,
+  errorCount: 2,
+  messages: [{
+    ruleId: 'no-ids',
+    line: 52,
+    column: 1,
+    message: 'ID selectors not allowed',
+    severity: 2
+  }, {
+    ruleId: 'no-ids',
+    line: 57,
+    column: 1,
+    message: 'ID selectors not allowed',
+    severity: 2
+  }]
+}];
+
 describe('sass lint', function () {
   //////////////////////////////
   // Not Error on Empty Files
@@ -71,3 +113,35 @@ describe('sass lint', function () {
     });
   });
 });
+
+describe('sassLint detect counts', function () {
+  //////////////////////////////
+  // Error count
+  //////////////////////////////
+  it('should equal 2 errors', function (done) {
+    var result = lint.errorCount(resultsObj);
+
+    assert.equal(2, result.count);
+    done();
+  });
+
+  //////////////////////////////
+  // Warning count
+  //////////////////////////////
+  it('should equal 3 warnings', function (done) {
+    var result = lint.warningCount(resultsObj);
+
+    assert.equal(3, result.count);
+    done();
+  });
+
+  //////////////////////////////
+  // Result count
+  //////////////////////////////
+  it('should equal 5 overall detects', function (done) {
+    var result = lint.resultCount(resultsObj);
+
+    assert.equal(5, result);
+    done();
+  });
+});
diff --git a/tests/sass/cli-error.scss b/tests/sass/cli-error.scss
new file mode 100644
index 0000000..1bc67ee
--- /dev/null
+++ b/tests/sass/cli-error.scss
@@ -0,0 +1,3 @@
+#cli {
+  color: red;
+}
diff --git a/tests/yml/.error-output.yml b/tests/yml/.error-output.yml
new file mode 100644
index 0000000..6f03b59
--- /dev/null
+++ b/tests/yml/.error-output.yml
@@ -0,0 +1,9 @@
+options:
+  formatter: stylish
+  cache-config: false
+  merge-default-rules: false
+files:
+  include: '**/*.s+(a|c)ss'
+rules:
+  no-ids: 2
+  no-color-literals: 1