Skip to content

Commit 7e0a0fc

Browse files
addaleaxaduh95
authored andcommitted
test: expand linting rules around assert w literal messages
Expands the existing restrictions around not using literal messages to the `not` variants of `strictEqual` and `deepStrictEqual`, and forbids `assert()` where the second argument is a non-string literal, which almost always is a mis-typed `assert.strictEquals()`. PR-URL: #59147 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
1 parent 96c9dd7 commit 7e0a0fc

11 files changed

+27
-15
lines changed

test/addons/report-fatalerror/test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const ARGS = [
2727
tmpdir.refresh();
2828
const args = ['--report-on-fatalerror', ...ARGS];
2929
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
30-
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
30+
assert.notStrictEqual(child.status, 0);
3131

3232
const reports = helper.findReports(child.pid, tmpdir.path);
3333
assert.strictEqual(reports.length, 1);
@@ -46,7 +46,7 @@ const ARGS = [
4646
// Verify that --report-on-fatalerror is respected when not set.
4747
const args = ARGS;
4848
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
49-
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
49+
assert.notStrictEqual(child.status, 0);
5050
const reports = helper.findReports(child.pid, tmpdir.path);
5151
assert.strictEqual(reports.length, 0);
5252
}

test/eslint.config_partial.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ export default [
3636
selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']",
3737
message: 'Do not use a literal for the third argument of assert.deepStrictEqual()',
3838
},
39+
{
40+
selector: "CallExpression:matches([callee.name='notDeepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']",
41+
message: 'Do not use a literal for the third argument of assert.notDeepStrictEqual()',
42+
},
3943
{
4044
selector: "CallExpression:matches([callee.name='doesNotThrow'], [callee.property.name='doesNotThrow'])",
4145
message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.',
@@ -48,10 +52,18 @@ export default [
4852
selector: "CallExpression:matches([callee.name='rejects'], [callee.property.name='rejects'])[arguments.length<2]",
4953
message: '`assert.rejects()` must be invoked with at least two arguments.',
5054
},
55+
{
56+
selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.2.type='Literal']",
57+
message: 'Do not use a literal for the third argument of assert.notStrictEqual()',
58+
},
5159
{
5260
selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']",
5361
message: 'Do not use a literal for the third argument of assert.strictEqual()',
5462
},
63+
{
64+
selector: "CallExpression[callee.name='assert'][arguments.1.type='Literal']:not([arguments.1.raw=/['\"`].*/])",
65+
message: 'Do not use a non-string literal for the second argument of assert()',
66+
},
5567
{
5668
selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.1.type='Literal']:not([arguments.1.regex])",
5769
message: 'Use an object as second argument of `assert.throws()`.',

test/js-native-api/test_object/test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ assert.strictEqual(newObject.test_string, 'test string');
140140
test_object.Wrap(wrapper);
141141

142142
assert(test_object.Unwrap(wrapper));
143-
assert(wrapper.protoA);
143+
assert.strictEqual(wrapper.protoA, true);
144144
}
145145

146146
{
@@ -155,8 +155,8 @@ assert.strictEqual(newObject.test_string, 'test string');
155155
Object.setPrototypeOf(wrapper, protoB);
156156

157157
assert(test_object.Unwrap(wrapper));
158-
assert(wrapper.protoA, true);
159-
assert(wrapper.protoB, true);
158+
assert.strictEqual(wrapper.protoA, true);
159+
assert.strictEqual(wrapper.protoB, true);
160160
}
161161

162162
{

test/parallel/test-buffer-write-fast.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,5 @@ testFastUtf8Write();
4141

4242
if (common.isDebug) {
4343
const { getV8FastApiCallCount } = internalBinding('debug');
44-
assert(getV8FastApiCallCount('buffer.writeString'), 4);
44+
assert.strictEqual(getV8FastApiCallCount('buffer.writeString'), 4);
4545
}

test/parallel/test-net-listen-exclusive-random-ports.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ if (cluster.isPrimary) {
1616
worker2.on('message', function(port2) {
1717
assert.strictEqual(port2, port2 | 0,
1818
`second worker could not listen on port ${port2}`);
19-
assert.notStrictEqual(port1, port2, 'ports should not be equal');
19+
assert.notStrictEqual(port1, port2);
2020
worker1.kill();
2121
worker2.kill();
2222
});

test/parallel/test-stream-toArray.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ const assert = require('assert');
5959
const ac = new AbortController();
6060
let stream;
6161
assert.rejects(async () => {
62-
stream = Readable.from([1, 2, 3]).map(async (x) => {
62+
stream = Readable.from([1, 2, 3, 4]).map(async (x) => {
6363
if (x === 3) {
6464
await new Promise(() => {}); // Explicitly do not pass signal here
6565
}
@@ -69,8 +69,8 @@ const assert = require('assert');
6969
}, {
7070
name: 'AbortError',
7171
}).then(common.mustCall(() => {
72-
// Only stops toArray, does not destroy the stream
73-
assert(stream.destroyed, false);
72+
// Stops toArray *and* destroys the stream
73+
assert.strictEqual(stream.destroyed, true);
7474
}));
7575
ac.abort();
7676
}

test/report/test-report-fatalerror-oomerror-compact.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const REPORT_FIELDS = [
2525
tmpdir.refresh();
2626
const args = ['--report-on-fatalerror', '--report-compact', ...ARGS];
2727
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
28-
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
28+
assert.notStrictEqual(child.status, 0);
2929

3030
const reports = helper.findReports(child.pid, tmpdir.path);
3131
assert.strictEqual(reports.length, 1);

test/report/test-report-fatalerror-oomerror-directory.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const REPORT_FIELDS = [
2727
const dir = '--report-directory=' + tmpdir.path;
2828
const args = ['--report-on-fatalerror', dir, ...ARGS];
2929
const child = spawnSync(process.execPath, args, { });
30-
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
30+
assert.notStrictEqual(child.status, 0);
3131

3232
const reports = helper.findReports(child.pid, tmpdir.path);
3333
assert.strictEqual(reports.length, 1);

test/report/test-report-fatalerror-oomerror-filename.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const REPORT_FIELDS = [
3030
...ARGS,
3131
];
3232
const child = spawnSync(process.execPath, args, { encoding: 'utf8' });
33-
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
33+
assert.notStrictEqual(child.status, 0);
3434

3535
const reports = helper.findReports(child.pid, tmpdir.path);
3636
assert.strictEqual(reports.length, 0);

test/report/test-report-fatalerror-oomerror-not-set.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const ARGS = [
2020
// Verify that --report-on-fatalerror is respected when not set.
2121
const args = ARGS;
2222
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
23-
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
23+
assert.notStrictEqual(child.status, 0);
2424
const reports = helper.findReports(child.pid, tmpdir.path);
2525
assert.strictEqual(reports.length, 0);
2626
}

0 commit comments

Comments
 (0)