Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Commit

Permalink
CLI: Support a maxErrors option to limit the number of reported errors (
Browse files Browse the repository at this point in the history
fixes #664, fixes #238)
  • Loading branch information
mdevils committed Oct 10, 2014
1 parent d0150e0 commit bbf85df
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 21 deletions.
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ jscs path[ path[...]] --reporter=./some-dir/my-reporter.js
### `--no-colors`
Clean output without colors.

### '--max-errors'
Set the maximum number of errors to report

### `--help`
Outputs usage information.

Expand Down Expand Up @@ -166,6 +169,20 @@ Values: A single file extension or an Array of file extensions, beginning with a
"fileExtensions": [".js"]
```

### maxErrors

Set the maximum number of errors to report

Type: `Number`

Default: Infinity

#### Example

```js
"maxErrors": 10
```

## Error Suppression

### Inline Comments
Expand Down
1 change: 1 addition & 0 deletions bin/jscs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ program
.option('-p, --preset <preset>', 'preset config')
.option('-r, --reporter <reporter>', 'error reporter, console - default, text, checkstyle, junit, inline')
.option('-v, --verbose', 'adds rule names to the error output')
.option('-m, --max-errors <number>', 'maximum number of errors to report')
.option('', 'Also accepts relative or absolute path to custom reporter')
.option('', 'For instance:')
.option('', '\t ../some-dir/my-reporter.js\t(relative path with extension)')
Expand Down
2 changes: 1 addition & 1 deletion lib/checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var fileExtensions = require('./options/file-extensions');
/**
* Starts Code Style checking process.
*
* @name StringChecker
* @name Checker
*/
var Checker = function() {
StringChecker.apply(this, arguments);
Expand Down
12 changes: 12 additions & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ module.exports = function(program) {
config.preset = program.preset;
}

if (program.maxErrors) {
config.maxErrors = Number(program.maxErrors);
}

if (program.reporter) {
reporterPath = path.resolve(process.cwd(), program.reporter);
returnArgs.reporter = reporterPath;
Expand Down Expand Up @@ -117,6 +121,7 @@ module.exports = function(program) {

checker.checkStdin().then(function(errors) {
reporter([errors]);
handleMaxErrors();

if (!errors.isEmpty()) {
defer.reject(2);
Expand All @@ -134,6 +139,7 @@ module.exports = function(program) {
var errorsCollection = [].concat.apply([], results);

reporter(errorsCollection);
handleMaxErrors();

errorsCollection.forEach(function(errors) {
if (!errors.isEmpty()) {
Expand All @@ -149,4 +155,10 @@ module.exports = function(program) {
}

return returnArgs;

function handleMaxErrors() {
if (checker.maxErrorsExceeded()) {
console.log('Too many errors... Increase `maxErrors` configuration option value to see more.');
}
}
};
9 changes: 9 additions & 0 deletions lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ Errors.prototype = {
return this._errorList.length;
},

/**
* Strips error list to the specified length.
*
* @param {Number} length
*/
stripErrorList: function(length) {
this._errorList.splice(length);
},

/**
* Formats error for further output.
*
Expand Down
14 changes: 14 additions & 0 deletions lib/options/max-errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Limits the total number of errors reported
*
* @param {Object} config
* @param {StringСhecker} instance
*/
module.exports = function(config, instance) {
instance._maxErrors = config.maxErrors;

Object.defineProperty(config, 'maxErrors', {
value: Number(config.maxErrors),
enumerable: false
});
};
55 changes: 39 additions & 16 deletions lib/string-checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var esprima = require('esprima');
var Errors = require('./errors');
var JsFile = require('./js-file');
var preset = require('./options/preset');
var maxErrors = require('./options/max-errors');

/**
* Starts Code Style checking process.
Expand All @@ -14,6 +15,8 @@ var StringChecker = function(verbose) {
this._activeRules = [];
this._config = {};
this._verbose = verbose || false;
this._errorsFound = 0;
this._maxErrorsExceeded = false;
};

StringChecker.prototype = {
Expand Down Expand Up @@ -158,6 +161,8 @@ StringChecker.prototype = {
* @param {Object} config
*/
configure: function(config) {
maxErrors(config, this);

this.throwNonCamelCaseErrorIfNeeded(config);

if (config.preset && !preset.exists(config.preset)) {
Expand Down Expand Up @@ -250,29 +255,47 @@ StringChecker.prototype = {
var file = new JsFile(filename, str, tree);
var errors = new Errors(file, this._verbose);

if (parseError) {
errors.setCurrentRule('parseError');
errors.add(parseError.description, parseError.lineNumber, parseError.column);
if (!this._maxErrorsExceeded) {
if (parseError) {
errors.setCurrentRule('parseError');
errors.add(parseError.description, parseError.lineNumber, parseError.column);

return errors;
}
return errors;
}

this._activeRules.forEach(function(rule) {
this._activeRules.forEach(function(rule) {
// Do not process the rule if it's equals to null (#203)
if (this._config[rule.getOptionName()] !== null) {
errors.setCurrentRule(rule.getOptionName());
rule.check(file, errors);
}
}, this);

// Do not process the rule if it's equals to null (#203)
if (this._config[rule.getOptionName()] !== null) {
errors.setCurrentRule(rule.getOptionName());
rule.check(file, errors);
}
// sort errors list to show errors as they appear in source
errors.getErrorList().sort(function(a, b) {
return (a.line - b.line) || (a.column - b.column);
});

}, this);
if (this._maxErrors !== undefined) {
if (!this._maxErrorsExceeded) {
this._maxErrorsExceeded = this._errorsFound + errors.getErrorCount() > this._maxErrors;
}
errors.stripErrorList(Math.max(0, this._maxErrors - this._errorsFound));
}

// sort errors list to show errors as they appear in source
errors.getErrorList().sort(function(a, b) {
return (a.line - b.line) || (a.column - b.column);
});
this._errorsFound += errors.getErrorCount();
}

return errors;
},

/**
* Returns `true` if error count exceeded `maxErrors` option value.
*
* @returns {Boolean}
*/
maxErrorsExceeded: function() {
return this._maxErrorsExceeded;
}
};

Expand Down
3 changes: 2 additions & 1 deletion test/checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ var sinon = require('sinon');
var Checker = require('../lib/checker');

describe('modules/checker', function() {
var checker = new Checker();
var checker;

beforeEach(function() {
checker = new Checker();
checker.registerDefaultRules();
checker.configure({
disallowKeywords: ['with']
Expand Down
41 changes: 38 additions & 3 deletions test/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ var hasAnsi = require('has-ansi');
var rewire = require('rewire');

var path = require('path');
var exec = require('child_process').exec;

var cli = rewire('../lib/cli');
var startingDir = process.cwd();
Expand Down Expand Up @@ -176,6 +175,10 @@ describe('modules/cli', function() {
sinon.spy(console, 'log');
});

afterEach(function() {
console.log.restore();
});

it('should not display rule names in error output by default', function() {
var result = cli({
args: ['test/data/cli/error.js'],
Expand All @@ -184,7 +187,6 @@ describe('modules/cli', function() {

return result.promise.fail(function() {
assert(console.log.getCall(0).args[0].indexOf('disallowKeywords:') === -1);
console.log.restore();
});
});

Expand All @@ -197,7 +199,6 @@ describe('modules/cli', function() {

return result.promise.fail(function() {
assert(console.log.getCall(0).args[0].indexOf('disallowKeywords:') === 0);
console.log.restore();
});
});
});
Expand Down Expand Up @@ -524,4 +525,38 @@ describe('modules/cli', function() {
});
});
});

describe('maxErrors option', function() {
beforeEach(function() {
sinon.spy(console, 'log');
});

afterEach(function() {
console.log.restore();
});

it('should limit the number of errors reported to the provided amount', function() {
return cli({
maxErrors: 1,
args: ['test/data/cli/error.js'],
config: 'test/data/cli/maxErrors.json'
})
.promise.always(function() {
assert(console.log.getCall(1).args[0].indexOf('1 code style error found.') !== -1);
rAfter();
});
});

it('should display a message indicating that there were more errors', function() {
return cli({
maxErrors: 1,
args: ['test/data/cli/error.js'],
config: 'test/data/cli/maxErrors.json'
})
.promise.always(function() {
assert(console.log.getCall(2).args[0].indexOf('Increase `maxErrors` configuration option') !== -1);
rAfter();
});
});
});
});
4 changes: 4 additions & 0 deletions test/data/cli/maxErrors.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"disallowKeywords": ["with"],
"requireSpaceBeforePostfixUnaryOperators": ["++"]
}
21 changes: 21 additions & 0 deletions test/string-checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,25 @@ describe('modules/string-checker', function() {
assert(error === undefined);
});
});

describe('maxErrors', function() {
beforeEach(function() {
checker.configure({
requireSpaceBeforeBinaryOperators: ['='],
maxErrors: 1
});
});

it('should allow a maximum number of reported errors to be set', function() {
var errors = checker.checkString('var foo=1;\n var bar=2;').getErrorList();
assert(errors.length === 1);
});

it('should not report more than the maximum errors across multiple checks', function() {
var errors = checker.checkString('var foo=1;\n var bar=2;').getErrorList();
var errors2 = checker.checkString('var baz=1;\n var qux=2;').getErrorList();
assert(errors.length === 1);
assert(errors2.length === 0);
});
});
});

2 comments on commit bbf85df

@jzaefferer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got a notification for this merge after the ticket I commented on was closed, a little late, anyway: A default of Infinity defeats the original purpose of this setting, see #238 (comment)

@mrjoelkemp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jzaefferer, we spoke about why the default was problematic here: #664 (comment). We either had to drop the default or require all integrations to set the maxErrors value to "Infinity". We opted for the least breaking approach; though, please feel free to file an issue to discuss the need for a default further.

Please sign in to comment.