From 8ab89926c251fc5e32ae0caf75282041a301e014 Mon Sep 17 00:00:00 2001 From: Lars Kappert Date: Sun, 24 Sep 2023 10:02:43 +0200 Subject: [PATCH] Support non-entry cross-reference imports in workspaces (resolves #244) --- .../@workspaces-cross-reference/lib-a | 1 + .../@workspaces-cross-reference/lib-b | 1 + .../workspaces-cross-reference/package.json | 6 + .../packages/lib-a/index.ts | 2 + .../packages/lib-a/mod-a.ts | 7 ++ .../packages/lib-a/package.json | 6 + .../packages/lib-a/tsconfig.json | 5 + .../packages/lib-b/index.ts | 1 + .../packages/lib-b/mod-b.ts | 7 ++ .../packages/lib-b/package.json | 6 + .../packages/lib-b/tsconfig.json | 5 + src/ConfigurationChief.ts | 4 +- src/PrincipalFactory.ts | 20 +-- src/ProjectPrincipal.ts | 10 +- src/index.ts | 114 +++++++++++------- tests/workspaces-cross-reference.test.ts | 25 ++++ 16 files changed, 163 insertions(+), 57 deletions(-) create mode 120000 fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-a create mode 120000 fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-b create mode 100644 fixtures/workspaces-cross-reference/package.json create mode 100644 fixtures/workspaces-cross-reference/packages/lib-a/index.ts create mode 100644 fixtures/workspaces-cross-reference/packages/lib-a/mod-a.ts create mode 100644 fixtures/workspaces-cross-reference/packages/lib-a/package.json create mode 100644 fixtures/workspaces-cross-reference/packages/lib-a/tsconfig.json create mode 100644 fixtures/workspaces-cross-reference/packages/lib-b/index.ts create mode 100644 fixtures/workspaces-cross-reference/packages/lib-b/mod-b.ts create mode 100644 fixtures/workspaces-cross-reference/packages/lib-b/package.json create mode 100644 fixtures/workspaces-cross-reference/packages/lib-b/tsconfig.json create mode 100644 tests/workspaces-cross-reference.test.ts diff --git a/fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-a b/fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-a new file mode 120000 index 000000000..15febe0bc --- /dev/null +++ b/fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-a @@ -0,0 +1 @@ +../../packages/lib-a \ No newline at end of file diff --git a/fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-b b/fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-b new file mode 120000 index 000000000..748356d3f --- /dev/null +++ b/fixtures/workspaces-cross-reference/node_modules/@workspaces-cross-reference/lib-b @@ -0,0 +1 @@ +../../packages/lib-b \ No newline at end of file diff --git a/fixtures/workspaces-cross-reference/package.json b/fixtures/workspaces-cross-reference/package.json new file mode 100644 index 000000000..19c7c40b3 --- /dev/null +++ b/fixtures/workspaces-cross-reference/package.json @@ -0,0 +1,6 @@ +{ + "name": "@workspaces-cross-reference/root", + "workspaces": [ + "packages/*" + ] +} diff --git a/fixtures/workspaces-cross-reference/packages/lib-a/index.ts b/fixtures/workspaces-cross-reference/packages/lib-a/index.ts new file mode 100644 index 000000000..61551b466 --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-a/index.ts @@ -0,0 +1,2 @@ +import a from './mod-a'; +a; diff --git a/fixtures/workspaces-cross-reference/packages/lib-a/mod-a.ts b/fixtures/workspaces-cross-reference/packages/lib-a/mod-a.ts new file mode 100644 index 000000000..432ad96b0 --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-a/mod-a.ts @@ -0,0 +1,7 @@ +import circularA, { B1 } from '@workspaces-cross-reference/lib-b/mod-b'; + +export const A = B1; + +export const unused = true; + +export default circularA; diff --git a/fixtures/workspaces-cross-reference/packages/lib-a/package.json b/fixtures/workspaces-cross-reference/packages/lib-a/package.json new file mode 100644 index 000000000..21f2352ad --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-a/package.json @@ -0,0 +1,6 @@ +{ + "name": "@workspaces-cross-reference/lib-a", + "dependencies": { + "@workspaces-cross-reference/lib-b": "*" + } +} diff --git a/fixtures/workspaces-cross-reference/packages/lib-a/tsconfig.json b/fixtures/workspaces-cross-reference/packages/lib-a/tsconfig.json new file mode 100644 index 000000000..187548bd8 --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-a/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "baseUrl": "./" + } +} diff --git a/fixtures/workspaces-cross-reference/packages/lib-b/index.ts b/fixtures/workspaces-cross-reference/packages/lib-b/index.ts new file mode 100644 index 000000000..aef22247d --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-b/index.ts @@ -0,0 +1 @@ +export default 1; diff --git a/fixtures/workspaces-cross-reference/packages/lib-b/mod-b.ts b/fixtures/workspaces-cross-reference/packages/lib-b/mod-b.ts new file mode 100644 index 000000000..04fca2bf5 --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-b/mod-b.ts @@ -0,0 +1,7 @@ +import { A } from '@workspaces-cross-reference/lib-a/mod-a'; + +export const B1 = 'B1'; + +export const unused = true; + +export default A; diff --git a/fixtures/workspaces-cross-reference/packages/lib-b/package.json b/fixtures/workspaces-cross-reference/packages/lib-b/package.json new file mode 100644 index 000000000..b2cfdbfb0 --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-b/package.json @@ -0,0 +1,6 @@ +{ + "name": "@workspaces-cross-reference/lib-b", + "dependencies": { + "@workspaces-cross-reference/lib-a": "*" + } +} diff --git a/fixtures/workspaces-cross-reference/packages/lib-b/tsconfig.json b/fixtures/workspaces-cross-reference/packages/lib-b/tsconfig.json new file mode 100644 index 000000000..187548bd8 --- /dev/null +++ b/fixtures/workspaces-cross-reference/packages/lib-b/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "baseUrl": "./" + } +} diff --git a/src/ConfigurationChief.ts b/src/ConfigurationChief.ts index 203f249f4..c1a67d448 100644 --- a/src/ConfigurationChief.ts +++ b/src/ConfigurationChief.ts @@ -70,7 +70,7 @@ type ConfigurationManagerOptions = { export type Workspace = { name: string; - pkgName?: string; + pkgName: string; dir: string; ancestors: string[]; config: WorkspaceConfiguration; @@ -316,7 +316,7 @@ export class ConfigurationChief { return workspaceNames.sort(byPathDepth).map( (name): Workspace => ({ name, - pkgName: this.manifestWorkspaces.get(name) ?? this.manifest?.name, + pkgName: this.manifestWorkspaces.get(name) ?? this.manifest?.name ?? `NOT_FOUND_${name}`, dir: join(this.cwd, name), config: this.getConfigForWorkspace(name), ancestors: this.availableWorkspaceNames.reduce(getAncestors(name), []), diff --git a/src/PrincipalFactory.ts b/src/PrincipalFactory.ts index 0f74e355d..76a24d632 100644 --- a/src/PrincipalFactory.ts +++ b/src/PrincipalFactory.ts @@ -5,7 +5,7 @@ import type { SyncCompilers, AsyncCompilers } from './types/compilers.js'; type Paths = ts.CompilerOptions['paths']; -type Principal = { principal: ProjectPrincipal; cwds: Set; pathKeys: Set }; +type Principal = { principal: ProjectPrincipal; cwds: Set; pathKeys: Set; pkgNames: Set }; type Principals = Set; type Options = { @@ -13,6 +13,7 @@ type Options = { compilerOptions: ts.CompilerOptions; paths: Paths; compilers: [SyncCompilers, AsyncCompilers]; + pkgName: string; }; const mergePaths = (cwd: string, compilerOptions: ts.CompilerOptions, paths: Paths = {}) => { @@ -33,11 +34,11 @@ export class PrincipalFactory { principals: Principals = new Set(); public getPrincipal(options: Options) { - const { cwd, compilerOptions, paths } = options; + const { cwd, compilerOptions, paths, pkgName } = options; options.compilerOptions = mergePaths(cwd, compilerOptions, paths); const principal = this.findReusablePrincipal(compilerOptions); if (principal) { - this.linkPrincipal(principal, cwd, compilerOptions); + this.linkPrincipal(principal, cwd, compilerOptions, pkgName); return principal.principal; } else { return this.addNewPrincipal(options); @@ -59,23 +60,28 @@ export class PrincipalFactory { return principal; } - private linkPrincipal(principal: Principal, cwd: string, compilerOptions: ts.CompilerOptions) { + private linkPrincipal(principal: Principal, cwd: string, compilerOptions: ts.CompilerOptions, pkgName: string) { const { pathsBasePath, paths } = compilerOptions; if (pathsBasePath) principal.principal.compilerOptions.pathsBasePath = pathsBasePath; Object.keys(paths ?? {}).forEach(p => principal.pathKeys.add(p)); principal.principal.compilerOptions.paths = { ...principal.principal.compilerOptions.paths, ...paths }; principal.cwds.add(cwd); + principal.pkgNames.add(pkgName); } private addNewPrincipal(options: Options) { - const { cwd, compilerOptions } = options; + const { cwd, compilerOptions, pkgName } = options; const pathKeys = new Set(Object.keys(compilerOptions?.paths ?? {})); const principal = new ProjectPrincipal(options); - this.principals.add({ principal, cwds: new Set([cwd]), pathKeys }); + this.principals.add({ principal, cwds: new Set([cwd]), pathKeys, pkgNames: new Set([pkgName]) }); return principal; } public getPrincipals() { - return Array.from(this.principals, p => p.principal); + return Array.from(this.principals, p => p.principal).reverse(); + } + + public getPrincipalByPackageName(packageName: string) { + return Array.from(this.principals).find(principal => principal.pkgNames.has(packageName))?.principal; } } diff --git a/src/ProjectPrincipal.ts b/src/ProjectPrincipal.ts index 3d3519e16..901e3376f 100644 --- a/src/ProjectPrincipal.ts +++ b/src/ProjectPrincipal.ts @@ -172,7 +172,8 @@ export class ProjectPrincipal { } public analyzeSourceFile(filePath: string, { skipTypeOnly }: { skipTypeOnly: boolean }) { - const sourceFile = this.backend.program?.getSourceFile(filePath); + // We request it from `fileManager` directly as `program` does not contain cross-referenced files + const sourceFile = this.backend.fileManager.getSourceFile(filePath); if (!sourceFile) throw new Error(`Unable to find ${filePath}`); @@ -270,6 +271,11 @@ export class ProjectPrincipal { } private findReferences(filePath: string, pos: number) { - return this.backend.lsFindReferences(filePath, pos) ?? []; + try { + return this.backend.lsFindReferences(filePath, pos) ?? []; + } catch { + // TS throws for (cross-referenced) files not in the program + return []; + } } } diff --git a/src/index.ts b/src/index.ts index 71082f3df..a1320e301 100644 --- a/src/index.ts +++ b/src/index.ts @@ -126,7 +126,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { }; for (const workspace of workspaces) { - const { name, dir, config, ancestors } = workspace; + const { name, dir, config, ancestors, pkgName } = workspace; const { paths, ignoreDependencies, ignoreBinaries } = config; const isRoot = name === ROOT_WORKSPACE_NAME; @@ -142,7 +142,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { const compilerOptions = await loadCompilerOptions(join(dir, tsConfigFile ?? 'tsconfig.json')); - const principal = factory.getPrincipal({ cwd: dir, paths, compilerOptions, compilers }); + const principal = factory.getPrincipal({ cwd: dir, paths, compilerOptions, compilers, pkgName }); const worker = new WorkspaceWorker({ name, @@ -245,23 +245,36 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { debugLog(`Installed ${principals.length} principals for ${workspaces.length} workspaces`); - for (const principal of principals) { - const exportedSymbols: Exports = new Map(); - const importedSymbols: Imports = new Map(); + const analyzedFiles: Set = new Set(); + const exportedSymbols: Exports = new Map(); + const importedSymbols: Imports = new Map(); - const analyzeSourceFile = (filePath: string) => { + for (const principal of principals) { + const analyzeSourceFile = (filePath: string, _principal: ProjectPrincipal = principal) => { const workspace = chief.findWorkspaceByFilePath(filePath); if (workspace) { - const { imports, exports, scripts } = principal.analyzeSourceFile(filePath, { skipTypeOnly: isProduction }); + const { imports, exports, scripts } = _principal.analyzeSourceFile(filePath, { skipTypeOnly: isProduction }); const { internal, external, unresolved } = imports; const { exported, duplicate } = exports; if (exported.size > 0) exportedSymbols.set(filePath, exported); for (const [specifierFilePath, importItems] of internal.entries()) { - // Mark "external" imports from other local workspaces as used dependency const packageName = getPackageNameFromModuleSpecifier(importItems.specifier); - if (packageName && chief.localWorkspaces.has(packageName)) external.add(packageName); + if (packageName && chief.localWorkspaces.has(packageName)) { + // Mark "external" imports from other local workspaces as used dependency + external.add(packageName); + if (_principal === principal) { + const workspace = chief.findWorkspaceByFilePath(specifierFilePath); + if (workspace) { + const principal = factory.getPrincipalByPackageName(workspace.pkgName); + if (principal && !principal.isGitIgnored(specifierFilePath)) { + analyzeSourceFile(specifierFilePath, principal); + analyzedFiles.add(specifierFilePath); + } + } + } + } if (!importedSymbols.has(specifierFilePath)) { importedSymbols.set(specifierFilePath, importItems); @@ -297,7 +310,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { }); _getDependenciesFromScripts(scripts, { cwd: dirname(filePath) }).forEach(specifier => { - handleReferencedDependency({ specifier, containingFilePath: filePath, principal, workspace }); + handleReferencedDependency({ specifier, containingFilePath: filePath, principal: _principal, workspace }); }); } }; @@ -308,7 +321,6 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { streamer.cast('Connecting the dots...'); - const analyzedFiles: Set = new Set(); let size = principal.entryPaths.size; let round = 0; @@ -323,44 +335,44 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { analyzedFiles.add(filePath); }); } while (size !== principal.entryPaths.size); + } - const unusedFiles = principal.getUnreferencedFiles(); - - collector.addFilesIssues(unusedFiles); - - collector.addFileCounts({ processed: analyzedFiles.size, unused: unusedFiles.length }); + const isSymbolImported = (symbol: string, importingModule?: ImportedModule): boolean => { + if (!importingModule) return false; + if (importingModule.symbols.has(symbol)) return true; + const { isReExport, isReExportedBy } = importingModule; + const hasSymbol = (file: string) => isSymbolImported(symbol, importedSymbols.get(file)); + return isReExport ? Array.from(isReExportedBy).some(hasSymbol) : false; + }; - const isSymbolImported = (symbol: string, importingModule?: ImportedModule): boolean => { - if (!importingModule) return false; - if (importingModule.symbols.has(symbol)) return true; - const { isReExport, isReExportedBy } = importingModule; - const hasSymbol = (file: string) => isSymbolImported(symbol, importedSymbols.get(file)); - return isReExport ? Array.from(isReExportedBy).some(hasSymbol) : false; - }; + const isExportedInEntryFile = (principal: ProjectPrincipal, importedModule?: ImportedModule): boolean => { + if (!importedModule) return false; + const { isReExport, isReExportedBy } = importedModule; + const { entryPaths } = principal; + const hasFile = (file: string) => + entryPaths.has(file) || isExportedInEntryFile(principal, importedSymbols.get(file)); + return isReExport ? Array.from(isReExportedBy).some(hasFile) : false; + }; - const isExportedInEntryFile = (importedModule?: ImportedModule): boolean => { - if (!importedModule) return false; - const { isReExport, isReExportedBy } = importedModule; - const { entryPaths } = principal; - const hasFile = (file: string) => entryPaths.has(file) || isExportedInEntryFile(importedSymbols.get(file)); - return isReExport ? Array.from(isReExportedBy).some(hasFile) : false; - }; + const isExportedItemReferenced = (principal: ProjectPrincipal, exportedItem: ExportItem, filePath: string) => { + const hasReferences = principal.getHasReferences(filePath, exportedItem); + return ( + hasReferences.external || + (hasReferences.internal && + (typeof chief.config.ignoreExportsUsedInFile === 'object' + ? exportedItem.type !== 'unknown' && !!chief.config.ignoreExportsUsedInFile[exportedItem.type] + : chief.config.ignoreExportsUsedInFile)) + ); + }; - const isExportedItemReferenced = (exportedItem: ExportItem, filePath: string) => { - const hasReferences = principal.getHasReferences(filePath, exportedItem); - return ( - hasReferences.external || - (hasReferences.internal && - (typeof chief.config.ignoreExportsUsedInFile === 'object' - ? exportedItem.type !== 'unknown' && !!chief.config.ignoreExportsUsedInFile[exportedItem.type] - : chief.config.ignoreExportsUsedInFile)) - ); - }; + if (isReportValues || isReportTypes) { + streamer.cast('Analyzing source files...'); - if (isReportValues || isReportTypes) { - streamer.cast('Analyzing source files...'); + for (const [filePath, exportItems] of exportedSymbols.entries()) { + const workspace = chief.findWorkspaceByFilePath(filePath); + const principal = workspace && factory.getPrincipalByPackageName(workspace.pkgName); - for (const [filePath, exportItems] of exportedSymbols.entries()) { + if (principal) { // Bail out when in entry file (unless --include-entry-exports) if (!isIncludeEntryExports && principal.entryPaths.has(filePath)) continue; @@ -375,7 +387,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { if (importingModule && isSymbolImported(symbol, importingModule)) { // Skip members of classes/enums that are eventually exported by entry files - if (importingModule.isReExport && isExportedInEntryFile(importingModule)) continue; + if (importingModule.isReExport && isExportedInEntryFile(principal, importingModule)) continue; if (report.enumMembers && exportedItem.type === 'enum' && exportedItem.members) { if (isProduction) continue; @@ -395,9 +407,10 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { } const isStar = Boolean(importingModule?.isStar); - const isReExportedByEntryFile = !isIncludeEntryExports && isStar && isExportedInEntryFile(importingModule); + const isReExportedByEntryFile = + !isIncludeEntryExports && isStar && isExportedInEntryFile(principal, importingModule); - if (!isReExportedByEntryFile && !isExportedItemReferenced(exportedItem, filePath)) { + if (!isReExportedByEntryFile && !isExportedItemReferenced(principal, exportedItem, filePath)) { if (['enum', 'type', 'interface'].includes(exportedItem.type)) { if (isProduction) continue; const type = isStar ? 'nsTypes' : 'types'; @@ -412,6 +425,15 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { } } + const unusedFiles = factory + .getPrincipals() + .flatMap(principal => principal.getUnreferencedFiles()) + .filter(filePath => !analyzedFiles.has(filePath)); + + collector.addFilesIssues(unusedFiles); + + collector.addFileCounts({ processed: analyzedFiles.size, unused: unusedFiles.length }); + if (isReportDependencies) { const { dependencyIssues, devDependencyIssues, optionalPeerDependencyIssues } = deputy.settleDependencyIssues(); const { configurationHints } = deputy.getConfigurationHints(); diff --git a/tests/workspaces-cross-reference.test.ts b/tests/workspaces-cross-reference.test.ts new file mode 100644 index 000000000..3351f9d99 --- /dev/null +++ b/tests/workspaces-cross-reference.test.ts @@ -0,0 +1,25 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { main } from '../src/index.js'; +import { resolve } from '../src/util/path.js'; +import baseArguments from './helpers/baseArguments.js'; +import baseCounters from './helpers/baseCounters.js'; + +const cwd = resolve('fixtures/workspaces-cross-reference'); + +test('Resolve imports in separate workspaces without entry file', async () => { + const { issues, counters } = await main({ + ...baseArguments, + cwd, + }); + + assert(issues.exports['packages/lib-a/mod-a.ts']['unused']); + assert(issues.exports['packages/lib-b/mod-b.ts']['unused']); + + assert.deepEqual(counters, { + ...baseCounters, + exports: 2, + processed: 4, + total: 4, + }); +});