Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create prefer-comparison-matcher rule #1015

Merged
merged 1 commit into from
Jan 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ installations requiring long-term consistency.
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] |
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | |
| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | |
| [prefer-comparison-matcher](docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | ![fixable][] |
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] |
| [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | ![fixable][] |
| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | |
Expand Down
55 changes: 55 additions & 0 deletions docs/rules/prefer-comparison-matcher.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Suggest using the built-in comparison matchers (`prefer-comparison-matcher`)

Jest has a number of built-in matchers for comparing numbers which allow for
more readable tests and error messages if an expectation fails.

## Rule details

This rule checks for comparisons in tests that could be replaced with one of the
following built-in comparison matchers:

- `toBeGreaterThan`
- `toBeGreaterThanOrEqual`
- `toBeLessThan`
- `toBeLessThanOrEqual`

Examples of **incorrect** code for this rule:

```js
expect(x > 5).toBe(true);
expect(x < 7).not.toEqual(true);
expect(x <= y).toStrictEqual(true);
```

Examples of **correct** code for this rule:

```js
expect(x).toBeGreaterThan(5);
expect(x).not.toBeLessThanOrEqual(7);
expect(x).toBeLessThanOrEqual(y);

// special case - see below
expect(x < 'Carl').toBe(true);
```

Note that these matchers only work with numbers and bigints, and that the rule
assumes that any variables on either side of the comparison operator are of one
of those types - this means if you're using the comparison operator with
strings, the fix applied by this rule will result in an error.

```js
expect(myName).toBeGreaterThanOrEqual(theirName); // Matcher error: received value must be a number or bigint
```

The reason for this is that comparing strings with these operators is expected
to be very rare and would mean not being able to have an automatic fixer for
this rule.

If for some reason you are using these operators to compare strings, you can
disable this rule using an inline
[configuration comment](https://eslint.org/docs/user-guide/configuring/rules#disabling-rules):

```js
// eslint-disable-next-line jest/prefer-comparison-matcher
expect(myName > theirName).toBe(true);
```
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/rules.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Object {
"jest/no-test-prefixes": "error",
"jest/no-test-return-statement": "error",
"jest/prefer-called-with": "error",
"jest/prefer-comparison-matcher": "error",
"jest/prefer-expect-assertions": "error",
"jest/prefer-expect-resolves": "error",
"jest/prefer-hooks-on-top": "error",
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { existsSync } from 'fs';
import { resolve } from 'path';
import plugin from '../';

const numberOfRules = 43;
const numberOfRules = 44;
const ruleNames = Object.keys(plugin.rules);
const deprecatedRules = Object.entries(plugin.rules)
.filter(([, rule]) => rule.meta.deprecated)
Expand Down
192 changes: 192 additions & 0 deletions src/rules/__tests__/prefer-comparison-matcher.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../prefer-comparison-matcher';
import { espreeParser } from './test-utils';

const ruleTester = new TSESLint.RuleTester({
parser: espreeParser,
parserOptions: {
ecmaVersion: 2015,
},
});

const generateInvalidCases = (
operator: string,
equalityMatcher: string,
preferredMatcher: string,
preferredMatcherWhenNegated: string,
): Array<TSESLint.InvalidTestCase<'useToBeComparison', never>> => {
return [
{
code: `expect(value ${operator} 1).${equalityMatcher}(true);`,
output: `expect(value).${preferredMatcher}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher },
column: 18 + operator.length,
line: 1,
},
],
},
{
code: `expect(value ${operator} 1)['${equalityMatcher}'](true);`,
output: `expect(value).${preferredMatcher}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher },
column: 18 + operator.length,
line: 1,
},
],
},
{
code: `expect(value ${operator} 1).${equalityMatcher}(false);`,
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher: preferredMatcherWhenNegated },
column: 18 + operator.length,
line: 1,
},
],
},
{
code: `expect(value ${operator} 1)['${equalityMatcher}'](false);`,
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher: preferredMatcherWhenNegated },
column: 18 + operator.length,
line: 1,
},
],
},
{
code: `expect(value ${operator} 1).not.${equalityMatcher}(true);`,
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher: preferredMatcherWhenNegated },
column: 18 + operator.length,
line: 1,
},
],
},
{
code: `expect(value ${operator} 1)['not'].${equalityMatcher}(true);`,
output: `expect(value).${preferredMatcherWhenNegated}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher: preferredMatcherWhenNegated },
column: 18 + operator.length,
line: 1,
},
],
},
{
code: `expect(value ${operator} 1).not.${equalityMatcher}(false);`,
output: `expect(value).${preferredMatcher}(1);`,
errors: [
{
messageId: 'useToBeComparison',
data: { preferredMatcher },
column: 18 + operator.length,
line: 1,
},
],
},
];
};

const generateValidStringLiteralCases = (operator: string, matcher: string) => {
return [
['x', "'y'"],
['x', '`y`'],
['x', '`y${z}`'],
].reduce((cases, [a, b]) => [
...cases,
...[
`expect(${a} ${operator} ${b}).${matcher}(true)`,
`expect(${a} ${operator} ${b}).${matcher}(false)`,
`expect(${a} ${operator} ${b}).not.${matcher}(true)`,
`expect(${a} ${operator} ${b}).not.${matcher}(false)`,
`expect(${b} ${operator} ${a}).${matcher}(true)`,
`expect(${b} ${operator} ${a}).${matcher}(false)`,
`expect(${b} ${operator} ${a}).not.${matcher}(true)`,
`expect(${b} ${operator} ${a}).not.${matcher}(false)`,
`expect(${a} ${operator} ${b}).${matcher}(true)`,
`expect(${a} ${operator} ${b}).${matcher}(false)`,
`expect(${a} ${operator} ${b}).not.${matcher}(true)`,
`expect(${a} ${operator} ${b}).not.${matcher}(false)`,
`expect(${b} ${operator} ${a}).${matcher}(true)`,
`expect(${b} ${operator} ${a}).${matcher}(false)`,
`expect(${b} ${operator} ${a}).not.${matcher}(true)`,
`expect(${b} ${operator} ${a}).not.${matcher}(false)`,
`expect(${b} ${operator} ${b}).not.${matcher}(false)`,
],
]);
};

const testComparisonOperator = (
operator: string,
preferredMatcher: string,
preferredMatcherWhenNegated: string,
) => {
ruleTester.run(`prefer-to-be-comparison: ${operator}`, rule, {
valid: [
'expect()',
'expect({}).toStrictEqual({})',
`expect(value).${preferredMatcher}(1);`,
`expect(value).${preferredMatcherWhenNegated}(1);`,
`expect(value).not.${preferredMatcher}(1);`,
`expect(value).not.${preferredMatcherWhenNegated}(1);`,
`expect(value).${preferredMatcher}(1);`,
...['toBe', 'toEqual', 'toStrictEqual'].reduce<string[]>(
(cases, equalityMatcher) => [
...cases,
...generateValidStringLiteralCases(operator, equalityMatcher),
],
[],
),
],
invalid: ['toBe', 'toEqual', 'toStrictEqual'].reduce<
Array<TSESLint.InvalidTestCase<'useToBeComparison', never>>
>(
(cases, equalityMatcher) => [
...cases,
...generateInvalidCases(
operator,
equalityMatcher,
preferredMatcher,
preferredMatcherWhenNegated,
),
],
[],
),
});
};

testComparisonOperator('>', 'toBeGreaterThan', 'toBeLessThanOrEqual');
testComparisonOperator('<', 'toBeLessThan', 'toBeGreaterThanOrEqual');
testComparisonOperator('>=', 'toBeGreaterThanOrEqual', 'toBeLessThan');
testComparisonOperator('<=', 'toBeLessThanOrEqual', 'toBeGreaterThan');

ruleTester.run(`prefer-to-be-comparison`, rule, {
valid: [
'expect()',
'expect({}).toStrictEqual({})',
'expect(a === b).toBe(true)',
'expect(a !== 2).toStrictEqual(true)',
'expect(a === b).not.toEqual(true)',
'expect(a !== "string").toStrictEqual(true)',
'expect(5 != a).toBe(true)',
'expect(a == "string").toBe(true)',
'expect(a == "string").not.toBe(true)',
],
invalid: [],
});
Loading