Skip to content

Commit

Permalink
fix(valid-expect): allow expect(value, "message") (#518)
Browse files Browse the repository at this point in the history
* fix(valid-expect): allow `expect(value, "message")`

* docs: update example

* fix: support template literal
  • Loading branch information
azu committed Aug 26, 2024
1 parent 7a683a7 commit c0d05fd
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 21 deletions.
5 changes: 4 additions & 1 deletion docs/rules/valid-expect.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,16 @@ This rule triggers a warning if `expect` is called with no argument or with more
- Default: `1`

- Enforce `expect` to be called with at most `maxArgs` arguments.
- Exception: `expect(value, "message")` is allowed.

```js
// ✅ good
expect(1).toBe(1)
expect(1, "expect value to be one").toBe(1)
const message = "expect value to be one"
expect(1, `Error Message: ${message}`).toBe(1)
// ❌ bad
expect(1, 2, 3, 4).toBe(1)
```

13 changes: 13 additions & 0 deletions src/rules/valid-expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,19 @@ export default createEslintRule<[
}

if (expect.arguments.length > maxArgs) {
// if expect(value, "message") and expect(value, `${message}`), it is valid usage
// Note: 2nd argument should be string, not a variable in current implementation
if (expect.arguments.length === 2) {
// expect(value, "string literal")
const isSecondArgString = expect.arguments[1].type === AST_NODE_TYPES.Literal &&
typeof expect.arguments[1].value === 'string';
// expect(value, `template literal`)
const isSecondArgTemplateLiteral = expect.arguments[1].type === AST_NODE_TYPES.TemplateLiteral;
if (isSecondArgString || isSecondArgTemplateLiteral) {
return;
}
}

const { start } = expect.arguments[maxArgs].loc
const { end } = expect.arguments[expect.arguments.length - 1].loc

Expand Down
72 changes: 52 additions & 20 deletions tests/valid-expect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ ruleTester.run(RULE_NAME, rule, {
}
});
`,
'expect(value, "message").toBe(1);',
'expect(value, `message`).toBe(1);',
'const message = "message"; expect(value, `${message}`).toBe(1);',
{
code: 'expect(1).toBe(2);',
options: [{ maxArgs: 2 }]
Expand Down Expand Up @@ -150,20 +153,6 @@ ruleTester.run(RULE_NAME, rule, {
}
]
},
{
code: 'expect("something", "else").toEqual("something");',
errors: [
{
endColumn: 28,
column: 21,
messageId: 'tooManyArgs',
data: {
s: '',
amount: 1
}
}
]
},
{
code: 'expect("something", "else", "entirely").toEqual("something");',
options: [{ maxArgs: 2 }],
Expand Down Expand Up @@ -236,18 +225,61 @@ ruleTester.run(RULE_NAME, rule, {
s: 's',
amount: 3
}
},
}
]
},
// expect() 2nd argument to be a string literal
{
code: 'expect("something", true).toEqual("something");',
errors: [
{
endColumn: 28,
endColumn: 26,
column: 21,
messageId: 'tooManyArgs',
data: {
s: '',
amount: 1
}
}
]
},
{
code: 'expect("something", 12).toEqual("something");',
errors: [
{
endColumn: 24,
column: 21,
messageId: 'tooManyArgs',
}
]
},
{
code: 'expect("something", {}).toEqual("something");',
errors: [
{
endColumn: 24,
column: 21,
messageId: 'tooManyArgs',
}
]
},
{
code: 'expect("something", []).toEqual("something");',
errors: [
{
endColumn: 24,
column: 21,
messageId: 'tooManyArgs',
}
]
},
// expect(value, "string literal") is valid, but expect(value, variable) is not
// because the AST does not have type information for variables
{
code: `const message = "message";
expect(1, message).toBe(2);`,
errors: [{
endColumn: 25,
column: 17,
messageId: 'tooManyArgs'
}]
},
{
code: 'expect("something");',
errors: [{ endColumn: 20, column: 1, messageId: 'matcherNotFound' }]
Expand Down

0 comments on commit c0d05fd

Please sign in to comment.