Added new rule require-assert-message rule #45
Conversation
|
Coverage after merging require-assert-message into main will be
Coverage Report
|
||||||||||||||||||||||||||||||||||||||
|
Beta Published - Install Command: |
|
|
||
| import rule from './require-assert-message'; | ||
|
|
||
| describe('no-uuid', () => { |
| valid: [ | ||
| { | ||
| code: `import { strict as assert } from 'node:assert'; | ||
| assert.ok(statusCode === StatusCodes.OK, 'Failed to get data.');`, |
There was a problem hiding this comment.
the indenting in the test code is weird, should be two spaces and line up correctly.
| const methodName = node.callee.property.name; | ||
|
|
||
| if (objectName === 'assert' && methodName !== 'ifError') { | ||
| const expectedMessageArgIndex = methodName === 'ok' || methodName === 'strict' ? 1 : 2; |
There was a problem hiding this comment.
i am not convinced this logic is correct. e.g. don't think the assert(value) form is accounted for? Maybe a better approach is to see if the argument named "message" is provided. (all the assert type definitions use message as the argument name). that should be available somehow.
| Error, | ||
| );`, | ||
| errors: [{ message: 'Missing message argument in doesNotReject() method.' }], | ||
| }, |
There was a problem hiding this comment.
I think we need a pass/fail test for every method https://nodejs.org/api/assert.html.
There was a problem hiding this comment.
I was using @typescript-eslint/utils; somehow all the arguments are of type: 'Literal' argument and argument named ‘message’ is not available. Should we follow approach to load all assert methods in runtime from node:assert and get all the arguments length and then report back if arguments length are less than expected?
|
❌ PR review status - reviewer has not approved |
Closes #30
Added rule to report for missing message argument to always be supplied to node:assert methods.