Merge pull request #458 from DanPurdy/feature/error-codes

Update to error codes

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
+});

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- '));
   }
 };
 

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();
     });

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();
+  });
+});

tests/sass/cli-error.scss

@@ -0,0 +1,3 @@
+#cli {
+  color: red;
+}

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