Merge pull request #857 from nottrobin/max-warnings-option

Reimplement max-warnings as a proper option

bin/sass-lint.js

@@ -6,15 +6,7 @@ var program = require('commander'),
     lint = require('../index');
 
 var configPath,
-    ignores,
-    configOptions = {},
-    exitCode = 0;
-
-var tooManyWarnings = function (detects) {
-  var warningCount = lint.warningCount(detects).count;
-
-  return warningCount > 0 && warningCount > program.maxWarnings;
-};
+    configOptions = {};
 
 var detectPattern = function (pattern) {
   var detects;
@@ -25,13 +17,7 @@ var detectPattern = function (pattern) {
     lint.outputResults(detects, configOptions, configPath);
   }
 
-  if (lint.errorCount(detects).count || tooManyWarnings(detects)) {
-    exitCode = 1;
-  }
-
-  if (program.exit) {
-    lint.failOnError(detects);
-  }
+  lint.failOnError(detects, configOptions, configPath);
 };
 
 program
@@ -47,47 +33,32 @@ program
   .option('--max-warnings [integer]', 'Number of warnings to trigger nonzero exit code')
   .parse(process.argv);
 
+// Create "options" and "files" dictionaries if they don't exist
+configOptions.files = configOptions.files || {};
+configOptions.options = configOptions.options || {};
 
 if (program.config && program.config !== true) {
   configPath = program.config;
 }
 
 if (program.ignore && program.ignore !== true) {
-  ignores = program.ignore.split(', ');
-  if (configOptions.hasOwnProperty('files')) {
-    configOptions.files.ignore = ignores;
-  }
-  else {
-    configOptions.files = {
-      'ignore': ignores
-    };
-  }
+  configOptions.files.ignore = program.ignore.split(', ');
 }
 
 if (program.syntax && ['sass', 'scss'].indexOf(program.syntax) > -1) {
   configOptions.syntax = program.syntax;
 }
 
 if (program.format && program.format !== true) {
-  if (configOptions.hasOwnProperty('options')) {
-    configOptions.options.formatter = program.format;
-  }
-  else {
-    configOptions.options = {
-      'formatter': program.format
-    };
-  }
+  configOptions.options.formatter = program.format;
 }
 
 if (program.output && program.output !== true) {
-  if (configOptions.hasOwnProperty('options')) {
-    configOptions.options['output-file'] = program.output;
-  }
-  else {
-    configOptions.options = {
-      'output-file': program.output
-    };
-  }
+  configOptions.options['output-file'] = program.output;
+}
+
+if (program.maxWarnings && program.maxWarnings !== true) {
+  configOptions.options['max-warnings'] = program.maxWarnings;
 }
 
 if (program.args.length === 0) {
@@ -98,7 +69,3 @@ else {
     detectPattern(path);
   });
 }
-
-process.on('exit', function () {
-  process.exit(exitCode); // eslint-disable-line no-process-exit
-});

docs/options/max-warnings.md

@@ -0,0 +1,29 @@
+# Max warnings
+
+An error will be thrown if the total number of warnings exceeds the `max-warnings` setting.
+
+## Examples
+
+This can be set as a command-line option:
+
+``` bash
+$ sass-lint --max-warnings 50
+```
+
+In `.sass-lint.yml`:
+
+``` yaml
+options:
+  max-warnings: 50
+```
+
+Or inside a script:
+
+``` javascript
+var sassLint = require('sass-lint'),
+    config = {options: {'max-warnings': 50}};
+
+results = sassLint.lintFiles('sass/**/*.scss', config)
+sassLint.failOnError(results, config);
+```
+

docs/sass-lint.yml

@@ -9,6 +9,8 @@ options:
   formatter: html
   # Output file instead of logging results
   output-file: 'linters/sass-lint.html'
+  # Raise an error if more than 50 warnings are generated
+  max-warnings: 50
 # File Options
 files:
   include: 'sass/**/*.s+(a|c)ss'

index.js

@@ -2,6 +2,7 @@
 
 var slConfig = require('./lib/config'),
     groot = require('./lib/groot'),
+    exceptions = require('./lib/exceptions'),
     helpers = require('./lib/helpers'),
     slRules = require('./lib/rules'),
     glob = require('glob'),
@@ -284,14 +285,30 @@ sassLint.outputResults = function (results, options, configPath) {
  * 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
+ * @param {object} results - our results object
+ * @param {object} [options] - extra options to use when running failOnError, e.g. max-warnings
+ * @param {string} [configPath] - path to the config file
  * @returns {void}
  */
-sassLint.failOnError = function (results) {
-  var errorCount = this.errorCount(results);
+sassLint.failOnError = function (results, options, configPath) {
+  // Default parameters
+  options = typeof options !== 'undefined' ? options : {};
+  configPath = typeof configPath !== 'undefined' ? configPath : null;
+
+  var errorCount = this.errorCount(results),
+      warningCount = this.warningCount(results),
+      configOptions = this.getConfig(options, configPath).options;
 
   if (errorCount.count > 0) {
-    throw new Error(errorCount.count + ' errors were detected in \n- ' + errorCount.files.join('\n- '));
+    throw new exceptions.SassLintFailureError(errorCount.count + ' errors were detected in \n- ' + errorCount.files.join('\n- '));
+  }
+
+  if (!isNaN(configOptions['max-warnings']) && warningCount.count > configOptions['max-warnings']) {
+    throw new exceptions.MaxWarningsExceededError(
+      'Number of warnings (' + warningCount.count +
+      ') exceeds the allowed maximum of ' + configOptions['max-warnings'] +
+      '.\n'
+    );
   }
 };
 

lib/exceptions.js

@@ -0,0 +1,19 @@
+'use strict';
+
+var util = require('util');
+
+module.exports = {
+  SassLintFailureError: function (message) {
+    Error.captureStackTrace(this, this.constructor);
+    this.name = 'SassLintFailureError';
+    this.message = message;
+  },
+  MaxWarningsExceededError: function (message) {
+    Error.captureStackTrace(this, this.constructor);
+    this.name = 'MaxWarningsExceededError';
+    this.message = message;
+  }
+};
+
+util.inherits(module.exports.SassLintFailureError, Error);
+util.inherits(module.exports.MaxWarningsExceededError, Error);

tests/cli.js

@@ -383,27 +383,27 @@ describe('cli', function () {
 
   });
 
-  it('should exit with exit code 1 when quiet', function (done) {
+  it('should exit with error when quiet', function (done) {
     var command = 'sass-lint -c tests/yml/.error-output.yml tests/cli/cli-error.scss --verbose --no-exit';
 
     exec(command, function (err) {
-      if (err.code === 1) {
+      if (err) {
         return done();
       }
 
-      return done(new Error('Error code not 1'));
+      return done(new Error('No error on exit'));
     });
   });
 
-  it('should exit with exit code 1 when more warnings than --max-warnings', function (done) {
+  it('should exit with error when more warnings than --max-warnings', function (done) {
     var command = 'sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --max-warnings 0';
 
     exec(command, function (err) {
-      if (err && err.code === 1) {
+      if (err) {
         return done();
       }
 
-      return done(new Error('Error code not 1'));
+      return done(new Error('No error on exit'));
     });
   });
 

tests/failures.js

@@ -0,0 +1,90 @@
+'use strict';
+
+var lint = require('../index'),
+    assert = require('assert'),
+    exceptions = require('../lib/exceptions');
+
+describe('failures', function () {
+  it('should raise SassLintFailureError if indentation is set to error', function (done) {
+    assert.throws(
+      function () {
+        var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {rules: {indentation: 2}});  // 14 errors
+        lint.failOnError(results); // Set indentation to error
+      },
+      exceptions.SassLintFailureError
+    );
+    assert.throws(
+      function () {
+        var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {}, 'tests/yml/.indentation-error.yml');  // 14 errors
+        lint.failOnError(results); // Set indentation to error
+      },
+      exceptions.SassLintFailureError
+    );
+
+    done();
+  });
+
+  it('should not raise error if indentation is only set to warn', function (done) {
+    // These should produce 55 warnings and 0 errors
+    var directResults = lint.lintFiles('sass/indentation/indentation-spaces.scss', {rules: {indentation: 1}});
+    var configResults = lint.lintFiles('sass/indentation/indentation-spaces.scss', {}, 'yml/.indentation-warn.yml');
+    lint.failOnError(directResults);
+    lint.failOnError(configResults);
+
+    done();
+  });
+
+  it('should raise MaxWarningsExceededError if warnings exceed `max-warnings` setting', function (done) {
+    assert.throws(
+      function () {
+        var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {});  // 55 warnings
+        lint.failOnError(results, {options: {'max-warnings': 10}});
+      },
+      exceptions.MaxWarningsExceededError
+    );
+    assert.throws(
+      function () {
+        var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {});  // 55 warnings
+        lint.failOnError(results, {}, 'tests/yml/.max-10-warnings.yml');
+      },
+      exceptions.MaxWarningsExceededError
+    );
+
+    done();
+  });
+
+  it('should raise MaxWarningsExceededError if warnings exceed `max-warnings` of zero', function (done) {
+    assert.throws(
+      function () {
+        var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {});  // 55 warnings
+        lint.failOnError(results, {options: {'max-warnings': 0}});
+      },
+      exceptions.MaxWarningsExceededError
+    );
+    assert.throws(
+      function () {
+        var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {});  // 55 warnings
+        lint.failOnError(results, {}, 'tests/yml/.max-0-warnings.yml');
+      },
+      exceptions.MaxWarningsExceededError
+    );
+
+    done();
+  });
+
+  it('should not raise error if warnings do not exceed `max-warnings` setting', function (done) {
+    var results = lint.lintFiles('sass/indentation/indentation-spaces.scss', {});  // 55 warnings
+    lint.failOnError(results, {'max-warnings': 100}); // should succceed
+    lint.failOnError(results, {}, 'yml/.max-100-warnings.yml'); // should succeed
+
+    done();
+  });
+
+  it('should not raise error if no warnings even if `max-warnings` is zero', function (done) {
+    var results = lint.lintFiles('sass/success.scss', {});  // no warnings
+    lint.failOnError(results, {'max-warnings': 0}); // should still succceed
+    lint.failOnError(results, {}, 'yml/.max-0-warnings.yml'); // should still succeed
+
+    done();
+  });
+});

tests/sass/success.scss

@@ -0,0 +1,3 @@
+.one {
+  margin-top: 0;
+}

tests/yml/.indentation-error.yml

@@ -0,0 +1,2 @@
+rules:
+  indentation: 2

tests/yml/.indentation-warn.yml

@@ -0,0 +1,2 @@
+rules:
+  indentation: 1

tests/yml/.max-0-warnings.yml

@@ -0,0 +1,2 @@
+options:
+  max-warnings: 0

tests/yml/.max-10-warnings.yml

@@ -0,0 +1,2 @@
+options:
+  max-warnings: 10

tests/yml/.max-100-warnings.yml

@@ -0,0 +1,2 @@
+options:
+  max-warnings: 100