From 8d2c17c449841465630bea5269de677455ef9a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20C=2E=20Viesca=20Ruiz?= Date: Mon, 27 Jul 2020 21:56:57 +0200 Subject: [PATCH] feat: create `no-interpolation-in-snapshots` rule (#553) * feat(rules): add no-interpolation-inline-snapshot rule * docs: fix fromat in rule documentation * fix: add chamges requested in review * test: update number of rules * docs: update docs/rules/no-interpolation-inline-snapshot.md Co-authored-by: Gareth Jones * feat: add toThrowErrorMatchingInlineSnapshot * docs: update rules table in README * docs: apply suggestions from code review Co-authored-by: Gareth Jones * docs: fix format in README * fix: add review suggestions * docs: add examples to README * chore: rename rule to no-interpolation-in-snapshots * chore: remove changes from CHANGELOG.md * docs: update rule table with new name * fix: apply suggestions from code review Co-authored-by: Gareth Jones Co-authored-by: Simen Bekkhus * test: add test to cover missing branch * docs: regenerate table with the new script * fix: edit rule description Co-authored-by: Gareth Jones Co-authored-by: Simen Bekkhus --- README.md | 83 +++++++++-------- docs/rules/no-interpolation-in-snapshots.md | 60 ++++++++++++ .../__snapshots__/rules.test.ts.snap | 1 + src/__tests__/rules.test.ts | 2 +- .../no-interpolation-in-snapshots.test.ts | 93 +++++++++++++++++++ src/rules/no-interpolation-in-snapshots.ts | 54 +++++++++++ 6 files changed, 251 insertions(+), 42 deletions(-) create mode 100644 docs/rules/no-interpolation-in-snapshots.md create mode 100644 src/rules/__tests__/no-interpolation-in-snapshots.test.ts create mode 100644 src/rules/no-interpolation-in-snapshots.ts diff --git a/README.md b/README.md index 5223b0cd6..231fa653a 100644 --- a/README.md +++ b/README.md @@ -128,47 +128,48 @@ installations requiring long-term consistency. -| Rule | Description | Configurations | Fixable | -| ---------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ | -| [consistent-test-it](docs/rules/consistent-test-it.md) | Have control over `test` and `it` usages | | ![fixable][] | -| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ![recommended][] | | -| [lowercase-name](docs/rules/lowercase-name.md) | Enforce lowercase test names | | ![fixable][] | -| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | ![style][] | ![fixable][] | -| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | ![recommended][] | | -| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Prevent calling `expect` conditionally | | | -| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | | ![fixable][] | -| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended][] | | -| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | -| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | ![recommended][] | | -| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended][] | ![fixable][] | -| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | -| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended][] | | -| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | | -| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | ![recommended][] | ![fixable][] | -| [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | | -| [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | | -| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | ![recommended][] | | -| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | -| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ![recommended][] | | -| [no-test-callback](docs/rules/no-test-callback.md) | Avoid using a callback in asynchronous tests | ![recommended][] | ![suggest][] | -| [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-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] | -| [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-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][] | -| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] | -| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] | -| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | -| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | -| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | | -| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | | -| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | | -| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | | ![fixable][] | +| Rule | Description | Configurations | Fixable | +| ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ | +| [consistent-test-it](docs/rules/consistent-test-it.md) | Have control over `test` and `it` usages | | ![fixable][] | +| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ![recommended][] | | +| [lowercase-name](docs/rules/lowercase-name.md) | Enforce lowercase test names | | ![fixable][] | +| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | ![style][] | ![fixable][] | +| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | ![recommended][] | | +| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Prevent calling `expect` conditionally | | | +| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | | ![fixable][] | +| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended][] | | +| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | +| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | ![recommended][] | | +| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended][] | ![fixable][] | +| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | +| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended][] | | +| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | | +| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | | | +| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | ![recommended][] | ![fixable][] | +| [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | | +| [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | | +| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | ![recommended][] | | +| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | +| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ![recommended][] | | +| [no-test-callback](docs/rules/no-test-callback.md) | Avoid using a callback in asynchronous tests | ![recommended][] | ![suggest][] | +| [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-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] | +| [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-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][] | +| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] | +| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] | +| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | +| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | +| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | | +| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | | +| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | | +| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | | ![fixable][] | diff --git a/docs/rules/no-interpolation-in-snapshots.md b/docs/rules/no-interpolation-in-snapshots.md new file mode 100644 index 000000000..0ac13540e --- /dev/null +++ b/docs/rules/no-interpolation-in-snapshots.md @@ -0,0 +1,60 @@ +# Disallow string interpolation inside snapshots (`no-interpolation-in-snapshots`) + +Prevents the use of string interpolations in snapshots. + +## Rule Details + +Interpolation prevents snapshots from being updated. Instead, properties should +be overloaded with a matcher by using +[property matchers](https://jestjs.io/docs/en/snapshot-testing#property-matchers). + +Examples of **incorrect** code for this rule: + +```js +expect(something).toMatchInlineSnapshot( + `Object { + property: ${interpolated} + }`, +); + +expect(something).toMatchInlineSnapshot( + { other: expect.any(Number) }, + `Object { + other: Any, + property: ${interpolated} + }`, +); + +expect(errorThrowingFunction).toThrowErrorMatchingInlineSnapshot( + `${interpolated}`, +); +``` + +Examples of **correct** code for this rule: + +```js +expect(something).toMatchInlineSnapshot(); + +expect(something).toMatchInlineSnapshot( + `Object { + property: 1 + }`, +); + +expect(something).toMatchInlineSnapshot( + { property: expect.any(Date) }, + `Object { + property: Any + }`, +); + +expect(errorThrowingFunction).toThrowErrorMatchingInlineSnapshot(); + +expect(errorThrowingFunction).toThrowErrorMatchingInlineSnapshot( + `Error Message`, +); +``` + +## When Not To Use It + +Don't use this rule on non-jest test files. diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index bbae8229a..d82ca52e3 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -25,6 +25,7 @@ Object { "jest/no-hooks": "error", "jest/no-identical-title": "error", "jest/no-if": "error", + "jest/no-interpolation-in-snapshots": "error", "jest/no-jasmine-globals": "error", "jest/no-jest-import": "error", "jest/no-large-snapshots": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 6c08c7a67..550353f1c 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -3,7 +3,7 @@ import { resolve } from 'path'; import plugin from '../'; const ruleNames = Object.keys(plugin.rules); -const numberOfRules = 43; +const numberOfRules = 44; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/src/rules/__tests__/no-interpolation-in-snapshots.test.ts b/src/rules/__tests__/no-interpolation-in-snapshots.test.ts new file mode 100644 index 000000000..b7bbb61fb --- /dev/null +++ b/src/rules/__tests__/no-interpolation-in-snapshots.test.ts @@ -0,0 +1,93 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import resolveFrom from 'resolve-from'; +import rule from '../no-interpolation-in-snapshots'; + +const ruleTester = new TSESLint.RuleTester({ + parser: resolveFrom(require.resolve('eslint'), 'espree'), + parserOptions: { + ecmaVersion: 2017, + }, +}); + +ruleTester.run('no-interpolation-in-snapshots', rule, { + valid: [ + 'expect("something").toEqual("else");', + 'expect(something).toMatchInlineSnapshot();', + 'expect(something).toMatchInlineSnapshot(`No interpolation`);', + 'expect(something).toMatchInlineSnapshot({}, `No interpolation`);', + 'expect(something);', + 'expect(something).not;', + 'expect.toHaveAssertions();', + 'myObjectWants.toMatchInlineSnapshot({}, `${interpolated}`);', + 'myObjectWants.toMatchInlineSnapshot({}, `${interpolated1} ${interpolated2}`);', + 'toMatchInlineSnapshot({}, `${interpolated}`);', + 'toMatchInlineSnapshot({}, `${interpolated1} ${interpolated2}`);', + 'expect(something).toThrowErrorMatchingInlineSnapshot();', + 'expect(something).toThrowErrorMatchingInlineSnapshot(`No interpolation`);', + ], + invalid: [ + { + code: 'expect(something).toMatchInlineSnapshot(`${interpolated}`);', + errors: [ + { + endColumn: 58, + column: 41, + messageId: 'noInterpolation', + }, + ], + }, + { + code: 'expect(something).not.toMatchInlineSnapshot(`${interpolated}`);', + errors: [ + { + endColumn: 62, + column: 45, + messageId: 'noInterpolation', + }, + ], + }, + { + code: 'expect(something).toMatchInlineSnapshot({}, `${interpolated}`);', + errors: [ + { + endColumn: 62, + column: 45, + messageId: 'noInterpolation', + }, + ], + }, + { + code: + 'expect(something).not.toMatchInlineSnapshot({}, `${interpolated}`);', + errors: [ + { + endColumn: 66, + column: 49, + messageId: 'noInterpolation', + }, + ], + }, + { + code: + 'expect(something).toThrowErrorMatchingInlineSnapshot(`${interpolated}`);', + errors: [ + { + endColumn: 71, + column: 54, + messageId: 'noInterpolation', + }, + ], + }, + { + code: + 'expect(something).not.toThrowErrorMatchingInlineSnapshot(`${interpolated}`);', + errors: [ + { + endColumn: 75, + column: 58, + messageId: 'noInterpolation', + }, + ], + }, + ], +}); diff --git a/src/rules/no-interpolation-in-snapshots.ts b/src/rules/no-interpolation-in-snapshots.ts new file mode 100644 index 000000000..11d68e38d --- /dev/null +++ b/src/rules/no-interpolation-in-snapshots.ts @@ -0,0 +1,54 @@ +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { createRule, isExpectCall, parseExpectCall } from './utils'; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Disallow string interpolation inside snapshots', + recommended: false, + }, + messages: { + noInterpolation: 'Do not use string interpolation inside of snapshots', + }, + schema: [], + type: 'problem', + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher } = parseExpectCall(node); + + if (!matcher) { + return; + } + + if ( + [ + 'toMatchInlineSnapshot', + 'toThrowErrorMatchingInlineSnapshot', + ].includes(matcher.name) + ) { + // Check all since the optional 'propertyMatchers' argument might be present + matcher.arguments?.forEach(argument => { + if ( + argument.type === AST_NODE_TYPES.TemplateLiteral && + argument.expressions.length > 0 + ) { + context.report({ + messageId: 'noInterpolation', + node: argument, + }); + } + }); + } + }, + }; + }, +});