From 5fc1c980acedfaf452621f99f97faa4aec2f1b16 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Fri, 19 Apr 2019 14:01:24 +0200 Subject: [PATCH] feat: add no-mocks-import rule BREAKING CHANGE: Add no-mocks-import to recommended rules --- README.md | 2 + __tests__/rules.test.js | 2 +- docs/rules/no-mocks-import.md | 27 +++++++ index.js | 1 + rules/__tests__/no-mocks-import.test.js | 96 +++++++++++++++++++++++++ rules/no-mocks-import.js | 34 +++++++++ 6 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-mocks-import.md create mode 100644 rules/__tests__/no-mocks-import.test.js create mode 100644 rules/no-mocks-import.js diff --git a/README.md b/README.md index e619ae66e..ea58f44b4 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ for more information about extending configuration files. | [no-identical-title][] | Disallow identical titles | ![recommended][] | | | [no-jasmine-globals][] | Disallow Jasmine globals | ![recommended][] | ![fixable-yellow][] | | [no-jest-import][] | Disallow importing `jest` | ![recommended][] | | +| [no-mocks-import][] | Disallow manually importing from `__mocks__` | | | | [no-large-snapshots][] | Disallow large snapshots | | | | [no-test-callback][] | Using a callback in asynchronous tests | | ![fixable-green][] | | [no-test-prefixes][] | Disallow using `f` & `x` prefixes to define focused/skipped tests | ![recommended][] | ![fixable-green][] | @@ -138,6 +139,7 @@ for more information about extending configuration files. [no-identical-title]: docs/rules/no-identical-title.md [no-jasmine-globals]: docs/rules/no-jasmine-globals.md [no-jest-import]: docs/rules/no-jest-import.md +[no-mocks-import]: docs/rules/no-mocks-import.md [no-large-snapshots]: docs/rules/no-large-snapshots.md [no-test-callback]: docs/rules/no-test-callback.md [no-test-prefixes]: docs/rules/no-test-prefixes.md diff --git a/__tests__/rules.test.js b/__tests__/rules.test.js index c882c7724..63f7f2d0a 100644 --- a/__tests__/rules.test.js +++ b/__tests__/rules.test.js @@ -5,7 +5,7 @@ const path = require('path'); const { rules } = require('../index'); const ruleNames = Object.keys(rules); -const numberOfRules = 30; +const numberOfRules = 31; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/docs/rules/no-mocks-import.md b/docs/rules/no-mocks-import.md new file mode 100644 index 000000000..4441dec11 --- /dev/null +++ b/docs/rules/no-mocks-import.md @@ -0,0 +1,27 @@ +# Disallow manually importing from `__mocks__` (no-mocks-import) + +When using `jest.mock`, your tests (just like the code being tested) should +import from `./x`, not `./__mocks__/x`. Not following this rule can lead to +confusion, because you will have multiple instances of the mocked module: + +```js +jest.mock('./x'); +const x1 = require('./x'); +const x2 = require('./__mocks__/x'); + +test('x', () => { + expect(x1).toBe(x2); // fails! They are both instances of `./__mocks__/x`, but not referentially equal +}); +``` + +### Rule details + +This rule reports imports from a path containing a `__mocks__` component. + +Example violations: + +```js +import thing from './__mocks__/index'; +require('./__mocks__/index'); +require('__mocks__'); +``` diff --git a/index.js b/index.js index 0e710f51a..2f9da28f1 100644 --- a/index.js +++ b/index.js @@ -27,6 +27,7 @@ module.exports = { 'jest/no-focused-tests': 'error', 'jest/no-identical-title': 'error', 'jest/no-jest-import': 'error', + // 'jest/no-mocks-import': 'error', 'jest/no-jasmine-globals': 'warn', 'jest/no-test-prefixes': 'error', 'jest/valid-describe': 'error', diff --git a/rules/__tests__/no-mocks-import.test.js b/rules/__tests__/no-mocks-import.test.js new file mode 100644 index 000000000..04ca37de0 --- /dev/null +++ b/rules/__tests__/no-mocks-import.test.js @@ -0,0 +1,96 @@ +'use strict'; + +const rule = require('../no-mocks-import.js'); +const { RuleTester } = require('eslint'); +const ruleTester = new RuleTester(); +const message = `Mocks should not be manually imported from a __mocks__ directory. Instead use jest.mock and import from the original module path.`; + +ruleTester.run('no-mocks-import', rule, { + valid: [ + { + code: 'import something from "something"', + parserOptions: { sourceType: 'module' }, + }, + 'require("somethingElse")', + 'require("./__mocks__.js")', + 'require("./__mocks__x")', + 'require("./__mocks__x/x")', + 'require("./x__mocks__")', + 'require("./x__mocks__/x")', + 'require()', + 'entirelyDifferent(fn)', + ], + invalid: [ + { + code: 'require("./__mocks__")', + errors: [ + { + endColumn: 22, + column: 9, + message, + }, + ], + }, + { + code: 'require("./__mocks__/")', + errors: [ + { + endColumn: 23, + column: 9, + message, + }, + ], + }, + { + code: 'require("./__mocks__/index")', + errors: [ + { + endColumn: 28, + column: 9, + message, + }, + ], + }, + { + code: 'require("__mocks__")', + errors: [ + { + endColumn: 20, + column: 9, + message, + }, + ], + }, + { + code: 'require("__mocks__/")', + errors: [ + { + endColumn: 21, + column: 9, + message, + }, + ], + }, + { + code: 'require("__mocks__/index")', + errors: [ + { + endColumn: 26, + column: 9, + message, + }, + ], + }, + { + code: 'import thing from "./__mocks__/index"', + parserOptions: { sourceType: 'module' }, + errors: [ + { + endColumn: 38, + column: 1, + message, + }, + ], + }, + ], +}); diff --git a/rules/no-mocks-import.js b/rules/no-mocks-import.js new file mode 100644 index 000000000..d4a99adcc --- /dev/null +++ b/rules/no-mocks-import.js @@ -0,0 +1,34 @@ +'use strict'; + +const { posix } = require('path'); +const { getDocsUrl } = require('./util'); + +const mocksDirName = '__mocks__'; +const message = `Mocks should not be manually imported from a ${mocksDirName} directory. Instead use jest.mock and import from the original module path.`; + +const isMockPath = path => path.split(posix.sep).includes(mocksDirName); + +module.exports = { + meta: { + docs: { + url: getDocsUrl(__filename), + }, + }, + create(context) { + return { + ImportDeclaration(node) { + if (isMockPath(node.source.value)) { + context.report({ node, message }); + } + }, + 'CallExpression[callee.name="require"]'(node) { + if (node.arguments.length && isMockPath(node.arguments[0].value)) { + context.report({ + loc: node.arguments[0].loc, + message, + }); + } + }, + }; + }, +};