Skip to content

Commit

Permalink
Reimplement max-warnings as a proper option
Browse files Browse the repository at this point in the history
- Remove custom, slightly hacky code for `max-warnings` from `bin/sass-lint.js`
- Tidy up `bin/sass-lint.js` more generally
- Add `max-warnings` as an option to be passed to `failOnError`, either through a config file or through passing options directly
- Add documentation for the max-warnings option
- Create new descriptive exceptions for `failOnError` - MaxWarningsExceededError and SassLintFailureError
- Test the functionality of `failOnError`
  • Loading branch information
nottrobin committed Sep 1, 2016
1 parent 01eb696 commit 24d793c
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 41 deletions.
48 changes: 11 additions & 37 deletions bin/sass-lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,9 @@ 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;
};

var detectPattern = function (pattern) {
var detects;

Expand All @@ -25,12 +18,8 @@ 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);
}
};

Expand All @@ -47,47 +36,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) {
Expand Down
29 changes: 29 additions & 0 deletions docs/options/max-warnings.md
Original file line number Diff line number Diff line change
@@ -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);
```

2 changes: 2 additions & 0 deletions docs/sass-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 10 warnings are generated
max-warnings: 50
# File Options
files:
include: 'sass/**/*.s+(a|c)ss'
Expand Down
25 changes: 21 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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 (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'
);
}
};

Expand Down
19 changes: 19 additions & 0 deletions lib/exceptions.js
Original file line number Diff line number Diff line change
@@ -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);
63 changes: 63 additions & 0 deletions tests/failures.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'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 SassLintFailureError 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}}); // Max 10 warnings
},
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'); // Max 10 warnings
},
exceptions.MaxWarningsExceededError
);

done();
});

it('should not raise MaxWarningsExceededError 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}); // Max 100 warnings, should succceed
lint.failOnError(results, {}, 'yml/.max-100-warnings.yml'); // Max 100 warnings, should succeed

done();
});
});
2 changes: 2 additions & 0 deletions tests/yml/.indentation-error.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
rules:
indentation: 2
2 changes: 2 additions & 0 deletions tests/yml/.indentation-warn.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
rules:
indentation: 1
2 changes: 2 additions & 0 deletions tests/yml/.max-10-warnings.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
options:
max-warnings: 10
2 changes: 2 additions & 0 deletions tests/yml/.max-100-warnings.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
options:
max-warnings: 100

0 comments on commit 24d793c

Please sign in to comment.