From 6940488d7b5a47577e2823e6d4385b511c5becf4 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 29 Sep 2021 20:38:17 +1300 Subject: [PATCH] fix(no-deprecated-functions): remove `process.cwd` from resolve paths (#889) --- README.md | 33 ++- docs/rules/no-deprecated-functions.md | 5 + src/index.ts | 2 +- src/rules/__tests__/detectJestVersion.test.ts | 227 ++++++++++++++++++ .../__tests__/no-deprecated-functions.test.ts | 159 +++--------- src/rules/detectJestVersion.ts | 44 ++++ src/rules/no-deprecated-functions.ts | 57 +---- 7 files changed, 347 insertions(+), 180 deletions(-) create mode 100644 src/rules/__tests__/detectJestVersion.test.ts create mode 100644 src/rules/detectJestVersion.ts diff --git a/README.md b/README.md index aea3318fe..d30dc8454 100644 --- a/README.md +++ b/README.md @@ -59,23 +59,43 @@ doing: This is included in all configs shared by this plugin, so can be omitted if extending them. -The behaviour of some rules (specifically `no-deprecated-functions`) change -depending on the version of `jest` being used. +### Jest `version` setting -This setting is detected automatically based off the version of the `jest` -package installed in `node_modules`, but it can also be provided explicitly if -desired: +The behaviour of some rules (specifically [`no-deprecated-functions`][]) change +depending on the version of Jest being used. + +By default, this plugin will attempt to determine to locate Jest using +`require.resolve`, meaning it will start looking in the closest `node_modules` +folder to the file being linted and work its way up. + +Since we cache the automatically determined version, if you're linting +sub-folders that have different versions of Jest, you may find that the wrong +version of Jest is considered when linting. You can work around this by +providing the Jest version explicitly in nested ESLint configs: ```json { "settings": { "jest": { - "version": 26 + "version": 27 } } } ``` +To avoid hard-coding a number, you can also fetch it from the installed version +of Jest if you use a JavaScript config file such as `.eslintrc.js`: + +```js +module.exports = { + settings: { + jest: { + version: require('jest/package.json').version, + }, + }, +}; +``` + ## Shareable configurations ### Recommended @@ -226,3 +246,4 @@ https://github.com/istanbuljs/eslint-plugin-istanbul [suggest]: https://img.shields.io/badge/-suggest-yellow.svg [fixable]: https://img.shields.io/badge/-fixable-green.svg [style]: https://img.shields.io/badge/-style-blue.svg +[`no-deprecated-functions`]: docs/rules/no-deprecated-functions.md diff --git a/docs/rules/no-deprecated-functions.md b/docs/rules/no-deprecated-functions.md index c591e0aeb..15f7b5390 100644 --- a/docs/rules/no-deprecated-functions.md +++ b/docs/rules/no-deprecated-functions.md @@ -6,6 +6,11 @@ either been renamed for clarity, or replaced with more powerful APIs. While typically these deprecated functions are kept in the codebase for a number of majors, eventually they are removed completely. +This rule requires knowing which version of Jest you're using - see +[this section of the readme](../../README.md#jest-version-setting) for details +on how that is obtained automatically and how you can explicitly provide a +version if needed. + ## Rule details This rule warns about calls to deprecated functions, and provides details on diff --git a/src/index.ts b/src/index.ts index 03d119a89..dfba92f46 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,7 +26,7 @@ const importDefault = (moduleName: string) => interopRequireDefault(require(moduleName)).default; const rulesDir = join(__dirname, 'rules'); -const excludedFiles = ['__tests__', 'utils']; +const excludedFiles = ['__tests__', 'detectJestVersion', 'utils']; const rules = readdirSync(rulesDir) .map(rule => parse(rule).name) diff --git a/src/rules/__tests__/detectJestVersion.test.ts b/src/rules/__tests__/detectJestVersion.test.ts new file mode 100644 index 000000000..1611d3032 --- /dev/null +++ b/src/rules/__tests__/detectJestVersion.test.ts @@ -0,0 +1,227 @@ +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { JSONSchemaForNPMPackageJsonFiles } from '@schemastore/package'; +import { create } from 'ts-node'; +import { detectJestVersion } from '../detectJestVersion'; + +const compileFnCode = (pathToFn: string) => { + const fnContents = fs.readFileSync(pathToFn, 'utf-8'); + + return create({ + transpileOnly: true, + compilerOptions: { sourceMap: false }, + }).compile(fnContents, pathToFn); +}; +const compiledFn = compileFnCode(require.resolve('../detectJestVersion.ts')); +const relativePathToFn = 'eslint-plugin-jest/lib/rules/detectJestVersion.js'; + +const runNodeScript = (cwd: string, script: string) => { + return spawnSync('node', ['-e', script.split('\n').join(' ')], { + cwd, + encoding: 'utf-8', + }); +}; + +const runDetectJestVersion = (cwd: string) => { + return runNodeScript( + cwd, + ` + try { + console.log(require('${relativePathToFn}').detectJestVersion()); + } catch (error) { + console.error(error.message); + } + `, + ); +}; + +/** + * Makes a new temp directory, prefixed with `eslint-plugin-jest-` + * + * @return {Promise} + */ +const makeTempDir = () => + fs.mkdtempSync(path.join(os.tmpdir(), 'eslint-plugin-jest-')); + +interface ProjectStructure { + [key: `${string}/package.json`]: JSONSchemaForNPMPackageJsonFiles; + [key: `${string}/${typeof relativePathToFn}`]: string; + [key: `${string}/`]: null; + 'package.json'?: JSONSchemaForNPMPackageJsonFiles; +} + +const setupFakeProject = (structure: ProjectStructure): string => { + const tempDir = makeTempDir(); + + for (const [filePath, contents] of Object.entries(structure)) { + if (contents === null) { + fs.mkdirSync(path.join(tempDir, filePath), { recursive: true }); + + continue; + } + + const folderPath = path.dirname(filePath); + + // make the directory (recursively) + fs.mkdirSync(path.join(tempDir, folderPath), { recursive: true }); + + const finalContents = + typeof contents === 'string' ? contents : JSON.stringify(contents); + + fs.writeFileSync(path.join(tempDir, filePath), finalContents); + } + + return tempDir; +}; + +describe('detectJestVersion', () => { + describe('basic tests', () => { + const packageJsonFactory = jest.fn(); + + beforeEach(() => { + jest.resetModules(); + jest.doMock(require.resolve('jest/package.json'), packageJsonFactory); + }); + + describe('when the package.json is missing the version property', () => { + it('throws an error', () => { + packageJsonFactory.mockReturnValue({}); + + expect(() => detectJestVersion()).toThrow( + /Unable to detect Jest version/iu, + ); + }); + }); + + it('caches versions', () => { + packageJsonFactory.mockReturnValue({ version: '1.2.3' }); + + const version = detectJestVersion(); + + jest.resetModules(); + + expect(detectJestVersion).not.toThrow(); + expect(detectJestVersion()).toBe(version); + }); + }); + + describe('when in a simple project', () => { + it('finds the correct version', () => { + const projectDir = setupFakeProject({ + 'package.json': { name: 'simple-project' }, + [`node_modules/${relativePathToFn}` as const]: compiledFn, + 'node_modules/jest/package.json': { + name: 'jest', + version: '21.0.0', + }, + }); + + const { stdout, stderr } = runDetectJestVersion(projectDir); + + expect(stdout.trim()).toBe('21'); + expect(stderr.trim()).toBe(''); + }); + }); + + describe('when in a hoisted mono-repo', () => { + it('finds the correct version', () => { + const projectDir = setupFakeProject({ + 'package.json': { name: 'mono-repo' }, + [`node_modules/${relativePathToFn}` as const]: compiledFn, + 'node_modules/jest/package.json': { + name: 'jest', + version: '19.0.0', + }, + 'packages/a/package.json': { name: 'package-a' }, + 'packages/b/package.json': { name: 'package-b' }, + }); + + const { stdout, stderr } = runDetectJestVersion(projectDir); + + expect(stdout.trim()).toBe('19'); + expect(stderr.trim()).toBe(''); + }); + }); + + describe('when in a subproject', () => { + it('finds the correct versions', () => { + const projectDir = setupFakeProject({ + 'backend/package.json': { name: 'package-a' }, + [`backend/node_modules/${relativePathToFn}` as const]: compiledFn, + 'backend/node_modules/jest/package.json': { + name: 'jest', + version: '24.0.0', + }, + 'frontend/package.json': { name: 'package-b' }, + [`frontend/node_modules/${relativePathToFn}` as const]: compiledFn, + 'frontend/node_modules/jest/package.json': { + name: 'jest', + version: '15.0.0', + }, + }); + + const { stdout: stdoutBackend, stderr: stderrBackend } = + runDetectJestVersion(path.join(projectDir, 'backend')); + + expect(stdoutBackend.trim()).toBe('24'); + expect(stderrBackend.trim()).toBe(''); + + const { stdout: stdoutFrontend, stderr: stderrFrontend } = + runDetectJestVersion(path.join(projectDir, 'frontend')); + + expect(stdoutFrontend.trim()).toBe('15'); + expect(stderrFrontend.trim()).toBe(''); + }); + }); + + describe('when jest is not installed', () => { + it('throws an error', () => { + const projectDir = setupFakeProject({ + 'package.json': { name: 'no-jest' }, + [`node_modules/${relativePathToFn}` as const]: compiledFn, + 'node_modules/pack/package.json': { name: 'pack' }, + }); + + const { stdout, stderr } = runDetectJestVersion(projectDir); + + expect(stdout.trim()).toBe(''); + expect(stderr.trim()).toContain('Unable to detect Jest version'); + }); + }); + + describe('when jest is changed on disk', () => { + it('uses the cached version', () => { + const projectDir = setupFakeProject({ + 'package.json': { name: 'no-jest' }, + [`node_modules/${relativePathToFn}` as const]: compiledFn, + 'node_modules/jest/package.json': { name: 'jest', version: '26.0.0' }, + }); + + const { stdout, stderr } = runNodeScript( + projectDir, + ` + const { detectJestVersion } = require('${relativePathToFn}'); + const fs = require('fs'); + + console.log(detectJestVersion()); + fs.writeFileSync( + 'node_modules/jest/package.json', + JSON.stringify({ + name: 'jest', + version: '25.0.0', + }), + ); + console.log(detectJestVersion()); + `, + ); + + const [firstCall, secondCall] = stdout.split('\n'); + + expect(firstCall).toBe('26'); + expect(secondCall).toBe('26'); + expect(stderr.trim()).toBe(''); + }); + }); +}); diff --git a/src/rules/__tests__/no-deprecated-functions.test.ts b/src/rules/__tests__/no-deprecated-functions.test.ts index 3c753def8..297c3c747 100644 --- a/src/rules/__tests__/no-deprecated-functions.test.ts +++ b/src/rules/__tests__/no-deprecated-functions.test.ts @@ -1,61 +1,17 @@ -import * as fs from 'fs'; -import * as os from 'os'; -import * as path from 'path'; -import { JSONSchemaForNPMPackageJsonFiles } from '@schemastore/package'; import { TSESLint } from '@typescript-eslint/experimental-utils'; -import rule, { - JestVersion, - _clearCachedJestVersion, -} from '../no-deprecated-functions'; +import { JestVersion, detectJestVersion } from '../detectJestVersion'; +import rule from '../no-deprecated-functions'; -const ruleTester = new TSESLint.RuleTester(); - -// pin the original cwd so that we can restore it after each test -const projectDir = process.cwd(); - -afterEach(() => process.chdir(projectDir)); - -/** - * Makes a new temp directory, prefixed with `eslint-plugin-jest-` - * - * @return {Promise} - */ -const makeTempDir = async () => - fs.mkdtempSync(path.join(os.tmpdir(), 'eslint-plugin-jest-')); - -/** - * Sets up a fake project with a `package.json` file located in - * `node_modules/jest` whose version is set to the given `jestVersion`. - * - * @param {JestVersion} jestVersion - * - * @return {Promise} - */ -const setupFakeProjectDirectory = async ( - jestVersion: JestVersion, -): Promise => { - const jestPackageJson: JSONSchemaForNPMPackageJsonFiles = { - name: 'jest', - version: `${jestVersion}.0.0`, - }; +jest.mock('../detectJestVersion'); - const tempDir = await makeTempDir(); - const jestPackagePath = path.join(tempDir, 'node_modules', 'jest'); +const detectJestVersionMock = detectJestVersion as jest.MockedFunction< + typeof detectJestVersion +>; - // todo: remove in node@10 & replace with { recursive: true } - fs.mkdirSync(path.join(tempDir, 'node_modules')); - - fs.mkdirSync(jestPackagePath); - await fs.writeFileSync( - path.join(jestPackagePath, 'package.json'), - JSON.stringify(jestPackageJson), - ); - - return tempDir; -}; +const ruleTester = new TSESLint.RuleTester(); const generateValidCases = ( - jestVersion: JestVersion | undefined, + jestVersion: JestVersion | string | undefined, functionCall: string, ): Array> => { const [name, func] = functionCall.split('.'); @@ -70,7 +26,7 @@ const generateValidCases = ( }; const generateInvalidCases = ( - jestVersion: JestVersion | undefined, + jestVersion: JestVersion | string | undefined, deprecation: string, replacement: string, ): Array> => { @@ -97,45 +53,18 @@ const generateInvalidCases = ( ]; }; -describe('the jest version cache', () => { - beforeEach(async () => process.chdir(await setupFakeProjectDirectory(17))); - - // change the jest version *after* each test case - afterEach(async () => { - const jestPackageJson: JSONSchemaForNPMPackageJsonFiles = { - name: 'jest', - version: '24.0.0', - }; - - const tempDir = process.cwd(); - - await fs.writeFileSync( - path.join(tempDir, 'node_modules', 'jest', 'package.json'), - JSON.stringify(jestPackageJson), - ); - }); - - ruleTester.run('no-deprecated-functions', rule, { - valid: [ - 'require("fs")', // this will cause jest version to be read & cached - 'jest.requireActual()', // deprecated after jest 17 - ], - invalid: [], - }); -}); - // contains the cache-clearing beforeEach so we can test the cache too describe('the rule', () => { - beforeEach(() => _clearCachedJestVersion()); - // a few sanity checks before doing our massive loop ruleTester.run('no-deprecated-functions', rule, { valid: [ - 'jest', - 'require("fs")', + { settings: { jest: { version: 14 } }, code: 'jest' }, + { settings: { jest: { version: 14 } }, code: 'require("fs")' }, ...generateValidCases(14, 'jest.resetModuleRegistry'), ...generateValidCases(17, 'require.requireActual'), ...generateValidCases(25, 'jest.genMockFromModule'), + ...generateValidCases('25.1.1', 'jest.genMockFromModule'), + ...generateValidCases('17.2', 'require.requireActual'), ], invalid: [ ...generateInvalidCases( @@ -149,15 +78,20 @@ describe('the rule', () => { 'jest.genMockFromModule', 'jest.createMockFromModule', ), + ...generateInvalidCases( + '26.0.0-next.11', + 'jest.genMockFromModule', + 'jest.createMockFromModule', + ), ], }); describe.each([ 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, ])('when using jest version %i', jestVersion => { - beforeEach(async () => - process.chdir(await setupFakeProjectDirectory(jestVersion)), - ); + beforeEach(async () => { + detectJestVersionMock.mockReturnValue(jestVersion); + }); const allowedFunctions: string[] = []; const deprecations = ( @@ -210,50 +144,23 @@ describe('the rule', () => { }); }); - describe('when no jest version is provided', () => { - describe('when the jest package.json is missing the version property', () => { - beforeEach(async () => { - const tempDir = await setupFakeProjectDirectory(1); - - await fs.writeFileSync( - path.join(tempDir, 'node_modules', 'jest', 'package.json'), - JSON.stringify({}), - ); - - process.chdir(tempDir); - }); - - it('requires the version to be set explicitly', () => { - expect(() => { - const linter = new TSESLint.Linter(); - - linter.defineRule('no-deprecated-functions', rule); - - linter.verify('jest.resetModuleRegistry()', { - rules: { 'no-deprecated-functions': 'error' }, - }); - }).toThrow( - 'Unable to detect Jest version - please ensure jest package is installed, or otherwise set version explicitly', - ); + describe('when there is an error in detecting the jest version', () => { + beforeEach(() => { + detectJestVersionMock.mockImplementation(() => { + throw new Error('oh noes!'); }); }); - describe('when the jest package.json is not found', () => { - beforeEach(async () => process.chdir(await makeTempDir())); + it('bubbles the error up', () => { + expect(() => { + const linter = new TSESLint.Linter(); - it('requires the version to be set explicitly', () => { - expect(() => { - const linter = new TSESLint.Linter(); + linter.defineRule('no-deprecated-functions', rule); - linter.defineRule('no-deprecated-functions', rule); - - linter.verify('jest.resetModuleRegistry()', { - rules: { 'no-deprecated-functions': 'error' }, - }); - }).toThrow( - 'Unable to detect Jest version - please ensure jest package is installed, or otherwise set version explicitly', - ); - }); + linter.verify('jest.resetModuleRegistry()', { + rules: { 'no-deprecated-functions': 'error' }, + }); + }).toThrow('oh noes!'); }); }); }); diff --git a/src/rules/detectJestVersion.ts b/src/rules/detectJestVersion.ts new file mode 100644 index 000000000..0e5ba5bea --- /dev/null +++ b/src/rules/detectJestVersion.ts @@ -0,0 +1,44 @@ +import { JSONSchemaForNPMPackageJsonFiles } from '@schemastore/package'; + +export type JestVersion = + | 14 + | 15 + | 16 + | 17 + | 18 + | 19 + | 20 + | 21 + | 22 + | 23 + | 24 + | 25 + | 26 + | 27 + | number; + +let cachedJestVersion: JestVersion | null = null; + +export const detectJestVersion = (): JestVersion => { + if (cachedJestVersion) { + return cachedJestVersion; + } + + try { + const jestPath = require.resolve('jest/package.json'); + + const jestPackageJson = + // eslint-disable-next-line @typescript-eslint/no-require-imports + require(jestPath) as JSONSchemaForNPMPackageJsonFiles; + + if (jestPackageJson.version) { + const [majorVersion] = jestPackageJson.version.split('.'); + + return (cachedJestVersion = parseInt(majorVersion, 10)); + } + } catch {} + + throw new Error( + 'Unable to detect Jest version - please ensure jest package is installed, or otherwise set version explicitly', + ); +}; diff --git a/src/rules/no-deprecated-functions.ts b/src/rules/no-deprecated-functions.ts index 94d56aa20..88e0f0d0a 100644 --- a/src/rules/no-deprecated-functions.ts +++ b/src/rules/no-deprecated-functions.ts @@ -1,64 +1,26 @@ -import { JSONSchemaForNPMPackageJsonFiles } from '@schemastore/package'; import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; +import { JestVersion, detectJestVersion } from './detectJestVersion'; import { createRule, getNodeName } from './utils'; interface ContextSettings { jest?: EslintPluginJestSettings; } -export type JestVersion = - | 14 - | 15 - | 16 - | 17 - | 18 - | 19 - | 20 - | 21 - | 22 - | 23 - | 24 - | 25 - | 26 - | 27 - | number; - interface EslintPluginJestSettings { - version: JestVersion; + version: JestVersion | string; } -let cachedJestVersion: JestVersion | null = null; - -/** @internal */ -export const _clearCachedJestVersion = () => (cachedJestVersion = null); - -const detectJestVersion = (): JestVersion => { - if (cachedJestVersion) { - return cachedJestVersion; +const parseJestVersion = (rawVersion: number | string): JestVersion => { + if (typeof rawVersion === 'number') { + return rawVersion; } - try { - const jestPath = require.resolve('jest/package.json', { - paths: [process.cwd()], - }); - - const jestPackageJson = - // eslint-disable-next-line @typescript-eslint/no-require-imports - require(jestPath) as JSONSchemaForNPMPackageJsonFiles; - - if (jestPackageJson.version) { - const [majorVersion] = jestPackageJson.version.split('.'); - - return (cachedJestVersion = parseInt(majorVersion, 10)); - } - } catch {} + const [majorVersion] = rawVersion.split('.'); - throw new Error( - 'Unable to detect Jest version - please ensure jest package is installed, or otherwise set version explicitly', - ); + return parseInt(majorVersion, 10); }; export default createRule({ @@ -79,9 +41,10 @@ export default createRule({ }, defaultOptions: [], create(context) { - const jestVersion = + const jestVersion = parseJestVersion( (context.settings as ContextSettings)?.jest?.version || - detectJestVersion(); + detectJestVersion(), + ); const deprecations: Record = { ...(jestVersion >= 15 && {