Skip to content

Commit eb11876

Browse files
G-RathG-Rath
authoredJan 15, 2022
feat: create prefer-comparison-matcher rule (#1015)
·
v28.11.0v25.6.0
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+
});

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

+166
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import {
2+
AST_NODE_TYPES,
3+
TSESTree,
4+
} from '@typescript-eslint/experimental-utils';
5+
import {
6+
MaybeTypeCast,
7+
ParsedEqualityMatcherCall,
8+
ParsedExpectMatcher,
9+
createRule,
10+
followTypeAssertionChain,
11+
isExpectCall,
12+
isParsedEqualityMatcherCall,
13+
isStringNode,
14+
parseExpectCall,
15+
} from './utils';
16+
17+
const isBooleanLiteral = (
18+
node: TSESTree.Node,
19+
): node is TSESTree.BooleanLiteral =>
20+
node.type === AST_NODE_TYPES.Literal && typeof node.value === 'boolean';
21+
22+
type ParsedBooleanEqualityMatcherCall = ParsedEqualityMatcherCall<
23+
MaybeTypeCast<TSESTree.BooleanLiteral>
24+
>;
25+
26+
/**
27+
* Checks if the given `ParsedExpectMatcher` is a call to one of the equality matchers,
28+
* with a boolean literal as the sole argument.
29+
*
30+
* @example javascript
31+
* toBe(true);
32+
* toEqual(false);
33+
*
34+
* @param {ParsedExpectMatcher} matcher
35+
*
36+
* @return {matcher is ParsedBooleanEqualityMatcher}
37+
*/
38+
const isBooleanEqualityMatcher = (
39+
matcher: ParsedExpectMatcher,
40+
): matcher is ParsedBooleanEqualityMatcherCall =>
41+
isParsedEqualityMatcherCall(matcher) &&
42+
isBooleanLiteral(followTypeAssertionChain(matcher.arguments[0]));
43+
44+
const isString = (node: TSESTree.Node) => {
45+
return isStringNode(node) || node.type === AST_NODE_TYPES.TemplateLiteral;
46+
};
47+
48+
const isComparingToString = (expression: TSESTree.BinaryExpression) => {
49+
return isString(expression.left) || isString(expression.right);
50+
};
51+
52+
const invertOperator = (operator: string) => {
53+
switch (operator) {
54+
case '>':
55+
return '<=';
56+
case '<':
57+
return '>=';
58+
case '>=':
59+
return '<';
60+
case '<=':
61+
return '>';
62+
}
63+
64+
return null;
65+
};
66+
67+
const determineMatcher = (
68+
operator: string,
69+
negated: boolean,
70+
): string | null => {
71+
const op = negated ? invertOperator(operator) : operator;
72+
73+
switch (op) {
74+
case '>':
75+
return 'toBeGreaterThan';
76+
case '<':
77+
return 'toBeLessThan';
78+
case '>=':
79+
return 'toBeGreaterThanOrEqual';
80+
case '<=':
81+
return 'toBeLessThanOrEqual';
82+
}
83+
84+
return null;
85+
};
86+
87+
export default createRule({
88+
name: __filename,
89+
meta: {
90+
docs: {
91+
category: 'Best Practices',
92+
description: 'Suggest using the built-in comparison matchers',
93+
recommended: false,
94+
},
95+
messages: {
96+
useToBeComparison: 'Prefer using `{{ preferredMatcher }}` instead',
97+
},
98+
fixable: 'code',
99+
type: 'suggestion',
100+
schema: [],
101+
},
102+
defaultOptions: [],
103+
create(context) {
104+
return {
105+
CallExpression(node) {
106+
if (!isExpectCall(node)) {
107+
return;
108+
}
109+
110+
const {
111+
expect: {
112+
arguments: [comparison],
113+
range: [, expectCallEnd],
114+
},
115+
matcher,
116+
modifier,
117+
} = parseExpectCall(node);
118+
119+
if (
120+
!matcher ||
121+
comparison?.type !== AST_NODE_TYPES.BinaryExpression ||
122+
isComparingToString(comparison) ||
123+
!isBooleanEqualityMatcher(matcher)
124+
) {
125+
return;
126+
}
127+
128+
const preferredMatcher = determineMatcher(
129+
comparison.operator,
130+
followTypeAssertionChain(matcher.arguments[0]).value === !!modifier,
131+
);
132+
133+
if (!preferredMatcher) {
134+
return;
135+
}
136+
137+
context.report({
138+
fix(fixer) {
139+
const sourceCode = context.getSourceCode();
140+
141+
return [
142+
// replace the comparison argument with the left-hand side of the comparison
143+
fixer.replaceText(
144+
comparison,
145+
sourceCode.getText(comparison.left),
146+
),
147+
// replace the current matcher & modifier with the preferred matcher
148+
fixer.replaceTextRange(
149+
[expectCallEnd, matcher.node.range[1]],
150+
`.${preferredMatcher}`,
151+
),
152+
// replace the matcher argument with the right-hand side of the comparison
153+
fixer.replaceText(
154+
matcher.arguments[0],
155+
sourceCode.getText(comparison.right),
156+
),
157+
];
158+
},
159+
messageId: 'useToBeComparison',
160+
data: { preferredMatcher },
161+
node: (modifier || matcher).node.property,
162+
});
163+
},
164+
};
165+
},
166+
});

0 commit comments

Comments
 (0)
Please sign in to comment.