From 261696e00e6a43f2027678fc7321cf0dda4a807e Mon Sep 17 00:00:00 2001 From: Spencer Snyder Date: Wed, 3 Nov 2021 09:16:36 -0600 Subject: [PATCH] Fix handling of tsconfig's (#632) --- index.js | 15 +- lib/options-manager.js | 194 ++++++++++-------- .../typescript/extends-config/package.json | 3 + .../typescript/extends-config/tsconfig.json | 3 + test/lint-files.js | 8 +- test/options-manager.js | 45 +++- 6 files changed, 160 insertions(+), 108 deletions(-) create mode 100644 test/fixtures/typescript/extends-config/package.json create mode 100644 test/fixtures/typescript/extends-config/tsconfig.json diff --git a/index.js b/index.js index 233a337e..1a4f2f01 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,7 @@ import path from 'node:path'; import {ESLint} from 'eslint'; import {globby, isGitIgnoredSync} from 'globby'; -import {isEqual, groupBy} from 'lodash-es'; +import {isEqual} from 'lodash-es'; import micromatch from 'micromatch'; import arrify from 'arrify'; import slash from 'slash'; @@ -9,6 +9,7 @@ import { parseOptions, getIgnores, mergeWithFileConfig, + getOptionGroups, } from './lib/options-manager.js'; import {mergeReports, processReport, getIgnoredReport} from './lib/report.js'; @@ -34,8 +35,8 @@ const getConfig = async options => { }; const lintText = async (string, options) => { - options = await parseOptions(options); - const {filePath, warnIgnored, eslintOptions, isQuiet} = options; + const [[options_]] = Object.values(await getOptionGroups([options && options.filePath], options)); + const {filePath, warnIgnored, eslintOptions, isQuiet} = options_; const {cwd, baseConfig: {ignorePatterns}} = eslintOptions; if (typeof filePath !== 'string' && !isEqual(getIgnores({}), ignorePatterns)) { @@ -65,13 +66,7 @@ const lintText = async (string, options) => { const lintFiles = async (patterns, options) => { const files = await globFiles(patterns, options); - const allOptions = await Promise.all( - files.map(filePath => parseOptions({...options, filePath})), - ); - - // Files with same `xoConfigPath` can lint together - // https://github.com/xojs/xo/issues/599 - const groups = groupBy(allOptions, 'eslintConfigId'); + const groups = await getOptionGroups(files, options); const reports = await Promise.all( Object.values(groups) diff --git a/lib/options-manager.js b/lib/options-manager.js index 7ebf3597..45c313bf 100644 --- a/lib/options-manager.js +++ b/lib/options-manager.js @@ -11,11 +11,10 @@ import semver from 'semver'; import {cosmiconfig, defaultLoaders} from 'cosmiconfig'; import micromatch from 'micromatch'; import JSON5 from 'json5'; -import toAbsoluteGlob from 'to-absolute-glob'; import stringify from 'json-stable-stringify-without-jsonify'; -import murmur from 'imurmurhash'; import {Legacy} from '@eslint/eslintrc'; import createEsmUtils from 'esm-utils'; +import MurmurHash3 from 'imurmurhash'; import { DEFAULT_IGNORES, DEFAULT_EXTENSION, @@ -42,7 +41,6 @@ resolveFrom.silent = (moduleId, fromDirectory) => { const resolveLocalConfig = name => resolveModule(normalizePackageName(name, 'eslint-config'), import.meta.url); -const nodeVersion = process && process.version; const cacheLocation = cwd => findCacheDir({name: CACHE_DIR_NAME, cwd}) || path.join(os.homedir() || os.tmpdir(), '.xo-cache/'); const DEFAULT_CONFIG = { @@ -109,93 +107,92 @@ const mergeWithFileConfig = async options => { } const searchPath = options.filePath || options.cwd; - const {config: xoOptions, filepath: xoConfigPath} = (await configExplorer.search(searchPath)) || {}; const {config: enginesOptions} = (await pkgConfigExplorer.search(searchPath)) || {}; - options = mergeOptions(options, xoOptions, enginesOptions); + options = normalizeOptions({ + ...xoOptions, + ...(enginesOptions && enginesOptions.node && semver.validRange(enginesOptions.node) ? {nodeVersion: enginesOptions.node} : {}), + ...options, + }); + options.extensions = [...DEFAULT_EXTENSION, ...(options.extensions || [])]; + options.ignores = getIgnores(options); options.cwd = xoConfigPath && path.dirname(xoConfigPath) !== options.cwd ? path.resolve(options.cwd, path.dirname(xoConfigPath)) : options.cwd; - // Very simple way to ensure eslint is ran minimal times across - // all linted files, once for each unique configuration - xo config path + override hash + tsconfig path - let eslintConfigId = xoConfigPath; + // Ensure eslint is ran minimal times across all linted files, once for each unique configuration + // incremental hash of: xo config path + override hash + tsconfig path + // ensures unique configurations + options.eslintConfigId = new MurmurHash3(xoConfigPath); if (options.filePath) { const overrides = applyOverrides(options.filePath, options); options = overrides.options; if (overrides.hash) { - eslintConfigId += overrides.hash; + options.eslintConfigId = options.eslintConfigId.hash(`${overrides.hash}`); } } const prettierOptions = options.prettier ? await prettier.resolveConfig(searchPath, {editorconfig: true}) || {} : {}; if (options.filePath && isTypescript(options.filePath)) { - // We can skip looking up the tsconfig if we have it defined - // in our parser options already. Otherwise we can look it up and create it as normal - const {project: tsConfigProjectPath, tsconfigRootDir} = options.parserOptions || {}; - - let tsConfig; - let tsConfigPath; - if (tsConfigProjectPath) { - tsConfigPath = path.resolve(options.cwd, tsConfigProjectPath); - tsConfig = await json.load(tsConfigPath); - } else { - const tsConfigExplorer = cosmiconfig([], { - searchPlaces: ['tsconfig.json'], - loaders: {'.json': (_, content) => JSON5.parse(content)}, - stopDir: tsconfigRootDir, - }); - const searchResults = (await tsConfigExplorer.search(options.filePath)) || {}; - tsConfigPath = searchResults.filepath; - tsConfig = searchResults.config; - } - - if (tsConfigPath) { - options.tsConfigPath = tsConfigPath; - eslintConfigId += tsConfigPath; - } else { - const {path: tsConfigCachePath, hash: tsConfigHash} = await getTsConfigCachePath([eslintConfigId], tsConfigPath, options.cwd); - eslintConfigId += tsConfigHash; - options.tsConfigPath = tsConfigCachePath; - const config = makeTSConfig(tsConfig, tsConfigPath, [options.filePath]); - await fs.mkdir(path.dirname(options.tsConfigPath), {recursive: true}); - await fs.writeFile(options.tsConfigPath, JSON.stringify(config)); - } - - options.ts = true; + options = await handleTSConfig(options); } - return {options, prettierOptions, eslintConfigId}; -}; + // Ensure this field ends up as a string + options.eslintConfigId = options.eslintConfigId.result(); -/** -Generate a unique and consistent path for the temporary `tsconfig.json`. -Hashing based on https://github.com/eslint/eslint/blob/cf38d0d939b62f3670cdd59f0143fd896fccd771/lib/cli-engine/lint-result-cache.js#L30 -*/ -const getTsConfigCachePath = async (files, tsConfigPath, cwd) => { - const {version} = await json.load('../package.json'); - const tsConfigHash = murmur(`${version}_${nodeVersion}_${stringify({files: files.sort(), tsConfigPath})}`).result().toString(36); - return { - path: path.join( - cacheLocation(cwd), - `tsconfig.${tsConfigHash}.json`, - ), - hash: tsConfigHash, - }; + return {options, prettierOptions}; }; -const makeTSConfig = (tsConfig, tsConfigPath, files) => { - const config = {files: files.filter(file => isTypescript(file))}; - - if (tsConfig) { - config.extends = tsConfigPath; - config.include = arrify(tsConfig.include).map(pattern => toAbsoluteGlob(pattern, {cwd: path.dirname(tsConfigPath)})); +/** + * Find the tsconfig or create a default config + * If a config is found but it doesn't cover the file as needed by parserOptions.project + * we create a temp config for that file that extends the found config. If no config is found + * for a file we apply a default config. + */ +const handleTSConfig = async options => { + // We can skip looking up the tsconfig if we have it defined + // in our parser options already. Otherwise we can look it up and create it as normal + options.ts = true; + options.tsConfig = {}; + options.tsConfigPath = ''; + + const {project: tsConfigProjectPath, tsconfigRootDir} = options.parserOptions || {}; + + if (tsConfigProjectPath) { + options.tsConfigPath = path.resolve(options.cwd, tsConfigProjectPath); + options.tsConfig = await json.load(options.tsConfigPath); } else { - Object.assign(config, TSCONFIG_DEFAULTS); + const tsConfigExplorer = cosmiconfig([], { + searchPlaces: ['tsconfig.json'], + loaders: {'.json': (_, content) => JSON5.parse(content)}, + stopDir: tsconfigRootDir, + }); + const searchResults = (await tsConfigExplorer.search(options.filePath)) || {}; + options.tsConfigPath = searchResults.filepath; + options.tsConfig = searchResults.config; } - return config; + // If there is no files of include property - ts uses **/* as default so all TS files are matched + // TODO: Improve this matching - however, even if we get it wrong, it should still lint correctly as it will just extend the nearest tsconfig + const hasMatch = options.tsConfig && !options.tsConfig.include && !options.tsConfig.files ? true : micromatch.contains(options.filePath, [ + ...(options.tsConfig && Array.isArray(options.tsConfig.include) ? options.tsConfig.include : []), + ...(options.tsConfig && Array.isArray(options.tsConfig.files) ? options.tsConfig.files : []), + ]); + + if (!hasMatch) { + // Only use our default tsconfig if no other tsconfig is found - otherwise extend the found config for linting + options.tsConfig = options.tsConfigPath ? {extends: options.tsConfigPath} : TSCONFIG_DEFAULTS; + options.tsConfigHash = new MurmurHash3(stringify(options.tsConfig)).result(); + options.tsConfigPath = path.join( + cacheLocation(options.cwd), + `tsconfig.${options.tsConfigHash}.json`, + ); + } + + options.eslintConfigId = options.eslintConfigId.hash(options.tsConfigPath); + + return options; }; const normalizeOptions = options => { @@ -235,22 +232,6 @@ const normalizeOptions = options => { const normalizeSpaces = options => typeof options.space === 'number' ? options.space : 2; -/** -Merge option passed via CLI/API via options found in config files. -*/ -const mergeOptions = (options, xoOptions = {}, enginesOptions = {}) => { - const mergedOptions = normalizeOptions({ - ...xoOptions, - ...(enginesOptions && enginesOptions.node && semver.validRange(enginesOptions.node) ? {nodeVersion: enginesOptions.node} : {}), - ...options, - }); - - mergedOptions.extensions = [...DEFAULT_EXTENSION, ...(mergedOptions.extensions || [])]; - mergedOptions.ignores = getIgnores(mergedOptions); - - return mergedOptions; -}; - /** Transform an XO options into ESLint compatible options: - Apply rules based on XO options (e.g `spaces` => `indent` rules or `semicolon` => `semi` rule). @@ -487,9 +468,6 @@ const buildTSConfig = options => config => { ? options.parserOptions.projectFolderIgnoreList : [new RegExp(`/node_modules/(?!.*\\.cache/${CACHE_DIR_NAME})`)], }; - - delete config.tsConfigPath; - delete config.ts; } return config; @@ -576,7 +554,8 @@ const gatherImportResolvers = options => { const parseOptions = async options => { options = normalizeOptions(options); - const {options: foundOptions, prettierOptions, eslintConfigId} = await mergeWithFileConfig(options); + const {options: foundOptions, prettierOptions} = await mergeWithFileConfig(options); + const {eslintConfigId, tsConfigHash, tsConfig, tsConfigPath} = foundOptions; const {filePath, warnIgnored, ...eslintOptions} = buildConfig(foundOptions, prettierOptions); return { filePath, @@ -584,9 +563,50 @@ const parseOptions = async options => { isQuiet: options.quiet, eslintOptions, eslintConfigId, + tsConfigHash, + tsConfigPath, + tsConfig, }; }; +const getOptionGroups = async (files, options) => { + const allOptions = await Promise.all( + arrify(files).map(filePath => parseOptions({...options, filePath})), + ); + + const tsGroups = {}; + const optionGroups = {}; + for (const options of allOptions) { + if (Array.isArray(optionGroups[options.eslintConfigId])) { + optionGroups[options.eslintConfigId].push(options); + } else { + optionGroups[options.eslintConfigId] = [options]; + } + + if (options.tsConfigHash) { + if (Array.isArray(tsGroups[options.tsConfigHash])) { + tsGroups[options.tsConfigHash].push(options); + } else { + tsGroups[options.tsConfigHash] = [options]; + } + } + } + + await Promise.all(Object.values(tsGroups).map(async tsGroup => { + await fs.mkdir(path.dirname(tsGroup[0].tsConfigPath), {recursive: true}); + await fs.writeFile(tsGroup[0].tsConfigPath, JSON.stringify({ + ...tsGroup[0].tsConfig, + files: tsGroup.map(o => o.filePath), + include: [], + exclude: [], + })); + })); + + // Files with same `xoConfigPath` can lint together + // https://github.com/xojs/xo/issues/599 + return optionGroups; +}; + export { parseOptions, getIgnores, @@ -598,4 +618,6 @@ export { mergeWithPrettierConfig, normalizeOptions, buildConfig, + getOptionGroups, + handleTSConfig, }; diff --git a/test/fixtures/typescript/extends-config/package.json b/test/fixtures/typescript/extends-config/package.json new file mode 100644 index 00000000..90bd27c7 --- /dev/null +++ b/test/fixtures/typescript/extends-config/package.json @@ -0,0 +1,3 @@ +{ + "xo": {} +} diff --git a/test/fixtures/typescript/extends-config/tsconfig.json b/test/fixtures/typescript/extends-config/tsconfig.json new file mode 100644 index 00000000..3cff3fa9 --- /dev/null +++ b/test/fixtures/typescript/extends-config/tsconfig.json @@ -0,0 +1,3 @@ +{ + "include": ["foo.ts"] +} diff --git a/test/lint-files.js b/test/lint-files.js index e2b55990..fab99a84 100644 --- a/test/lint-files.js +++ b/test/lint-files.js @@ -187,7 +187,7 @@ test('find configurations close to linted file', async t => { ); }); -test('typescript files', async t => { +test.serial('typescript files', async t => { const {results} = await xo.lintFiles('**/*', {cwd: 'fixtures/typescript'}); t.true( @@ -215,18 +215,18 @@ test('typescript files', async t => { ); }); -test('typescript 2 space option', async t => { +test.serial('typescript 2 space option', async t => { const {errorCount, results} = await xo.lintFiles('two-spaces.tsx', {cwd: 'fixtures/typescript', space: 2}); // eslint-disable-next-line ava/assertion-arguments t.is(errorCount, 0, JSON.stringify(results[0].messages)); }); -test('typescript 4 space option', async t => { +test.serial('typescript 4 space option', async t => { const {errorCount} = await xo.lintFiles('child/sub-child/four-spaces.ts', {cwd: 'fixtures/typescript', space: 4}); t.is(errorCount, 0); }); -test('typescript no semicolon option', async t => { +test.serial('typescript no semicolon option', async t => { const {errorCount} = await xo.lintFiles('child/no-semicolon.ts', {cwd: 'fixtures/typescript', semicolon: false}); t.is(errorCount, 0); }); diff --git a/test/options-manager.js b/test/options-manager.js index 6988e98d..59c2b3ab 100644 --- a/test/options-manager.js +++ b/test/options-manager.js @@ -3,7 +3,8 @@ import path from 'node:path'; import test from 'ava'; import slash from 'slash'; import createEsmUtils from 'esm-utils'; -import {DEFAULT_EXTENSION, DEFAULT_IGNORES} from '../lib/constants.js'; +import MurmurHash3 from 'imurmurhash'; +import {DEFAULT_EXTENSION, DEFAULT_IGNORES, TSCONFIG_DEFAULTS} from '../lib/constants.js'; import * as manager from '../lib/options-manager.js'; const {__dirname, require, json} = createEsmUtils(import.meta); @@ -494,14 +495,16 @@ test('findApplicableOverrides', t => { test('mergeWithFileConfig: use child if closest', async t => { const cwd = path.resolve('fixtures', 'nested', 'child'); const {options} = await manager.mergeWithFileConfig({cwd}); - const expected = {...childConfig.xo, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd}; + const eslintConfigId = new MurmurHash3(path.join(cwd, 'package.json')).result(); + const expected = {...childConfig.xo, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, eslintConfigId}; t.deepEqual(options, expected); }); test('mergeWithFileConfig: use parent if closest', async t => { const cwd = path.resolve('fixtures', 'nested'); const {options} = await manager.mergeWithFileConfig({cwd}); - const expected = {...parentConfig.xo, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd}; + const eslintConfigId = new MurmurHash3(path.join(cwd, 'package.json')).result(); + const expected = {...parentConfig.xo, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, eslintConfigId}; t.deepEqual(options, expected); }); @@ -509,34 +512,39 @@ test('mergeWithFileConfig: use parent if child is ignored', async t => { const cwd = path.resolve('fixtures', 'nested'); const filePath = path.resolve(cwd, 'child-ignore', 'file.js'); const {options} = await manager.mergeWithFileConfig({cwd, filePath}); - const expected = {...parentConfig.xo, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, filePath}; + const eslintConfigId = new MurmurHash3(path.join(cwd, 'package.json')).result(); + const expected = {...parentConfig.xo, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, filePath, eslintConfigId}; t.deepEqual(options, expected); }); test('mergeWithFileConfig: use child if child is empty', async t => { const cwd = path.resolve('fixtures', 'nested', 'child-empty'); const {options} = await manager.mergeWithFileConfig({cwd}); - t.deepEqual(options, {extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd}); + const eslintConfigId = new MurmurHash3(path.join(cwd, 'package.json')).result(); + t.deepEqual(options, {extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, eslintConfigId}); }); test('mergeWithFileConfig: read engines from package.json', async t => { const cwd = path.resolve('fixtures', 'engines'); const {options} = await manager.mergeWithFileConfig({cwd}); - const expected = {nodeVersion: enginesConfig.engines.node, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd}; + const eslintConfigId = new MurmurHash3().result(); + const expected = {nodeVersion: enginesConfig.engines.node, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, eslintConfigId}; t.deepEqual(options, expected); }); test('mergeWithFileConfig: XO engine options supersede package.json\'s', async t => { const cwd = path.resolve('fixtures', 'engines'); const {options} = await manager.mergeWithFileConfig({cwd, nodeVersion: '>=8'}); - const expected = {nodeVersion: '>=8', extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd}; + const eslintConfigId = new MurmurHash3().result(); + const expected = {nodeVersion: '>=8', extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, eslintConfigId}; t.deepEqual(options, expected); }); test('mergeWithFileConfig: XO engine options false supersede package.json\'s', async t => { const cwd = path.resolve('fixtures', 'engines'); const {options} = await manager.mergeWithFileConfig({cwd, nodeVersion: false}); - const expected = {nodeVersion: false, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd}; + const eslintConfigId = new MurmurHash3().result(); + const expected = {nodeVersion: false, extensions: DEFAULT_EXTENSION, ignores: DEFAULT_IGNORES, cwd, eslintConfigId}; t.deepEqual(options, expected); }); @@ -544,7 +552,9 @@ test('mergeWithFileConfig: resolves expected typescript file options', async t = const cwd = path.resolve('fixtures', 'typescript', 'child'); const filePath = path.resolve(cwd, 'file.ts'); const tsConfigPath = path.resolve(cwd, 'tsconfig.json'); + const tsConfig = await json.load(tsConfigPath); const {options} = await manager.mergeWithFileConfig({cwd, filePath}); + const eslintConfigId = new MurmurHash3(path.resolve(cwd, 'package.json')).hash(tsConfigPath).result(); const expected = { filePath, extensions: DEFAULT_EXTENSION, @@ -553,6 +563,8 @@ test('mergeWithFileConfig: resolves expected typescript file options', async t = semicolon: false, ts: true, tsConfigPath, + eslintConfigId, + tsConfig, }; t.deepEqual(options, expected); }); @@ -562,6 +574,8 @@ test('mergeWithFileConfig: resolves expected tsx file options', async t => { const filePath = path.resolve(cwd, 'file.tsx'); const {options} = await manager.mergeWithFileConfig({cwd, filePath}); const tsConfigPath = path.resolve(cwd, 'tsconfig.json'); + const tsConfig = await json.load(tsConfigPath); + const eslintConfigId = new MurmurHash3(path.join(cwd, 'package.json')).hash(tsConfigPath).result(); const expected = { filePath, extensions: DEFAULT_EXTENSION, @@ -570,6 +584,8 @@ test('mergeWithFileConfig: resolves expected tsx file options', async t => { semicolon: false, ts: true, tsConfigPath, + eslintConfigId, + tsConfig, }; t.deepEqual(options, expected); }); @@ -582,12 +598,25 @@ test('mergeWithFileConfig: uses specified parserOptions.project as tsconfig', as t.is(options.tsConfigPath, expectedTsConfigPath); }); +test('mergeWithFileConfig: extends ts config if needed', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'extends-config'); + const filePath = path.resolve(cwd, 'does-not-matter.ts'); + const expectedConfigPath = new RegExp(`${slash(cwd)}/node_modules/.cache/xo-linter/tsconfig\\..*\\.json[\\/]?$`, 'u'); + const expected = { + extends: path.resolve(cwd, 'tsconfig.json'), + }; + const {options} = await manager.mergeWithFileConfig({cwd, filePath}); + t.regex(slash(options.tsConfigPath), expectedConfigPath); + t.deepEqual(expected, options.tsConfig); +}); + test('mergeWithFileConfig: creates temp tsconfig if none present', async t => { const cwd = path.resolve('fixtures', 'typescript'); const expectedConfigPath = new RegExp(`${slash(cwd)}/node_modules/.cache/xo-linter/tsconfig\\..*\\.json[\\/]?$`, 'u'); const filePath = path.resolve(cwd, 'does-not-matter.ts'); const {options} = await manager.mergeWithFileConfig({cwd, filePath}); t.regex(slash(options.tsConfigPath), expectedConfigPath); + t.deepEqual(options.tsConfig, TSCONFIG_DEFAULTS); }); test('applyOverrides', t => {