Skip to content

Commit

Permalink
Refactor getImportsAndExports return structure
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Dec 22, 2024
1 parent 39b4c29 commit 3cb27eb
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 49 deletions.
16 changes: 2 additions & 14 deletions packages/knip/src/ProjectPrincipal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,7 @@ export class ProjectPrincipal {

const resolve = (specifier: string) => this.backend.resolveModuleNames([specifier], sourceFile.fileName)[0];

const { imports, exports, scripts, traceRefs } = _getImportsAndExports(sourceFile, resolve, typeChecker, {
...options,
skipExports,
});
const { imports, ...rest } = _getImportsAndExports(sourceFile, resolve, typeChecker, { ...options, skipExports });

const { internal, resolved, specifiers, unresolved, external } = imports;

Expand Down Expand Up @@ -293,16 +290,7 @@ export class ProjectPrincipal {
}
}

return {
imports: {
internal,
unresolved: unresolvedImports,
external,
},
exports,
scripts,
traceRefs,
};
return { imports: { internal, unresolved: unresolvedImports, external }, ...rest };
}

invalidateFile(filePath: string) {
Expand Down
11 changes: 6 additions & 5 deletions packages/knip/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {

const workspace = chief.findWorkspaceByFilePath(filePath);
if (workspace) {
const { imports, exports, scripts, traceRefs } = principal.analyzeSourceFile(
const { imports, exports, duplicates, scripts, traceRefs } = principal.analyzeSourceFile(
filePath,
{
skipTypeOnly: isStrict,
Expand All @@ -326,6 +326,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {

file.imports = imports;
file.exports = exports;
file.duplicates = duplicates;
file.scripts = scripts;
file.traceRefs = traceRefs;

Expand Down Expand Up @@ -402,7 +403,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
streamer.cast('Connecting the dots...');

for (const [filePath, file] of graph.entries()) {
const exportItems = file.exports?.exported;
const exportItems = file.exports;

if (!exportItems || exportItems.size === 0) continue;

Expand Down Expand Up @@ -454,7 +455,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
continue;
}
// Skip exports if re-exported from entry file and tagged
const reExportedItem = graph.get(reExportingEntryFile)?.exports.exported.get(identifier);
const reExportedItem = graph.get(reExportingEntryFile)?.exports.get(identifier);
if (reExportedItem && shouldIgnore(reExportedItem.jsDocTags)) continue;
}

Expand Down Expand Up @@ -571,8 +572,8 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
const ws = chief.findWorkspaceByFilePath(filePath);

if (ws) {
if (file.exports?.duplicate) {
for (const symbols of file.exports.duplicate) {
if (file.duplicates) {
for (const symbols of file.duplicates) {
if (symbols.length > 1) {
const symbol = symbols.map(s => s.symbol).join('|');
collector.addIssue({ type: 'duplicates', filePath, workspace: ws.name, symbol, symbols });
Expand Down
6 changes: 2 additions & 4 deletions packages/knip/src/types/dependency-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ export type FileNode = {
external: Set<string>;
unresolved: Set<UnresolvedImport>;
};
exports: {
exported: ExportMap;
duplicate: Iterable<Array<IssueSymbol>>;
};
exports: ExportMap;
duplicates: Iterable<Array<IssueSymbol>>;
scripts: Set<string>;
imported?: ImportDetails;
internalImportCache?: ImportMap;
Expand Down
36 changes: 14 additions & 22 deletions packages/knip/src/typescript/get-imports-and-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ const getImportsAndExports = (
options: GetImportsAndExportsOptions
) => {
const { skipTypeOnly, tags, ignoreExportsUsedInFile } = options;
const internalImports: ImportMap = new Map();
const externalImports = new Set<string>();
const unresolvedImports = new Set<UnresolvedImport>();
const internal: ImportMap = new Map();
const external = new Set<string>();
const unresolved = new Set<UnresolvedImport>();
const resolved = new Set<string>();
const specifiers = new Set<[string, string]>();
const exports: ExportMap = new Map();
Expand Down Expand Up @@ -122,11 +122,11 @@ const getImportsAndExports = (

specifiers.add([specifier, filePath]);

const file = internalImports.get(filePath);
const file = internal.get(filePath);

const imports = file ?? createImports();

if (!file) internalImports.set(filePath, imports);
if (!file) internal.set(filePath, imports);

const nsOrAlias = symbol ? String(symbol.escapedName) : alias;

Expand Down Expand Up @@ -191,7 +191,7 @@ const getImportsAndExports = (

// Module resolver may return DTS references or unaliased npm package names,
// but in the rest of the program we want the package name based on the original specifier.
externalImports.add(sanitizedSpecifier);
external.add(sanitizedSpecifier);
}
}
} else {
Expand All @@ -201,9 +201,9 @@ const getImportsAndExports = (

if (typeof pos === 'number') {
const { line, character } = sourceFile.getLineAndCharacterOfPosition(pos);
unresolvedImports.add({ specifier, pos, line: line + 1, col: character + 1 });
unresolved.add({ specifier, pos, line: line + 1, col: character + 1 });
} else {
unresolvedImports.add({ specifier });
unresolved.add({ specifier });
}
}
};
Expand All @@ -215,7 +215,7 @@ const getImportsAndExports = (
const importedSymbolFilePath = importedInternalSymbols.get(symbol);
if (importedSymbolFilePath) {
const importId = String(symbol.escapedName);
const internalImport = internalImports.get(importedSymbolFilePath);
const internalImport = internal.get(importedSymbolFilePath);
if (internalImport) {
if (importId !== identifier) {
// Pattern: import { id as alias } from 'specifier'; export = id; export default id;
Expand Down Expand Up @@ -316,7 +316,7 @@ const getImportsAndExports = (
if (symbol) {
if (filePath) {
if (!isImportSpecifier(node)) {
const imports = internalImports.get(filePath);
const imports = internal.get(filePath);
if (imports) {
traceRefs.add(id);
if (isAccessExpression(node.parent)) {
Expand Down Expand Up @@ -378,7 +378,7 @@ const getImportsAndExports = (
const namespace = left.text;
const { filePath } = getImport(namespace, node);
if (filePath) {
const internalImport = internalImports.get(filePath);
const internalImport = internal.get(filePath);
if (internalImport) addNsMemberRefs(internalImport, namespace, right.text);
}
}
Expand Down Expand Up @@ -421,17 +421,9 @@ const getImportsAndExports = (
}

return {
imports: {
internal: internalImports,
external: externalImports,
resolved,
specifiers,
unresolved: unresolvedImports,
},
exports: {
exported: exports,
duplicate: [...aliasedExports.values()],
},
imports: { internal, external, resolved, specifiers, unresolved },
exports,
duplicates: [...aliasedExports.values()],
scripts,
traceRefs,
};
Expand Down
6 changes: 2 additions & 4 deletions packages/knip/src/util/dependency-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ const createFileNode = (): FileNode => ({
external: new Set(),
unresolved: new Set(),
},
exports: {
exported: new Map(),
duplicate: new Set(),
},
exports: new Map(),
duplicates: new Set(),
scripts: new Set(),
traceRefs: new Set(),
});
Expand Down

0 comments on commit 3cb27eb

Please sign in to comment.