diff --git a/README.md b/README.md index d30dc8454..5b4a0bd56 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,7 @@ installations requiring long-term consistency. | [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | ![fixable][] | | [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | ![suggest][] | +| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | | ![fixable][] | | [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | ![style][] | ![fixable][] | | [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | ![style][] | ![fixable][] | | [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | ![style][] | ![fixable][] | diff --git a/docs/rules/prefer-to-be.md b/docs/rules/prefer-to-be.md new file mode 100644 index 000000000..8611b42cf --- /dev/null +++ b/docs/rules/prefer-to-be.md @@ -0,0 +1,53 @@ +# Suggest using `toBe()` for primitive literals (`prefer-to-be`) + +When asserting against primitive literals such as numbers and strings, the +equality matchers all operate the same, but read slightly differently in code. + +This rule recommends using the `toBe` matcher in these situations, as it forms +the most grammatically natural sentence. For `null`, `undefined`, and `NaN` this +rule recommends using their specific `toBe` matchers, as they give better error +messages as well. + +## Rule details + +This rule triggers a warning if `toEqual()` or `toStrictEqual()` are used to +assert a primitive literal value such as numbers, strings, and booleans. + +The following patterns are considered warnings: + +```js +expect(value).not.toEqual(5); +expect(getMessage()).toStrictEqual('hello world'); +expect(loadMessage()).resolves.toEqual('hello world'); +``` + +The following pattern is not warning: + +```js +expect(value).not.toBe(5); +expect(getMessage()).toBe('hello world'); +expect(loadMessage()).resolves.toBe('hello world'); +expect(didError).not.toBe(true); + +expect(catchError()).toStrictEqual({ message: 'oh noes!' }); +``` + +For `null`, `undefined`, and `NaN`, this rule triggers a warning if `toBe` is +used to assert against those literal values instead of their more specific +`toBe` counterparts: + +```js +expect(value).not.toBe(undefined); +expect(getMessage()).toBe(null); +expect(countMessages()).resolves.not.toBe(NaN); +``` + +The following pattern is not warning: + +```js +expect(value).toBeDefined(); +expect(getMessage()).toBeNull(); +expect(countMessages()).resolves.not.toBeNaN(); + +expect(catchError()).toStrictEqual({ message: undefined }); +``` diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index eb3178266..bf6e2af3b 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -40,6 +40,7 @@ Object { "jest/prefer-hooks-on-top": "error", "jest/prefer-spy-on": "error", "jest/prefer-strict-equal": "error", + "jest/prefer-to-be": "error", "jest/prefer-to-be-null": "error", "jest/prefer-to-be-undefined": "error", "jest/prefer-to-contain": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 6703e7a13..88f10b1a1 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 = 46; +const numberOfRules = 47; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/prefer-to-be.test.ts b/src/rules/__tests__/prefer-to-be.test.ts new file mode 100644 index 000000000..2cf92e69b --- /dev/null +++ b/src/rules/__tests__/prefer-to-be.test.ts @@ -0,0 +1,270 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import rule from '../prefer-to-be'; + +const ruleTester = new TSESLint.RuleTester(); + +ruleTester.run('prefer-to-be', rule, { + valid: [ + 'expect(null).toBeNull();', + 'expect(null).not.toBeNull();', + 'expect(null).toBe(1);', + 'expect(obj).toStrictEqual([ x, 1 ]);', + 'expect(obj).toStrictEqual({ x: 1 });', + 'expect(obj).not.toStrictEqual({ x: 1 });', + 'expect(value).toMatchSnapshot();', + "expect(catchError()).toStrictEqual({ message: 'oh noes!' })", + 'expect("something");', + ], + invalid: [ + { + code: 'expect(value).toEqual("my string");', + output: 'expect(value).toBe("my string");', + errors: [{ messageId: 'useToBe', column: 15, line: 1 }], + }, + { + code: 'expect(value).toStrictEqual("my string");', + output: 'expect(value).toBe("my string");', + errors: [{ messageId: 'useToBe', column: 15, line: 1 }], + }, + { + code: 'expect(value).toStrictEqual(1);', + output: 'expect(value).toBe(1);', + errors: [{ messageId: 'useToBe', column: 15, line: 1 }], + }, + { + code: 'expect(loadMessage()).resolves.toStrictEqual("hello world");', + output: 'expect(loadMessage()).resolves.toBe("hello world");', + errors: [{ messageId: 'useToBe', column: 32, line: 1 }], + }, + { + code: 'expect(loadMessage()).resolves.toStrictEqual(false);', + output: 'expect(loadMessage()).resolves.toBe(false);', + errors: [{ messageId: 'useToBe', column: 32, line: 1 }], + }, + ], +}); + +ruleTester.run('prefer-to-be: null', rule, { + valid: [ + 'expect(null).toBeNull();', + 'expect(null).not.toBeNull();', + 'expect(null).toBe(1);', + 'expect(obj).toStrictEqual([ x, 1 ]);', + 'expect(obj).toStrictEqual({ x: 1 });', + 'expect(obj).not.toStrictEqual({ x: 1 });', + 'expect(value).toMatchSnapshot();', + "expect(catchError()).toStrictEqual({ message: 'oh noes!' })", + 'expect("something");', + // + 'expect(null).not.toEqual();', + 'expect(null).toBe();', + 'expect(null).toMatchSnapshot();', + 'expect("a string").toMatchSnapshot(null);', + 'expect("a string").not.toMatchSnapshot();', + 'expect(null).toBe', + ], + invalid: [ + { + code: 'expect(null).toBe(null);', + output: 'expect(null).toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 14, line: 1 }], + }, + { + code: 'expect(null).toEqual(null);', + output: 'expect(null).toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 14, line: 1 }], + }, + { + code: 'expect(null).toStrictEqual(null);', + output: 'expect(null).toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 14, line: 1 }], + }, + { + code: 'expect("a string").not.toBe(null);', + output: 'expect("a string").not.toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 24, line: 1 }], + }, + { + code: 'expect("a string").not.toEqual(null);', + output: 'expect("a string").not.toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 24, line: 1 }], + }, + { + code: 'expect("a string").not.toStrictEqual(null);', + output: 'expect("a string").not.toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 24, line: 1 }], + }, + ], +}); + +ruleTester.run('prefer-to-be: undefined', rule, { + valid: [ + 'expect(undefined).toBeUndefined();', + 'expect(true).toBeDefined();', + 'expect({}).toEqual({});', + 'expect(something).toBe()', + 'expect(something).toBe(somethingElse)', + 'expect(something).toEqual(somethingElse)', + 'expect(something).not.toBe(somethingElse)', + 'expect(something).not.toEqual(somethingElse)', + 'expect(undefined).toBe', + 'expect("something");', + ], + + invalid: [ + { + code: 'expect(undefined).toBe(undefined);', + output: 'expect(undefined).toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 19, line: 1 }], + }, + { + code: 'expect(undefined).toEqual(undefined);', + output: 'expect(undefined).toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 19, line: 1 }], + }, + { + code: 'expect(undefined).toStrictEqual(undefined);', + output: 'expect(undefined).toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 19, line: 1 }], + }, + { + code: 'expect("a string").not.toBe(undefined);', + output: 'expect("a string").toBeDefined();', + errors: [{ messageId: 'useToBeDefined', column: 24, line: 1 }], + }, + { + code: 'expect("a string").not.toEqual(undefined);', + output: 'expect("a string").toBeDefined();', + errors: [{ messageId: 'useToBeDefined', column: 24, line: 1 }], + }, + { + code: 'expect("a string").not.toStrictEqual(undefined);', + output: 'expect("a string").toBeDefined();', + errors: [{ messageId: 'useToBeDefined', column: 24, line: 1 }], + }, + ], +}); + +ruleTester.run('prefer-to-be: NaN', rule, { + valid: [ + 'expect(NaN).toBeNaN();', + 'expect(true).not.toBeNaN();', + 'expect({}).toEqual({});', + 'expect(something).toBe()', + 'expect(something).toBe(somethingElse)', + 'expect(something).toEqual(somethingElse)', + 'expect(something).not.toBe(somethingElse)', + 'expect(something).not.toEqual(somethingElse)', + 'expect(undefined).toBe', + 'expect("something");', + ], + invalid: [ + { + code: 'expect(NaN).toBe(NaN);', + output: 'expect(NaN).toBeNaN();', + errors: [{ messageId: 'useToBeNaN', column: 13, line: 1 }], + }, + { + code: 'expect(NaN).toEqual(NaN);', + output: 'expect(NaN).toBeNaN();', + errors: [{ messageId: 'useToBeNaN', column: 13, line: 1 }], + }, + { + code: 'expect(NaN).toStrictEqual(NaN);', + output: 'expect(NaN).toBeNaN();', + errors: [{ messageId: 'useToBeNaN', column: 13, line: 1 }], + }, + { + code: 'expect("a string").not.toBe(NaN);', + output: 'expect("a string").not.toBeNaN();', + errors: [{ messageId: 'useToBeNaN', column: 24, line: 1 }], + }, + { + code: 'expect("a string").not.toEqual(NaN);', + output: 'expect("a string").not.toBeNaN();', + errors: [{ messageId: 'useToBeNaN', column: 24, line: 1 }], + }, + { + code: 'expect("a string").not.toStrictEqual(NaN);', + output: 'expect("a string").not.toBeNaN();', + errors: [{ messageId: 'useToBeNaN', column: 24, line: 1 }], + }, + ], +}); + +ruleTester.run('prefer-to-be: undefined vs defined', rule, { + valid: [ + 'expect(NaN).toBeNaN();', + 'expect(true).not.toBeNaN();', + 'expect({}).toEqual({});', + 'expect(something).toBe()', + 'expect(something).toBe(somethingElse)', + 'expect(something).toEqual(somethingElse)', + 'expect(something).not.toBe(somethingElse)', + 'expect(something).not.toEqual(somethingElse)', + 'expect(undefined).toBe', + 'expect("something");', + ], + invalid: [ + { + code: 'expect(undefined).not.toBeDefined();', + output: 'expect(undefined).toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 23, line: 1 }], + }, + { + code: 'expect(undefined).resolves.not.toBeDefined();', + output: 'expect(undefined).resolves.toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 32, line: 1 }], + }, + { + code: 'expect("a string").not.toBeUndefined();', + output: 'expect("a string").toBeDefined();', + errors: [{ messageId: 'useToBeDefined', column: 24, line: 1 }], + }, + { + code: 'expect("a string").rejects.not.toBeUndefined();', + output: 'expect("a string").rejects.toBeDefined();', + errors: [{ messageId: 'useToBeDefined', column: 32, line: 1 }], + }, + ], +}); + +new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), +}).run('prefer-to-be: typescript edition', rule, { + valid: [ + "(expect('Model must be bound to an array if the multiple property is true') as any).toHaveBeenTipped()", + ], + invalid: [ + { + code: 'expect(null).toEqual(1 as unknown as string as unknown as any);', + output: 'expect(null).toBe(1 as unknown as string as unknown as any);', + errors: [{ messageId: 'useToBe', column: 14, line: 1 }], + }, + { + code: 'expect("a string").not.toStrictEqual("string" as number);', + output: 'expect("a string").not.toBe("string" as number);', + errors: [{ messageId: 'useToBe', column: 24, line: 1 }], + }, + { + code: 'expect(null).toBe(null as unknown as string as unknown as any);', + output: 'expect(null).toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 14, line: 1 }], + }, + { + code: 'expect("a string").not.toEqual(null as number);', + output: 'expect("a string").not.toBeNull();', + errors: [{ messageId: 'useToBeNull', column: 24, line: 1 }], + }, + { + code: 'expect(undefined).toBe(undefined as unknown as string as any);', + output: 'expect(undefined).toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 19, line: 1 }], + }, + { + code: 'expect("a string").toEqual(undefined as number);', + output: 'expect("a string").toBeUndefined();', + errors: [{ messageId: 'useToBeUndefined', column: 20, line: 1 }], + }, + ], +}); diff --git a/src/rules/prefer-to-be.ts b/src/rules/prefer-to-be.ts new file mode 100644 index 000000000..687d774b8 --- /dev/null +++ b/src/rules/prefer-to-be.ts @@ -0,0 +1,171 @@ +import { + AST_NODE_TYPES, + TSESLint, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + EqualityMatcher, + MaybeTypeCast, + ModifierName, + ParsedEqualityMatcherCall, + ParsedExpectMatcher, + ParsedExpectModifier, + createRule, + followTypeAssertionChain, + isExpectCall, + isIdentifier, + isParsedEqualityMatcherCall, + parseExpectCall, +} from './utils'; + +const isNullLiteral = (node: TSESTree.Node): node is TSESTree.NullLiteral => + node.type === AST_NODE_TYPES.Literal && node.value === null; + +/** + * Checks if the given `ParsedEqualityMatcherCall` is a call to one of the equality matchers, + * with a `null` literal as the sole argument. + */ +const isNullEqualityMatcher = ( + matcher: ParsedEqualityMatcherCall, +): matcher is ParsedEqualityMatcherCall> => + isNullLiteral(getFirstArgument(matcher)); + +const isFirstArgumentIdentifier = ( + matcher: ParsedEqualityMatcherCall, + name: string, +) => isIdentifier(getFirstArgument(matcher), name); + +const isPrimitiveLiteral = (matcher: ParsedEqualityMatcherCall) => + getFirstArgument(matcher).type === AST_NODE_TYPES.Literal; + +const getFirstArgument = (matcher: ParsedEqualityMatcherCall) => { + return followTypeAssertionChain(matcher.arguments[0]); +}; + +type MessageId = + | 'useToBe' + | 'useToBeUndefined' + | 'useToBeDefined' + | 'useToBeNull' + | 'useToBeNaN'; + +type ToBeWhat = MessageId extends `useToBe${infer M}` ? M : never; + +const reportPreferToBe = ( + context: TSESLint.RuleContext, + whatToBe: ToBeWhat, + matcher: ParsedExpectMatcher, + modifier?: ParsedExpectModifier, +) => { + const modifierNode = modifier?.negation || modifier?.node; + + context.report({ + messageId: `useToBe${whatToBe}`, + fix(fixer) { + const fixes = [ + fixer.replaceText(matcher.node.property, `toBe${whatToBe}`), + ]; + + if (matcher.arguments?.length && whatToBe !== '') { + fixes.push(fixer.remove(matcher.arguments[0])); + } + + if (modifierNode) { + fixes.push( + fixer.removeRange([ + modifierNode.property.range[0] - 1, + modifierNode.property.range[1], + ]), + ); + } + + return fixes; + }, + node: matcher.node.property, + }); +}; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest using `toBe()` for primitive literals', + recommended: false, + }, + messages: { + useToBe: 'Use `toBe` when expecting primitive literals', + useToBeUndefined: 'Use `toBeUndefined` instead', + useToBeDefined: 'Use `toBeDefined` instead', + useToBeNull: 'Use `toBeNull` instead', + useToBeNaN: 'Use `toBeNaN` instead', + }, + fixable: 'code', + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher, modifier } = parseExpectCall(node); + + if (!matcher) { + return; + } + + if ( + (modifier?.name === ModifierName.not || modifier?.negation) && + ['toBeUndefined', 'toBeDefined'].includes(matcher.name) + ) { + reportPreferToBe( + context, + matcher.name === 'toBeDefined' ? 'Undefined' : 'Defined', + matcher, + modifier, + ); + + return; + } + + if (!isParsedEqualityMatcherCall(matcher)) { + return; + } + + if (isNullEqualityMatcher(matcher)) { + reportPreferToBe(context, 'Null', matcher); + + return; + } + + if (isFirstArgumentIdentifier(matcher, 'undefined')) { + const name = + modifier?.name === ModifierName.not || modifier?.negation + ? 'Defined' + : 'Undefined'; + + reportPreferToBe(context, name, matcher, modifier); + + return; + } + + if (isFirstArgumentIdentifier(matcher, 'NaN')) { + reportPreferToBe(context, 'NaN', matcher); + + return; + } + + if ( + isPrimitiveLiteral(matcher) && + matcher.name !== EqualityMatcher.toBe + ) { + reportPreferToBe(context, '', matcher); + } + }, + }; + }, +}); diff --git a/src/rules/utils.ts b/src/rules/utils.ts index 24ce68d34..776169f58 100644 --- a/src/rules/utils.ts +++ b/src/rules/utils.ts @@ -210,7 +210,7 @@ interface KnownIdentifier extends TSESTree.Identifier { * * @template V */ -const isIdentifier = ( +export const isIdentifier = ( node: TSESTree.Node, name?: V, ): node is KnownIdentifier => @@ -391,7 +391,7 @@ export interface NotNegatableParsedModifier< negation?: never; } -type ParsedExpectModifier = +export type ParsedExpectModifier = | NotNegatableParsedModifier | NegatableParsedModifier;