From f606c2eaa30d42d5c196def8f9e4a648d634b55d Mon Sep 17 00:00:00 2001 From: Spencer Snyder Date: Sun, 7 Aug 2022 14:33:06 -0400 Subject: [PATCH 1/3] feat: full tsconfig resolution (closes #637 closes #661) --- lib/options-manager.js | 93 +++++++++++++++++-- .../deep-extends/config/tsconfig.json | 4 + .../typescript/deep-extends/package.json | 3 + .../fixtures/typescript/excludes/package.json | 3 + .../typescript/excludes/tsconfig.json | 3 + .../parseroptions-project/tsconfig.json | 3 +- .../relative-configs/config/tsconfig.json | 4 + .../typescript/relative-configs/package.json | 7 ++ test/options-manager.js | 60 +++++++++++- 9 files changed, 169 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/typescript/deep-extends/config/tsconfig.json create mode 100644 test/fixtures/typescript/deep-extends/package.json create mode 100644 test/fixtures/typescript/excludes/package.json create mode 100644 test/fixtures/typescript/excludes/tsconfig.json create mode 100644 test/fixtures/typescript/relative-configs/config/tsconfig.json create mode 100644 test/fixtures/typescript/relative-configs/package.json diff --git a/lib/options-manager.js b/lib/options-manager.js index 79e38d44..42021aaf 100644 --- a/lib/options-manager.js +++ b/lib/options-manager.js @@ -173,12 +173,40 @@ const handleTSConfig = async options => { options.tsConfig = searchResults.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 (options.tsConfig) { + // If the tsconfig extends from another file, we need to ensure that the file is covered by the tsconfig + // or not. The basefile could have includes/excludes/files properties that should be applied to the final tsconfig representation. + options.tsConfig = await recursiveBuildTsConfig(options.tsConfig, options.tsConfigPath); + } + + let hasMatch; + + // If there is no files or include property - ts uses **/* as default so all TS files are matched + // in tsconfig, excludes override includes - so we need to prioritize that matching logic + if ( + options.tsConfig + && !options.tsConfig.include + && !options.tsConfig.files + ) { + // If we have an excludes property, we need to check it + // if we match on excluded, then we definitively know that there is no tsconfig match + if (Array.isArray(options.tsConfig.exclude)) { + const exclude = options.tsConfig && Array.isArray(options.tsConfig.exclude) ? options.tsConfig.exclude : []; + hasMatch = !micromatch.contains(options.filePath, exclude); + } else { + // Not explicitly excluded and included by tsconfig defaults + hasMatch = true; + } + } else { + // We have either and include or a files property in tsconfig + const include = options.tsConfig && Array.isArray(options.tsConfig.include) ? options.tsConfig.include : []; + const files = options.tsConfig && Array.isArray(options.tsConfig.files) ? options.tsConfig.files : []; + const exclude = options.tsConfig && Array.isArray(options.tsConfig.exclude) ? options.tsConfig.exclude : []; + // If we also have an exlcude we need to check all the arrays, (files, include, exclude) + // this check not excluded and included in one of the file/include array + hasMatch = !micromatch.contains(options.filePath, exclude) + && micromatch.contains(options.filePath, [...include, ...files]); + } if (!hasMatch) { // Only use our default tsconfig if no other tsconfig is found - otherwise extend the found config for linting @@ -607,6 +635,58 @@ const getOptionGroups = async (files, options) => { return optionGroups; }; +async function recursiveBuildTsConfig(tsConfig, tsConfigPath) { + tsConfig = tsConfigAbsPaths(tsConfig, tsConfigPath); + + if (!tsConfig.extends || (typeof tsConfig.extends === 'string' && tsConfig.extends.includes('node_modules'))) { + return tsConfig; + } + + // If any of the following are missing, then we need to look up the base config as it could apply + const basePath = path.isAbsolute(tsConfig.extends) + ? tsConfig.extends + : path.resolve(path.dirname(tsConfigPath), tsConfig.extends); + + const baseTsConfig = await readJson(basePath); + + delete tsConfig.extends; + + tsConfig = { + compilerOptions: { + ...baseTsConfig.compilerOptions, + ...tsConfig.compilerOptions, + }, + ...baseTsConfig, + ...tsConfig, + }; + + return recursiveBuildTsConfig(tsConfig, basePath); +} + +// Convert all include, files, and exclude to absolute paths +// and or globs. This works because ts only allows simple glob subset +const tsConfigAbsPaths = (tsConfig, tsConfigPath) => { + if (Array.isArray(tsConfig.files)) { + tsConfig.files = tsConfig.files.map( + f => path.isAbsolute(f) ? f : path.resolve(path.dirname(tsConfigPath), f), + ); + } + + if (Array.isArray(tsConfig.include)) { + tsConfig.include = tsConfig.include.map( + f => path.isAbsolute(f) ? f : path.resolve(path.dirname(tsConfigPath), f), + ); + } + + if (Array.isArray(tsConfig.exclude)) { + tsConfig.exclude = tsConfig.exclude.map( + f => path.isAbsolute(f) ? f : path.resolve(path.dirname(tsConfigPath), f), + ); + } + + return tsConfig; +}; + export { parseOptions, getIgnores, @@ -620,4 +700,5 @@ export { buildConfig, getOptionGroups, handleTSConfig, + tsConfigAbsPaths, }; diff --git a/test/fixtures/typescript/deep-extends/config/tsconfig.json b/test/fixtures/typescript/deep-extends/config/tsconfig.json new file mode 100644 index 00000000..4712350c --- /dev/null +++ b/test/fixtures/typescript/deep-extends/config/tsconfig.json @@ -0,0 +1,4 @@ +{ + "include": ["../included-file.ts"], + "exclude": ["../excluded-file.ts"] +} diff --git a/test/fixtures/typescript/deep-extends/package.json b/test/fixtures/typescript/deep-extends/package.json new file mode 100644 index 00000000..90bd27c7 --- /dev/null +++ b/test/fixtures/typescript/deep-extends/package.json @@ -0,0 +1,3 @@ +{ + "xo": {} +} diff --git a/test/fixtures/typescript/excludes/package.json b/test/fixtures/typescript/excludes/package.json new file mode 100644 index 00000000..90bd27c7 --- /dev/null +++ b/test/fixtures/typescript/excludes/package.json @@ -0,0 +1,3 @@ +{ + "xo": {} +} diff --git a/test/fixtures/typescript/excludes/tsconfig.json b/test/fixtures/typescript/excludes/tsconfig.json new file mode 100644 index 00000000..b3ec6cf5 --- /dev/null +++ b/test/fixtures/typescript/excludes/tsconfig.json @@ -0,0 +1,3 @@ +{ + "exclude": ["excluded-file.ts"] +} diff --git a/test/fixtures/typescript/parseroptions-project/tsconfig.json b/test/fixtures/typescript/parseroptions-project/tsconfig.json index 10671361..c203c4be 100644 --- a/test/fixtures/typescript/parseroptions-project/tsconfig.json +++ b/test/fixtures/typescript/parseroptions-project/tsconfig.json @@ -1,3 +1,4 @@ { - "include": ["**/*.ts", "**/*.tsx"] + "include": ["included-file.ts"], + "exclude": ["excluded-file.ts"] } diff --git a/test/fixtures/typescript/relative-configs/config/tsconfig.json b/test/fixtures/typescript/relative-configs/config/tsconfig.json new file mode 100644 index 00000000..4712350c --- /dev/null +++ b/test/fixtures/typescript/relative-configs/config/tsconfig.json @@ -0,0 +1,4 @@ +{ + "include": ["../included-file.ts"], + "exclude": ["../excluded-file.ts"] +} diff --git a/test/fixtures/typescript/relative-configs/package.json b/test/fixtures/typescript/relative-configs/package.json new file mode 100644 index 00000000..da497d78 --- /dev/null +++ b/test/fixtures/typescript/relative-configs/package.json @@ -0,0 +1,7 @@ +{ + "xo": { + "parserOptions": { + "project": "./config/tsconfig.json" + } + } +} diff --git a/test/options-manager.js b/test/options-manager.js index 98042e06..d5752cdc 100644 --- a/test/options-manager.js +++ b/test/options-manager.js @@ -564,7 +564,7 @@ test('mergeWithFileConfig: resolves expected typescript file options', async t = ts: true, tsConfigPath, eslintConfigId, - tsConfig, + tsConfig: manager.tsConfigAbsPaths(tsConfig, tsConfigPath), }; t.deepEqual(options, expected); }); @@ -585,20 +585,52 @@ test('mergeWithFileConfig: resolves expected tsx file options', async t => { ts: true, tsConfigPath, eslintConfigId, - tsConfig, + tsConfig: manager.tsConfigAbsPaths(tsConfig, tsConfigPath), }; t.deepEqual(options, expected); }); test('mergeWithFileConfig: uses specified parserOptions.project as tsconfig', async t => { const cwd = path.resolve('fixtures', 'typescript', 'parseroptions-project'); - const filePath = path.resolve(cwd, 'does-not-matter.ts'); + const filePath = path.resolve(cwd, 'included-file.ts'); const expectedTsConfigPath = path.resolve(cwd, 'projectconfig.json'); const {options} = await manager.mergeWithFileConfig({cwd, filePath}); t.is(options.tsConfigPath, expectedTsConfigPath); }); -test('mergeWithFileConfig: extends ts config if needed', async t => { +test('mergeWithFileConfig: correctly resolves relative tsconfigs excluded file', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'relative-configs'); + const excludedFilePath = path.resolve(cwd, 'excluded-file.ts'); + const excludeTsConfigPath = new RegExp(`${slash(cwd)}/node_modules/.cache/xo-linter/tsconfig\\..*\\.json[\\/]?$`, 'u'); + const {options} = await manager.mergeWithFileConfig({cwd, filePath: excludedFilePath}); + t.regex(options.tsConfigPath, excludeTsConfigPath); +}); + +test('mergeWithFileConfig: correctly resolves relative tsconfigs included file', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'relative-configs'); + const includedFilePath = path.resolve(cwd, 'included-file.ts'); + const includeTsConfigPath = path.resolve(cwd, 'config/tsconfig.json'); + const {options} = await manager.mergeWithFileConfig({cwd, filePath: includedFilePath}); + t.is(options.tsConfigPath, includeTsConfigPath); +}); + +test('mergeWithFileConfig: uses generated tsconfig if specified parserOptions.project excludes file', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'parseroptions-project'); + const filePath = path.resolve(cwd, 'excluded-file.ts'); + const expectedTsConfigPath = new RegExp(`${slash(cwd)}/node_modules/.cache/xo-linter/tsconfig\\..*\\.json[\\/]?$`, 'u'); + const {options} = await manager.mergeWithFileConfig({cwd, filePath}); + t.regex(options.tsConfigPath, expectedTsConfigPath); +}); + +test('mergeWithFileConfig: uses generated tsconfig if specified parserOptions.project misses file', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'parseroptions-project'); + const filePath = path.resolve(cwd, 'missed-by-options-file.ts'); + const expectedTsConfigPath = new RegExp(`${slash(cwd)}/node_modules/.cache/xo-linter/tsconfig\\..*\\.json[\\/]?$`, 'u'); + const {options} = await manager.mergeWithFileConfig({cwd, filePath}); + t.regex(options.tsConfigPath, expectedTsConfigPath); +}); + +test('mergeWithFileConfig: auto generated ts config extends found ts config if file is not covered', 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'); @@ -610,6 +642,26 @@ test('mergeWithFileConfig: extends ts config if needed', async t => { t.deepEqual(expected, options.tsConfig); }); +test('mergeWithFileConfig: used found ts config if file is covered', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'extends-config'); + const filePath = path.resolve(cwd, 'foo.ts'); + const expectedConfigPath = path.resolve(cwd, 'tsconfig.json'); + const {options} = await manager.mergeWithFileConfig({cwd, filePath}); + t.is(slash(options.tsConfigPath), expectedConfigPath); +}); + +test('mergeWithFileConfig: auto generated ts config extends found ts config if file is explicitly excluded', async t => { + const cwd = path.resolve('fixtures', 'typescript', 'excludes'); + const filePath = path.resolve(cwd, 'excluded-file.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'); From f48456690e723396005d48415013217196453b08 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Fri, 19 Aug 2022 16:47:05 +0200 Subject: [PATCH 2/3] Update options-manager.js --- lib/options-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/options-manager.js b/lib/options-manager.js index 42021aaf..6ae4f9bf 100644 --- a/lib/options-manager.js +++ b/lib/options-manager.js @@ -189,7 +189,7 @@ const handleTSConfig = async options => { && !options.tsConfig.files ) { // If we have an excludes property, we need to check it - // if we match on excluded, then we definitively know that there is no tsconfig match + // If we match on excluded, then we definitively know that there is no tsconfig match if (Array.isArray(options.tsConfig.exclude)) { const exclude = options.tsConfig && Array.isArray(options.tsConfig.exclude) ? options.tsConfig.exclude : []; hasMatch = !micromatch.contains(options.filePath, exclude); From d7361fadb99b0bc09842c0b4878f7a2b6433821c Mon Sep 17 00:00:00 2001 From: Spencer Snyder Date: Mon, 22 Aug 2022 17:27:08 -0400 Subject: [PATCH 3/3] chore: improve resolving ts config paths --- lib/options-manager.js | 14 ++++++++------ test/options-manager.js | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/options-manager.js b/lib/options-manager.js index 6ae4f9bf..2a59705c 100644 --- a/lib/options-manager.js +++ b/lib/options-manager.js @@ -636,7 +636,7 @@ const getOptionGroups = async (files, options) => { }; async function recursiveBuildTsConfig(tsConfig, tsConfigPath) { - tsConfig = tsConfigAbsPaths(tsConfig, tsConfigPath); + tsConfig = tsConfigResolvePaths(tsConfig, tsConfigPath); if (!tsConfig.extends || (typeof tsConfig.extends === 'string' && tsConfig.extends.includes('node_modules'))) { return tsConfig; @@ -665,22 +665,24 @@ async function recursiveBuildTsConfig(tsConfig, tsConfigPath) { // Convert all include, files, and exclude to absolute paths // and or globs. This works because ts only allows simple glob subset -const tsConfigAbsPaths = (tsConfig, tsConfigPath) => { +const tsConfigResolvePaths = (tsConfig, tsConfigPath) => { + const tsConfigDirectory = path.dirname(tsConfigPath); + if (Array.isArray(tsConfig.files)) { tsConfig.files = tsConfig.files.map( - f => path.isAbsolute(f) ? f : path.resolve(path.dirname(tsConfigPath), f), + filePath => path.resolve(tsConfigDirectory, filePath), ); } if (Array.isArray(tsConfig.include)) { tsConfig.include = tsConfig.include.map( - f => path.isAbsolute(f) ? f : path.resolve(path.dirname(tsConfigPath), f), + globPath => path.resolve(tsConfigDirectory, globPath), ); } if (Array.isArray(tsConfig.exclude)) { tsConfig.exclude = tsConfig.exclude.map( - f => path.isAbsolute(f) ? f : path.resolve(path.dirname(tsConfigPath), f), + globPath => path.resolve(tsConfigDirectory, globPath), ); } @@ -700,5 +702,5 @@ export { buildConfig, getOptionGroups, handleTSConfig, - tsConfigAbsPaths, + tsConfigResolvePaths, }; diff --git a/test/options-manager.js b/test/options-manager.js index d5752cdc..9eeef13d 100644 --- a/test/options-manager.js +++ b/test/options-manager.js @@ -564,7 +564,7 @@ test('mergeWithFileConfig: resolves expected typescript file options', async t = ts: true, tsConfigPath, eslintConfigId, - tsConfig: manager.tsConfigAbsPaths(tsConfig, tsConfigPath), + tsConfig: manager.tsConfigResolvePaths(tsConfig, tsConfigPath), }; t.deepEqual(options, expected); }); @@ -585,7 +585,7 @@ test('mergeWithFileConfig: resolves expected tsx file options', async t => { ts: true, tsConfigPath, eslintConfigId, - tsConfig: manager.tsConfigAbsPaths(tsConfig, tsConfigPath), + tsConfig: manager.tsConfigResolvePaths(tsConfig, tsConfigPath), }; t.deepEqual(options, expected); });