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: https://github.com/nodejs/node/pull/59147
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
This commit is contained in:
Anna Henningsen 2025-07-23 18:28:06 +02:00 committed by GitHub
parent 075125505e
commit 4b289d047d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 27 additions and 15 deletions

View File

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

View File

@ -36,6 +36,10 @@ export default [
selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']", selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']",
message: 'Do not use a literal for the third argument of assert.deepStrictEqual()', message: 'Do not use a literal for the third argument of assert.deepStrictEqual()',
}, },
{
selector: "CallExpression:matches([callee.name='notDeepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']",
message: 'Do not use a literal for the third argument of assert.notDeepStrictEqual()',
},
{ {
selector: "CallExpression:matches([callee.name='doesNotThrow'], [callee.property.name='doesNotThrow'])", selector: "CallExpression:matches([callee.name='doesNotThrow'], [callee.property.name='doesNotThrow'])",
message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.', message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.',
@ -48,10 +52,18 @@ export default [
selector: "CallExpression:matches([callee.name='rejects'], [callee.property.name='rejects'])[arguments.length<2]", selector: "CallExpression:matches([callee.name='rejects'], [callee.property.name='rejects'])[arguments.length<2]",
message: '`assert.rejects()` must be invoked with at least two arguments.', message: '`assert.rejects()` must be invoked with at least two arguments.',
}, },
{
selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.2.type='Literal']",
message: 'Do not use a literal for the third argument of assert.notStrictEqual()',
},
{ {
selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']", selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']",
message: 'Do not use a literal for the third argument of assert.strictEqual()', message: 'Do not use a literal for the third argument of assert.strictEqual()',
}, },
{
selector: "CallExpression[callee.name='assert'][arguments.1.type='Literal']:not([arguments.1.raw=/['\"`].*/])",
message: 'Do not use a non-string literal for the second argument of assert()',
},
{ {
selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.1.type='Literal']:not([arguments.1.regex])", selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.1.type='Literal']:not([arguments.1.regex])",
message: 'Use an object as second argument of `assert.throws()`.', message: 'Use an object as second argument of `assert.throws()`.',

View File

@ -140,7 +140,7 @@ assert.strictEqual(newObject.test_string, 'test string');
test_object.Wrap(wrapper); test_object.Wrap(wrapper);
assert(test_object.Unwrap(wrapper)); assert(test_object.Unwrap(wrapper));
assert(wrapper.protoA); assert.strictEqual(wrapper.protoA, true);
} }
{ {
@ -155,8 +155,8 @@ assert.strictEqual(newObject.test_string, 'test string');
Object.setPrototypeOf(wrapper, protoB); Object.setPrototypeOf(wrapper, protoB);
assert(test_object.Unwrap(wrapper)); assert(test_object.Unwrap(wrapper));
assert(wrapper.protoA, true); assert.strictEqual(wrapper.protoA, true);
assert(wrapper.protoB, true); assert.strictEqual(wrapper.protoB, true);
} }
{ {

View File

@ -41,5 +41,5 @@ testFastUtf8Write();
if (common.isDebug) { if (common.isDebug) {
const { getV8FastApiCallCount } = internalBinding('debug'); const { getV8FastApiCallCount } = internalBinding('debug');
assert(getV8FastApiCallCount('buffer.writeString'), 4); assert.strictEqual(getV8FastApiCallCount('buffer.writeString'), 4);
} }

View File

@ -16,7 +16,7 @@ if (cluster.isPrimary) {
worker2.on('message', function(port2) { worker2.on('message', function(port2) {
assert.strictEqual(port2, port2 | 0, assert.strictEqual(port2, port2 | 0,
`second worker could not listen on port ${port2}`); `second worker could not listen on port ${port2}`);
assert.notStrictEqual(port1, port2, 'ports should not be equal'); assert.notStrictEqual(port1, port2);
worker1.kill(); worker1.kill();
worker2.kill(); worker2.kill();
}); });

View File

@ -59,7 +59,7 @@ const assert = require('assert');
const ac = new AbortController(); const ac = new AbortController();
let stream; let stream;
assert.rejects(async () => { assert.rejects(async () => {
stream = Readable.from([1, 2, 3]).map(async (x) => { stream = Readable.from([1, 2, 3, 4]).map(async (x) => {
if (x === 3) { if (x === 3) {
await new Promise(() => {}); // Explicitly do not pass signal here await new Promise(() => {}); // Explicitly do not pass signal here
} }
@ -69,8 +69,8 @@ const assert = require('assert');
}, { }, {
name: 'AbortError', name: 'AbortError',
}).then(common.mustCall(() => { }).then(common.mustCall(() => {
// Only stops toArray, does not destroy the stream // Stops toArray *and* destroys the stream
assert(stream.destroyed, false); assert.strictEqual(stream.destroyed, true);
})); }));
ac.abort(); ac.abort();
} }

View File

@ -25,7 +25,7 @@ const REPORT_FIELDS = [
tmpdir.refresh(); tmpdir.refresh();
const args = ['--report-on-fatalerror', '--report-compact', ...ARGS]; const args = ['--report-on-fatalerror', '--report-compact', ...ARGS];
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); assert.notStrictEqual(child.status, 0);
const reports = helper.findReports(child.pid, tmpdir.path); const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 1); assert.strictEqual(reports.length, 1);

View File

@ -27,7 +27,7 @@ const REPORT_FIELDS = [
const dir = '--report-directory=' + tmpdir.path; const dir = '--report-directory=' + tmpdir.path;
const args = ['--report-on-fatalerror', dir, ...ARGS]; const args = ['--report-on-fatalerror', dir, ...ARGS];
const child = spawnSync(process.execPath, args, { }); const child = spawnSync(process.execPath, args, { });
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); assert.notStrictEqual(child.status, 0);
const reports = helper.findReports(child.pid, tmpdir.path); const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 1); assert.strictEqual(reports.length, 1);

View File

@ -30,7 +30,7 @@ const REPORT_FIELDS = [
...ARGS, ...ARGS,
]; ];
const child = spawnSync(process.execPath, args, { encoding: 'utf8' }); const child = spawnSync(process.execPath, args, { encoding: 'utf8' });
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); assert.notStrictEqual(child.status, 0);
const reports = helper.findReports(child.pid, tmpdir.path); const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 0); assert.strictEqual(reports.length, 0);

View File

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

View File

@ -24,7 +24,7 @@ const REPORT_FIELDS = [
tmpdir.refresh(); tmpdir.refresh();
const args = ['--report-on-fatalerror', ...ARGS]; const args = ['--report-on-fatalerror', ...ARGS];
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); assert.notStrictEqual(child.status, 0);
const reports = helper.findReports(child.pid, tmpdir.path); const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 1); assert.strictEqual(reports.length, 1);