Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: enforce throw new Error() with lint rule #3714

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ rules:

# Custom rules in tools/eslint-rules
require-buffer: 2
new-with-error: [2, "Error", "RangeError", "TypeError", "SyntaxError", "ReferenceError"]


# Global scoped method and vars
globals:
Expand Down
4 changes: 2 additions & 2 deletions test/addons/make-callback/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ assert.strictEqual(42, makeCallback(recv, 'two', 1337));
const target = vm.runInNewContext(`
(function($Object) {
if (Object === $Object)
throw Error('bad');
throw new Error('bad');
return Object;
})
`);
Expand All @@ -55,7 +55,7 @@ const forward = vm.runInNewContext(`
// Runs in outer context.
const endpoint = function($Object) {
if (Object === $Object)
throw Error('bad');
throw new Error('bad');
return Object;
};
assert.strictEqual(Object, makeCallback(process, forward, endpoint));
2 changes: 1 addition & 1 deletion test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,6 @@ testBlockTypeError(assert.doesNotThrow, undefined);

// https://github.com/nodejs/node/issues/3275
assert.throws(() => { throw 'error'; }, err => err === 'error');
assert.throws(() => { throw Error(); }, err => err instanceof Error);
assert.throws(() => { throw new Error(); }, err => err instanceof Error);

console.log('All OK');
2 changes: 1 addition & 1 deletion test/parallel/test-http-mutable-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ function nextTest() {
break;

default:
throw Error('?');
throw new Error('?');
}

response.setEncoding('utf8');
Expand Down
2 changes: 1 addition & 1 deletion test/pummel/test-regress-GH-814.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var timeToQuit = Date.now() + 8e3; //Test during no more than this seconds.
}
}
else {
throw Error("Buffer GC'ed test -> FAIL");
throw new Error("Buffer GC'ed test -> FAIL");
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're here, can you change to single quotes on this line? There is one other line in this file that uses double quotes, but it might not be appropriate to change in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

The lint rules allow for double quotes if it is to avoid escaping a single quote as it is here. I'm inclined to leave it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough. I don't feel strongly about it.

}

if (Date.now() < timeToQuit) {
Expand Down
2 changes: 1 addition & 1 deletion test/pummel/test-regress-GH-814_2.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function tailCB(data) {
console.error('[FAIL]\n DATA -> ');
console.error(data);
console.error('\n');
throw Error('Buffers GC test -> FAIL');
throw new Error('Buffers GC test -> FAIL');
}
}

Expand Down
36 changes: 36 additions & 0 deletions tools/eslint-rules/new-with-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* @fileoverview Require `throw new Error()` rather than `throw Error()`
* @author Rich Trott
*/
'use strict';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = function(context) {

var errorList = context.options.length !== 0 ? context.options : ['Error'];

return {
'ThrowStatement': function(node) {
if (node.argument.type === 'CallExpression' &&
errorList.indexOf(node.argument.callee.name) !== -1) {
context.report(node, 'Use new keyword when throwing.');
}
}
};
};

module.exports.schema = {
'type': 'array',
'items': [
{
'enum': [0, 1, 2]
}
],
'additionalItems': {
'type': 'string'
},
'uniqueItems': true
};