Skip to content

Commit

Permalink
tools: prevent string literals in some assertions
Browse files Browse the repository at this point in the history
String literals provided as the third argument to assert.strictEqual()
or assert.deepStrictEqual() will mask the values that are causing
issues. Use a lint rule to prevent such usage.

Backport-PR-URL: #22912
PR-URL: #22849
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
Trott authored and targos committed Sep 20, 2018
1 parent ff6e4ea commit 8ffcb2d
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ module.exports = {
],
/* eslint-disable max-len */
// If this list is modified, please copy the change to lib/.eslintrc.yaml
// and test/.eslintrc.yaml.
'no-restricted-syntax': [
'error',
{
Expand All @@ -166,6 +167,10 @@ module.exports = {
selector: "CallExpression[callee.object.name='assert'][callee.property.name='rejects'][arguments.length<2]",
message: 'assert.rejects() must be invoked with at least two arguments.',
},
{
selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']",
message: 'Do not use a literal for the third argument of assert.strictEqual()'
},
{
selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])",
message: 'Use an object as second argument of assert.throws()',
Expand Down
4 changes: 4 additions & 0 deletions lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ rules:
no-restricted-syntax:
# Config copied from .eslintrc.js
- error
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']"
message: "Do not use a literal for the third argument of assert.deepStrictEqual()"
- 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='rejects'][arguments.length<2]"
message: "assert.rejects() must be invoked with at least two arguments."
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']"
message: "Do not use a literal for the third argument of assert.strictEqual()"
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])"
message: "Use an object as second argument of assert.throws()"
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]"
Expand Down
4 changes: 4 additions & 0 deletions test/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@ rules:
no-restricted-syntax:
# Config copied from .eslintrc.js
- error
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']"
message: "Do not use a literal for the third argument of assert.deepStrictEqual()"
- 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='rejects'][arguments.length<2]"
message: "assert.rejects() must be invoked with at least two arguments."
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']"
message: "Do not use a literal for the third argument of assert.strictEqual()"
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])"
message: "Use an object as second argument of assert.throws()"
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]"
Expand Down
8 changes: 3 additions & 5 deletions test/parallel/test-timers-same-timeout-wrong-list-deleted.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const handle1 = setTimeout(common.mustCall(function() {

// Make sure our clearTimeout succeeded. One timer finished and
// the other was canceled, so none should be active.
assert.strictEqual(activeTimers.length, 0, 'Timers remain.');
assert.strictEqual(activeTimers.length, 0);
}));
}));
}), 1);
Expand All @@ -53,11 +53,9 @@ const handle1 = setTimeout(common.mustCall(function() {

// Make sure our clearTimeout succeeded. One timer finished and
// the other was canceled, so none should be active.
assert.strictEqual(activeTimers.length, 3,
'There should be 3 timers in the list.');
assert.strictEqual(activeTimers.length, 3);
assert(shortTimer instanceof Timer, 'The shorter timer is not in the list.');
assert.strictEqual(longTimers.length, 2,
'Both longer timers should be in the list.');
assert.strictEqual(longTimers.length, 2);

// When this callback completes, `listOnTimeout` should now look at the
// correct list and refrain from removing the new TIMEOUT list which
Expand Down
6 changes: 2 additions & 4 deletions test/parallel/test-timers-unref-reuse-no-exposed-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ require('../common');
const assert = require('assert');

const timer1 = setTimeout(() => {}, 1).unref();
assert.strictEqual(timer1._handle._list, undefined,
'timer1._handle._list should be undefined');
assert.strictEqual(timer1._handle._list, undefined);

// Check that everything works even if the handle was not re-used.
setTimeout(() => {}, 1);
const timer2 = setTimeout(() => {}, 1).unref();
assert.strictEqual(timer2._handle._list, undefined,
'timer2._handle._list should be undefined');
assert.strictEqual(timer2._handle._list, undefined);

0 comments on commit 8ffcb2d

Please sign in to comment.