From 341353bc7d57685cc5e0b31501d6ca336a0dbaf0 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 16 Jan 2022 09:37:14 +1300 Subject: [PATCH] feat: create `prefer-equality-matcher` rule (#1016) --- README.md | 1 + docs/rules/prefer-equality-matcher.md | 29 ++++ .../__snapshots__/rules.test.ts.snap | 1 + src/__tests__/rules.test.ts | 2 +- .../__tests__/prefer-equality-matcher.test.ts | 155 ++++++++++++++++++ src/rules/prefer-equality-matcher.ts | 138 ++++++++++++++++ 6 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 docs/rules/prefer-equality-matcher.md create mode 100644 src/rules/__tests__/prefer-equality-matcher.test.ts create mode 100644 src/rules/prefer-equality-matcher.ts diff --git a/README.md b/README.md index 580632896..317290b12 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,7 @@ installations requiring long-term consistency. | [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-equality-matcher](docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | ![suggest][] | | [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 | | | diff --git a/docs/rules/prefer-equality-matcher.md b/docs/rules/prefer-equality-matcher.md new file mode 100644 index 000000000..b0a0cb0ac --- /dev/null +++ b/docs/rules/prefer-equality-matcher.md @@ -0,0 +1,29 @@ +# Suggest using the built-in equality matchers (`prefer-equality-matcher`) + +Jest has built-in matchers for expecting equality which allow for more readable +tests and error messages if an expectation fails. + +## Rule details + +This rule checks for _strict_ equality checks (`===` & `!==`) in tests that +could be replaced with one of the following built-in equality matchers: + +- `toBe` +- `toEqual` +- `toStrictEqual` + +Examples of **incorrect** code for this rule: + +```js +expect(x === 5).toBe(true); +expect(name === 'Carl').not.toEqual(true); +expect(myObj !== thatObj).toStrictEqual(true); +``` + +Examples of **correct** code for this rule: + +```js +expect(x).toBe(5); +expect(name).not.toEqual('Carl'); +expect(myObj).toStrictEqual(thatObj); +``` diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index f4949b970..3adcd0db1 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -36,6 +36,7 @@ Object { "jest/no-test-return-statement": "error", "jest/prefer-called-with": "error", "jest/prefer-comparison-matcher": "error", + "jest/prefer-equality-matcher": "error", "jest/prefer-expect-assertions": "error", "jest/prefer-expect-resolves": "error", "jest/prefer-hooks-on-top": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index d37e1bd4a..6a8ff2df1 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -2,7 +2,7 @@ import { existsSync } from 'fs'; import { resolve } from 'path'; import plugin from '../'; -const numberOfRules = 44; +const numberOfRules = 45; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/prefer-equality-matcher.test.ts b/src/rules/__tests__/prefer-equality-matcher.test.ts new file mode 100644 index 000000000..757425cdb --- /dev/null +++ b/src/rules/__tests__/prefer-equality-matcher.test.ts @@ -0,0 +1,155 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import rule from '../prefer-equality-matcher'; +import { espreeParser } from './test-utils'; + +const ruleTester = new TSESLint.RuleTester({ + parser: espreeParser, + parserOptions: { + ecmaVersion: 2015, + }, +}); + +type RuleMessages> = + TRuleModule extends TSESLint.RuleModule + ? TMessageIds + : never; + +type RuleSuggestionOutput = TSESLint.SuggestionOutput< + RuleMessages +>; + +const expectSuggestions = ( + output: (equalityMatcher: string) => string, +): RuleSuggestionOutput[] => { + return ['toBe', 'toEqual', 'toStrictEqual'].map( + equalityMatcher => ({ + messageId: 'suggestEqualityMatcher', + data: { equalityMatcher }, + output: output(equalityMatcher), + }), + ); +}; + +ruleTester.run('prefer-equality-matcher: ===', rule, { + valid: [ + 'expect(a == 1).toBe(true)', + 'expect(1 == a).toBe(true)', + 'expect(a == b).toBe(true)', + ], + invalid: [ + { + code: 'expect(a === b).toBe(true);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + { + code: 'expect(a === b).toBe(false);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).not.${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + { + code: 'expect(a === b).not.toBe(true);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).not.${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + { + code: 'expect(a === b).not.toBe(false);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + ], +}); + +ruleTester.run('prefer-equality-matcher: !==', rule, { + valid: [ + 'expect(a != 1).toBe(true)', + 'expect(1 != a).toBe(true)', + 'expect(a != b).toBe(true)', + ], + invalid: [ + { + code: 'expect(a !== b).toBe(true);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).not.${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + { + code: 'expect(a !== b).toBe(false);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + { + code: 'expect(a !== b).not.toBe(true);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + { + code: 'expect(a !== b).not.toBe(false);', + errors: [ + { + messageId: 'useEqualityMatcher', + suggestions: expectSuggestions( + equalityMatcher => `expect(a).not.${equalityMatcher}(b);`, + ), + column: 17, + line: 1, + }, + ], + }, + ], +}); diff --git a/src/rules/prefer-equality-matcher.ts b/src/rules/prefer-equality-matcher.ts new file mode 100644 index 000000000..93570ffc2 --- /dev/null +++ b/src/rules/prefer-equality-matcher.ts @@ -0,0 +1,138 @@ +import { + AST_NODE_TYPES, + TSESLint, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + MaybeTypeCast, + ModifierName, + ParsedEqualityMatcherCall, + ParsedExpectMatcher, + createRule, + followTypeAssertionChain, + isExpectCall, + isParsedEqualityMatcherCall, + parseExpectCall, +} from './utils'; + +const isBooleanLiteral = ( + node: TSESTree.Node, +): node is TSESTree.BooleanLiteral => + node.type === AST_NODE_TYPES.Literal && typeof node.value === 'boolean'; + +type ParsedBooleanEqualityMatcherCall = ParsedEqualityMatcherCall< + MaybeTypeCast +>; + +/** + * Checks if the given `ParsedExpectMatcher` is a call to one of the equality matchers, + * with a boolean literal as the sole argument. + * + * @example javascript + * toBe(true); + * toEqual(false); + * + * @param {ParsedExpectMatcher} matcher + * + * @return {matcher is ParsedBooleanEqualityMatcher} + */ +const isBooleanEqualityMatcher = ( + matcher: ParsedExpectMatcher, +): matcher is ParsedBooleanEqualityMatcherCall => + isParsedEqualityMatcherCall(matcher) && + isBooleanLiteral(followTypeAssertionChain(matcher.arguments[0])); + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest using the built-in equality matchers', + recommended: false, + suggestion: true, + }, + messages: { + useEqualityMatcher: 'Prefer using one of the equality matchers instead', + suggestEqualityMatcher: 'Use `{{ equalityMatcher }}`', + }, + hasSuggestions: true, + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { + expect: { + arguments: [comparison], + range: [, expectCallEnd], + }, + matcher, + modifier, + } = parseExpectCall(node); + + if ( + !matcher || + comparison?.type !== AST_NODE_TYPES.BinaryExpression || + (comparison.operator !== '===' && comparison.operator !== '!==') || + !isBooleanEqualityMatcher(matcher) + ) { + return; + } + + const matcherValue = followTypeAssertionChain( + matcher.arguments[0], + ).value; + + // we need to negate the expectation if the current expected + // value is itself negated by the "not" modifier + const addNotModifier = + (comparison.operator === '!==' ? !matcherValue : matcherValue) === + !!modifier; + + const buildFixer = + (equalityMatcher: string): TSESLint.ReportFixFunction => + fixer => { + const sourceCode = context.getSourceCode(); + + return [ + // replace the comparison argument with the left-hand side of the comparison + fixer.replaceText( + comparison, + sourceCode.getText(comparison.left), + ), + // replace the current matcher & modifier with the preferred matcher + fixer.replaceTextRange( + [expectCallEnd, matcher.node.range[1]], + addNotModifier + ? `.${ModifierName.not}.${equalityMatcher}` + : `.${equalityMatcher}`, + ), + // replace the matcher argument with the right-hand side of the comparison + fixer.replaceText( + matcher.arguments[0], + sourceCode.getText(comparison.right), + ), + ]; + }; + + context.report({ + messageId: 'useEqualityMatcher', + suggest: ['toBe', 'toEqual', 'toStrictEqual'].map( + equalityMatcher => ({ + messageId: 'suggestEqualityMatcher', + data: { equalityMatcher }, + fix: buildFixer(equalityMatcher), + }), + ), + node: (modifier || matcher).node.property, + }); + }, + }; + }, +});