Skip to content

Commit

Permalink
tools: add assert.doesNotThrow eslint rule
Browse files Browse the repository at this point in the history
Prohibit the usage of `assert.doesNotThrow()`.

PR-URL: #18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
BridgeAR committed Feb 16, 2018
1 parent 644fdd6 commit 15bb843
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 9 deletions.
5 changes: 4 additions & 1 deletion .eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,11 @@ rules:
no-mixed-spaces-and-tabs: error
no-multiple-empty-lines: [error, {max: 2, maxEOF: 0, maxBOF: 0}]
no-restricted-syntax: [error, {
selector: "CallExpression[callee.object.name='assert'][callee.property.name='doesNotThrow']",
message: "Please replace `assert.doesNotThrow()` and add a comment next to the code instead."
}, {
selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])",
message: "use a regular expression for second argument of assert.throws()"
message: "Use a regular expression for second argument of assert.throws()"
}, {
selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]",
message: "assert.throws() must be invoked with at least two arguments."
Expand Down
1 change: 1 addition & 0 deletions benchmark/assert/throws.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ function main({ n, method }) {
case 'doesNotThrow':
bench.start();
for (i = 0; i < n; ++i) {
// eslint-disable-next-line no-restricted-syntax
assert.doesNotThrow(doesNotThrow);
}
bench.end(n);
Expand Down
3 changes: 3 additions & 0 deletions doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ to the caller.
The following, for instance, will throw the [`TypeError`][] because there is no
matching error type in the assertion:

<!-- eslint-disable no-restricted-syntax -->
```js
assert.doesNotThrow(
() => {
Expand All @@ -353,6 +354,7 @@ assert.doesNotThrow(
However, the following will result in an `AssertionError` with the message
'Got unwanted exception (TypeError)..':

<!-- eslint-disable no-restricted-syntax -->
```js
assert.doesNotThrow(
() => {
Expand All @@ -366,6 +368,7 @@ If an `AssertionError` is thrown and a value is provided for the `message`
parameter, the value of `message` will be appended to the `AssertionError`
message:

<!-- eslint-disable no-restricted-syntax -->
```js
assert.doesNotThrow(
() => {
Expand Down
14 changes: 6 additions & 8 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ assert.throws(() => thrower(TypeError));
}

common.expectsError(
() => assert.doesNotThrow(() => thrower(Error), 'user message'),
() => a.doesNotThrow(() => thrower(Error), 'user message'),
{
type: a.AssertionError,
code: 'ERR_ASSERTION',
Expand All @@ -131,15 +131,15 @@ common.expectsError(
);

common.expectsError(
() => assert.doesNotThrow(() => thrower(Error), 'user message'),
() => a.doesNotThrow(() => thrower(Error), 'user message'),
{
code: 'ERR_ASSERTION',
message: /Got unwanted exception: user message\n\[object Object\]/
}
);

common.expectsError(
() => assert.doesNotThrow(() => thrower(Error)),
() => a.doesNotThrow(() => thrower(Error)),
{
code: 'ERR_ASSERTION',
message: /Got unwanted exception\.\n\[object Object\]/
Expand Down Expand Up @@ -292,7 +292,7 @@ try {

// Verify AssertionError is the result from doesNotThrow with custom Error.
try {
assert.doesNotThrow(() => {
a.doesNotThrow(() => {
throw new TypeError('wrong type');
}, TypeError, rangeError);
} catch (e) {
Expand Down Expand Up @@ -760,7 +760,6 @@ common.expectsError(

errObj.code = '404';
common.expectsError(
// eslint-disable-next-line no-restricted-syntax
() => assert.throws(errFn, errObj),
{
code: 'ERR_ASSERTION',
Expand All @@ -772,7 +771,6 @@ common.expectsError(
errObj.code = 404;
errObj.foo = 'bar';
common.expectsError(
// eslint-disable-next-line no-restricted-syntax
() => assert.throws(errFn, errObj),
{
code: 'ERR_ASSERTION',
Expand All @@ -791,7 +789,7 @@ common.expectsError(
);

common.expectsError(
() => assert.doesNotThrow(() => { throw new Error(); }, { foo: 'bar' }),
() => a.doesNotThrow(() => { throw new Error(); }, { foo: 'bar' }),
{
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
Expand Down Expand Up @@ -822,7 +820,7 @@ common.expectsError(
assert.throws(() => { throw undefined; }, /undefined/);
common.expectsError(
// eslint-disable-next-line no-throw-literal
() => assert.doesNotThrow(() => { throw undefined; }),
() => a.doesNotThrow(() => { throw undefined; }),
{
type: assert.AssertionError,
code: 'ERR_ASSERTION',
Expand Down

0 comments on commit 15bb843

Please sign in to comment.