Added rule to report if assert.rejects and assert.throws doesnt provide an AssertPredicate parameter #63
Conversation
…de an AssertPredicate parameter
There was a problem hiding this comment.
The issue isn't about whether or not rejects/throws has a message/string parameter, it's about the error parameter. It needs to make sure the error/2nd parameter to these functions is of type AssertPredicate. see https://nodejs.org/api/assert.html#assertthrowsfn-error-message and https://nodejs.org/api/assert.html#assertrejectsasyncfn-error-message.
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@checkdigit/eslint-plugin", | |||
| "version": "6.0.1", | |||
| "version": "6.0.2", | |||
There was a problem hiding this comment.
a rule is being added, this should be MINOR
package.json
Outdated
| @@ -34,7 +34,7 @@ | |||
| }, | |||
| "sideEffects": false, | |||
| "devDependencies": { | |||
| "@checkdigit/jest-config": "^6.0.0", | |||
| "@checkdigit/jest-config": "6.0.0", | |||
There was a problem hiding this comment.
It fails saying process.loadEnvFile and we don't have any env file.
There was a problem hiding this comment.
ok I created an issue for this, checkdigit/jest-config#33
… type AssertPredicate
| if ( | ||
| callee.type === 'MemberExpression' && | ||
| callee.object.type === 'Identifier' && | ||
| callee.object.name === 'assert' && |
There was a problem hiding this comment.
i think its ok for now, but we are making an assumption that the import from node:assert will be called "assert", which may not be the case.
| (assertPredicate.type !== 'FunctionExpression' && | ||
| assertPredicate.type !== 'ArrowFunctionExpression' && | ||
| assertPredicate.type !== 'ObjectExpression' && | ||
| assertPredicate.type !== 'Identifier' && |
There was a problem hiding this comment.
again, probably ok to start with, but we're assuming the identifier is of type AssertPredicate, which it may not be.
There was a problem hiding this comment.
a few minor things that don't necessarily need to be addressed now, but the main change is the file naming. require-assert-message-rejects-throws.spec.ts still has message in the name, should be require-assert-predicate.. etc. Also can you confirm if this has been tested against some of our larger code bases?
|
Coverage after merging require-reject-throws-error into main will be
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Beta Published - Install Command: |
Tested in aws-nock. |
|
✅ PR review status - All reviews completed and approved! |
Closes #62