Skip to content

Commit eb11876

Browse files
G-RathG-Rath
authored
feat: create prefer-comparison-matcher rule (#1015)
1 parent 1f1f62e commit eb11876

File tree

6 files changed

+416
-1
lines changed

6 files changed

+416
-1
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ installations requiring long-term consistency.
177177
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] |
178178
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | |
179179
| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | |
180+
| [prefer-comparison-matcher](docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | ![fixable][] |
180181
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] |
181182
| [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | ![fixable][] |
182183
| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | |

‎docs/rules/prefer-comparison-matcher.md

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Suggest using the built-in comparison matchers (`prefer-comparison-matcher`)
2+
3+
Jest has a number of built-in matchers for comparing numbers which allow for
4+
more readable tests and error messages if an expectation fails.
5+
6+
## Rule details
7+
8+
This rule checks for comparisons in tests that could be replaced with one of the
9+
following built-in comparison matchers:
10+
11+
- `toBeGreaterThan`
12+
- `toBeGreaterThanOrEqual`
13+
- `toBeLessThan`
14+
- `toBeLessThanOrEqual`
15+
16+
Examples of **incorrect** code for this rule:
17+
18+
```js
19+
expect(x > 5).toBe(true);
20+
expect(x < 7).not.toEqual(true);
21+
expect(x <= y).toStrictEqual(true);
22+
```
23+
24+
Examples of **correct** code for this rule:
25+
26+
```js
27+
expect(x).toBeGreaterThan(5);
28+
expect(x).not.toBeLessThanOrEqual(7);
29+
expect(x).toBeLessThanOrEqual(y);
30+
31+
// special case - see below
32+
expect(x < 'Carl').toBe(true);
33+
```
34+
35+
Note that these matchers only work with numbers and bigints, and that the rule
36+
assumes that any variables on either side of the comparison operator are of one
37+
of those types - this means if you're using the comparison operator with
38+
strings, the fix applied by this rule will result in an error.
39+
40+
```js
41+
expect(myName).toBeGreaterThanOrEqual(theirName); // Matcher error: received value must be a number or bigint
42+
```
43+
44+
The reason for this is that comparing strings with these operators is expected
45+
to be very rare and would mean not being able to have an automatic fixer for
46+
this rule.
47+
48+
If for some reason you are using these operators to compare strings, you can
49+
disable this rule using an inline
50+
[configuration comment](https://eslint.org/docs/user-guide/configuring/rules#disabling-rules):
51+
52+
```js
53+
// eslint-disable-next-line jest/prefer-comparison-matcher
54+
expect(myName > theirName).toBe(true);
55+
```

‎src/__tests__/__snapshots__/rules.test.ts.snap

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Object {
3535
"jest/no-test-prefixes": "error",
3636
"jest/no-test-return-statement": "error",
3737
"jest/prefer-called-with": "error",
38+
"jest/prefer-comparison-matcher": "error",
3839
"jest/prefer-expect-assertions": "error",
3940
"jest/prefer-expect-resolves": "error",
4041
"jest/prefer-hooks-on-top": "error",

‎src/__tests__/rules.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync } from 'fs';
22
import { resolve } from 'path';
33
import plugin from '../';
44

5-
const numberOfRules = 43;
5+
const numberOfRules = 44;
66
const ruleNames = Object.keys(plugin.rules);
77
const deprecatedRules = Object.entries(plugin.rules)
88
.filter(([, rule]) => rule.meta.deprecated)

‎src/rules/__tests__/prefer-comparison-matcher.test.ts

+192
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
import { TSESLint } from '@typescript-eslint/experimental-utils';
2+
import rule from '../prefer-comparison-matcher';
3+
import { espreeParser } from './test-utils';
4+
5+
const ruleTester = new TSESLint.RuleTester({
6+
parser: espreeParser,
7+
parserOptions: {
8+
ecmaVersion: 2015,
9+
},
10+
});
11+
12+
const generateInvalidCases = (
13+
operator: string,
14+
equalityMatcher: string,
15+
preferredMatcher: string,
16+
preferredMatcherWhenNegated: string,
17+
): Array<TSESLint.InvalidTestCase<'useToBeComparison', never>> => {
18+
return [
19+
{
20+
code: `expect(value ${operator} 1).${equalityMatcher}(true);`,
21+
output: `expect(value).${preferredMatcher}(1);`,
22+
errors: [
23+
{
24+
messageId: 'useToBeComparison',
25+
data: { preferredMatcher },
26+
column: 18 + operator.length,
27+
line: 1,
28+
},
29+
],
30+
},
31+
{
32+
code: `expect(value ${operator} 1)['${equalityMatcher}'](true);`,
33+
output: `expect(value).${preferredMatcher}(1);`,
34+
errors: [
35+
{
36+
messageId: 'useToBeComparison',
37+
data: { preferredMatcher },
38+
column: 18 + operator.length,
39+
line: 1,
40+
},
41+
],
42+
},
43+
{
44+
code: `expect(value ${operator} 1).${equalityMatcher}(false);`,
45+
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
46+
errors: [
47+
{
48+
messageId: 'useToBeComparison',
49+
data: { preferredMatcher: preferredMatcherWhenNegated },
50+
column: 18 + operator.length,
51+
line: 1,
52+
},
53+
],
54+
},
55+
{
56+
code: `expect(value ${operator} 1)['${equalityMatcher}'](false);`,
57+
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
58+
errors: [
59+
{
60+
messageId: 'useToBeComparison',
61+
data: { preferredMatcher: preferredMatcherWhenNegated },
62+
column: 18 + operator.length,
63+
line: 1,
64+
},
65+
],
66+
},
67+
{
68+
code: `expect(value ${operator} 1).not.${equalityMatcher}(true);`,
69+
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
70+
errors: [
71+
{
72+
messageId: 'useToBeComparison',
73+
data: { preferredMatcher: preferredMatcherWhenNegated },
74+
column: 18 + operator.length,
75+
line: 1,
76+
},
77+
],
78+
},
79+
{
80+
code: `expect(value ${operator} 1)['not'].${equalityMatcher}(true);`,
81+
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
82+
errors: [
83+
{
84+
messageId: 'useToBeComparison',
85+
data: { preferredMatcher: preferredMatcherWhenNegated },
86+
column: 18 + operator.length,
87+
line: 1,
88+
},
89+
],
90+
},
91+
{
92+
code: `expect(value ${operator} 1).not.${equalityMatcher}(false);`,
93+
output: `expect(value).${preferredMatcher}(1);`,
94+
errors: [
95+
{
96+
messageId: 'useToBeComparison',
97+
data: { preferredMatcher },
98+
column: 18 + operator.length,
99+
line: 1,
100+
},
101+
],
102+
},
103+
];
104+
};
105+
106+
const generateValidStringLiteralCases = (operator: string, matcher: string) => {
107+
return [
108+
['x', "'y'"],
109+
['x', '`y`'],
110+
['x', '`y${z}`'],
111+
].reduce((cases, [a, b]) => [
112+
...cases,
113+
...[
114+
`expect(${a} ${operator} ${b}).${matcher}(true)`,
115+
`expect(${a} ${operator} ${b}).${matcher}(false)`,
116+
`expect(${a} ${operator} ${b}).not.${matcher}(true)`,
117+
`expect(${a} ${operator} ${b}).not.${matcher}(false)`,
118+
`expect(${b} ${operator} ${a}).${matcher}(true)`,
119+
`expect(${b} ${operator} ${a}).${matcher}(false)`,
120+
`expect(${b} ${operator} ${a}).not.${matcher}(true)`,
121+
`expect(${b} ${operator} ${a}).not.${matcher}(false)`,
122+
`expect(${a} ${operator} ${b}).${matcher}(true)`,
123+
`expect(${a} ${operator} ${b}).${matcher}(false)`,
124+
`expect(${a} ${operator} ${b}).not.${matcher}(true)`,
125+
`expect(${a} ${operator} ${b}).not.${matcher}(false)`,
126+
`expect(${b} ${operator} ${a}).${matcher}(true)`,
127+
`expect(${b} ${operator} ${a}).${matcher}(false)`,
128+
`expect(${b} ${operator} ${a}).not.${matcher}(true)`,
129+
`expect(${b} ${operator} ${a}).not.${matcher}(false)`,
130+
`expect(${b} ${operator} ${b}).not.${matcher}(false)`,
131+
],
132+
]);
133+
};
134+
135+
const testComparisonOperator = (
136+
operator: string,
137+
preferredMatcher: string,
138+
preferredMatcherWhenNegated: string,
139+
) => {
140+
ruleTester.run(`prefer-to-be-comparison: ${operator}`, rule, {
141+
valid: [
142+
'expect()',
143+
'expect({}).toStrictEqual({})',
144+
`expect(value).${preferredMatcher}(1);`,
145+
`expect(value).${preferredMatcherWhenNegated}(1);`,
146+
`expect(value).not.${preferredMatcher}(1);`,
147+
`expect(value).not.${preferredMatcherWhenNegated}(1);`,
148+
`expect(value).${preferredMatcher}(1);`,
149+
...['toBe', 'toEqual', 'toStrictEqual'].reduce<string[]>(
150+
(cases, equalityMatcher) => [
151+
...cases,
152+
...generateValidStringLiteralCases(operator, equalityMatcher),
153+
],
154+
[],
155+
),
156+
],
157+
invalid: ['toBe', 'toEqual', 'toStrictEqual'].reduce<
158+
Array<TSESLint.InvalidTestCase<'useToBeComparison', never>>
159+
>(
160+
(cases, equalityMatcher) => [
161+
...cases,
162+
...generateInvalidCases(
163+
operator,
164+
equalityMatcher,
165+
preferredMatcher,
166+
preferredMatcherWhenNegated,
167+
),
168+
],
169+
[],
170+
),
171+
});
172+
};
173+
174+
testComparisonOperator('>', 'toBeGreaterThan', 'toBeLessThanOrEqual');
175+
testComparisonOperator('<', 'toBeLessThan', 'toBeGreaterThanOrEqual');
176+
testComparisonOperator('>=', 'toBeGreaterThanOrEqual', 'toBeLessThan');
177+
testComparisonOperator('<=', 'toBeLessThanOrEqual', 'toBeGreaterThan');
178+
179+
ruleTester.run(`prefer-to-be-comparison`, rule, {
180+
valid: [
181+
'expect()',
182+
'expect({}).toStrictEqual({})',
183+
'expect(a === b).toBe(true)',
184+
'expect(a !== 2).toStrictEqual(true)',
185+
'expect(a === b).not.toEqual(true)',
186+
'expect(a !== "string").toStrictEqual(true)',
187+
'expect(5 != a).toBe(true)',
188+
'expect(a == "string").toBe(true)',
189+
'expect(a == "string").not.toBe(true)',
190+
],
191+
invalid: [],
192+
});

0 commit comments

Comments
 (0)