From d7aad62c441ca5750306b88e0d8305723ed0e475 Mon Sep 17 00:00:00 2001 From: Cole Cansler Date: Thu, 14 May 2026 13:44:53 -0700 Subject: [PATCH 1/2] fix export start reexports, add tests --- .changeset/export-star-rewrites.md | 5 + codemods/debarrel/scripts/codemod.ts | 29 +++- codemods/debarrel/scripts/utils/exportStar.ts | 130 ++++++++++++++++++ codemods/debarrel/scripts/utils/imports.ts | 4 +- codemods/debarrel/scripts/utils/paths.ts | 28 ++++ codemods/debarrel/scripts/utils/specifiers.ts | 101 +++++++++++++- .../expected/src/consumer.ts | 17 +++ .../expected/src/lib/appError.ts | 7 + .../expected/src/lib/index.ts | 2 + .../expected/src/lib/operations.ts | 14 ++ .../expected/src/metrics.json | 11 ++ .../expected/tsconfig.json | 9 ++ .../input/src/consumer.ts | 23 ++++ .../input/src/lib/appError.ts | 7 + .../input/src/lib/index.ts | 2 + .../input/src/lib/operations.ts | 14 ++ .../input/src/metrics.json | 11 ++ .../export-star-reexport/input/tsconfig.json | 9 ++ .../tests/export-star-reexport/metrics.json | 11 ++ 19 files changed, 425 insertions(+), 9 deletions(-) create mode 100644 .changeset/export-star-rewrites.md create mode 100644 codemods/debarrel/scripts/utils/exportStar.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/expected/src/consumer.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/expected/src/lib/appError.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/expected/src/lib/index.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/expected/src/lib/operations.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/expected/src/metrics.json create mode 100644 codemods/debarrel/tests/export-star-reexport/expected/tsconfig.json create mode 100644 codemods/debarrel/tests/export-star-reexport/input/src/consumer.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/input/src/lib/appError.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/input/src/lib/index.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/input/src/lib/operations.ts create mode 100644 codemods/debarrel/tests/export-star-reexport/input/src/metrics.json create mode 100644 codemods/debarrel/tests/export-star-reexport/input/tsconfig.json create mode 100644 codemods/debarrel/tests/export-star-reexport/metrics.json diff --git a/.changeset/export-star-rewrites.md b/.changeset/export-star-rewrites.md new file mode 100644 index 0000000..38bc96d --- /dev/null +++ b/.changeset/export-star-rewrites.md @@ -0,0 +1,5 @@ +--- +"debarrel": patch +--- + +Rewrite consumer imports that flow through bare `export * from "./x"` re-exports. Previously the semantic analyzer couldn't enumerate the wildcard's bindings, so the codemod silently left those imports pointing at the barrel; now the codemod manually walks the barrel's `export *` chain to find the declaring file. Also preserves the top-level `import type` modifier when every specifier in a type-only import is rewritten. diff --git a/codemods/debarrel/scripts/codemod.ts b/codemods/debarrel/scripts/codemod.ts index 16907bc..c63fd2d 100644 --- a/codemods/debarrel/scripts/codemod.ts +++ b/codemods/debarrel/scripts/codemod.ts @@ -41,6 +41,12 @@ const codemod: Codemod = async (root, options) => { .find((c) => c.is("import_clause")); if (!importClause) continue; + // Top-level `import type { … } from "…"` — preserved across rewrites so + // we don't downgrade a type-only import to a value import (which can + // break under --verbatimModuleSyntax / --isolatedModules when the + // resolved declarations are `export type` aliases). + const isTypeOnlyImport = /^\s*import\s+type\b/.test(importStmt.text()); + const rewrites: SpecRewrite[] = []; let totalSpecifiers = 0; @@ -59,7 +65,13 @@ const codemod: Codemod = async (root, options) => { if (!localBinding) continue; const def = localBinding.definition(); if (!def) continue; - const rw = resolveSpecifier(localBinding, importPath, def); + const rw = resolveSpecifier( + localBinding, + importPath, + def, + filename, + relativeFilename, + ); if (rw) rewrites.push(rw); } } @@ -75,7 +87,13 @@ const codemod: Codemod = async (root, options) => { totalSpecifiers += 1; const def = defaultIdent.definition(); if (def) { - const rw = resolveSpecifier(defaultIdent, importPath, def); + const rw = resolveSpecifier( + defaultIdent, + importPath, + def, + filename, + relativeFilename, + ); if (rw) rewrites.push(rw); } } @@ -100,7 +118,7 @@ const codemod: Codemod = async (root, options) => { // when the barrel import is the last/only import in the file. const lines: string[] = []; for (const [sourcePath, specs] of byPath) { - lines.push(buildImportText(sourcePath, specs, quoteChar)); + lines.push(buildImportText(sourcePath, specs, quoteChar, isTypeOnlyImport)); } edits.push(importStmt.replace(lines.join("\n"))); } else { @@ -123,12 +141,13 @@ const codemod: Codemod = async (root, options) => { } const lines: string[] = []; if (remainingSpecTexts.length > 0) { + const typeKeyword = isTypeOnlyImport ? "type " : ""; lines.push( - `import { ${remainingSpecTexts.join(", ")} } from ${quoteChar}${importPath}${quoteChar};`, + `import ${typeKeyword}{ ${remainingSpecTexts.join(", ")} } from ${quoteChar}${importPath}${quoteChar};`, ); } for (const [sourcePath, specs] of byPath) { - lines.push(buildImportText(sourcePath, specs, quoteChar)); + lines.push(buildImportText(sourcePath, specs, quoteChar, isTypeOnlyImport)); } edits.push(importStmt.replace(lines.join("\n"))); } diff --git a/codemods/debarrel/scripts/utils/exportStar.ts b/codemods/debarrel/scripts/utils/exportStar.ts new file mode 100644 index 0000000..199e701 --- /dev/null +++ b/codemods/debarrel/scripts/utils/exportStar.ts @@ -0,0 +1,130 @@ +import fs from "fs"; +import { isLocalRelativePath, resolveImportPath } from "./paths.ts"; + +// Note on parsing strategy: +// +// We deliberately use regex-driven parsing here rather than re-entering +// ast-grep. The semantic analyzer's `definition()` does not chase through +// bare `export * from "./y"` in this jssg runtime — for those specifiers +// `def.kind` is "import" and `def.root.filename()` is the importer itself +// — so to find the symbol's actual source we have to walk the barrel +// chain by reading the files off disk ourselves. Limiting that walk to +// regex matching keeps the helper self-contained and avoids needing a +// second ast-grep entry point per file. + +// Matches bare `export * from "./y"` and `export type * from "./y"`, but +// NOT `export * as Ns from "./y"` — namespace re-exports wrap their target +// in a single binding, which the semantic analyzer already resolves on its +// own through the namespace import. +const EXPORT_STAR_RE = + /^\s*export\s+(?:type\s+)?\*\s+from\s+["']([^"']+)["']/gm; + +function findExportStarSources(source: string): string[] { + const results = new Set(); + EXPORT_STAR_RE.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = EXPORT_STAR_RE.exec(source))) { + if (m[1]) results.add(m[1]); + } + return [...results]; +} + +function escapeRegex(s: string): string { + return s.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&"); +} + +/** + * Returns true if `source` exposes `name` as a top-level export — either + * a direct declaration (`export const NAME`, `export type NAME`, …) or + * inside an export clause (`export { …, NAME, … }`, possibly aliased, + * possibly re-exported from another module). + */ +function fileDeclaresName(source: string, name: string): boolean { + const escaped = escapeRegex(name); + + // Direct declarations. + const declRe = new RegExp( + `\\bexport\\s+(?:async\\s+)?(?:abstract\\s+)?(?:const|let|var|function|class|enum|interface|type)\\s+${escaped}\\b`, + ); + if (declRe.test(source)) return true; + + // `export default function NAME`/`export default class NAME` are rare but + // worth catching for symmetry; named default-aliased re-exports happen + // elsewhere. + const defaultDeclRe = new RegExp( + `\\bexport\\s+default\\s+(?:async\\s+)?(?:function|class)\\s+${escaped}\\b`, + ); + if (defaultDeclRe.test(source)) return true; + + // Export clauses: `export { … }` or `export type { … }`, with or without + // a trailing `from "..."`. We strip block comments + line comments before + // scanning the clause so commented-out names don't produce false positives. + const clauseRe = /\bexport\s+(?:type\s+)?\{([^}]*)\}/g; + let m: RegExpExecArray | null; + while ((m = clauseRe.exec(source))) { + const inside = (m[1] ?? "") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/[^\n]*/g, ""); + // Match either "NAME" or "X as NAME" between commas / clause boundaries. + const nameInClauseRe = new RegExp( + `(^|,)\\s*(?:type\\s+)?(?:[A-Za-z_$][\\w$]*\\s+as\\s+)?${escaped}\\s*(,|$)`, + ); + if (nameInClauseRe.test(inside)) return true; + } + return false; +} + +/** + * Walk `barrelFile`'s bare `export * from "./y"` chain to find which file + * declares `name`. Returns the absolute path of that file, or null if `name` + * isn't reachable through any wildcard re-export. + * + * Stops at non-local re-export targets (e.g. workspace packages, node_modules) + * since those would route the import through a different package boundary + * the codemod isn't authorized to rewrite. + */ +export function findSymbolViaExportStar( + barrelFile: string, + name: string, +): string | null { + return walk(barrelFile, name, new Set(), 0); +} + +function walk( + file: string, + name: string, + visited: Set, + depth: number, +): string | null { + if (depth > 10) return null; + if (visited.has(file)) return null; + visited.add(file); + + let source: string; + try { + source = fs.readFileSync(file, "utf8"); + } catch { + return null; + } + + for (const subPath of findExportStarSources(source)) { + if (!isLocalRelativePath(subPath)) continue; + const targetFile = resolveImportPath(file, subPath); + if (!targetFile) continue; + let targetSource: string; + try { + targetSource = fs.readFileSync(targetFile, "utf8"); + } catch { + continue; + } + if (fileDeclaresName(targetSource, name)) return targetFile; + + // The first re-export hop didn't declare the symbol directly — keep + // walking that file's own `export *` chain. We don't try to follow + // named `export { X } from "./y"` re-exports here; those are + // single-hop by design, mirroring the existing named-reexport branch. + const nested = walk(targetFile, name, visited, depth + 1); + if (nested) return nested; + } + return null; +} diff --git a/codemods/debarrel/scripts/utils/imports.ts b/codemods/debarrel/scripts/utils/imports.ts index 16879c8..1bdf5b6 100644 --- a/codemods/debarrel/scripts/utils/imports.ts +++ b/codemods/debarrel/scripts/utils/imports.ts @@ -7,6 +7,7 @@ export function buildImportText( sourcePath: string, specs: SpecRewrite[], quoteChar: string, + typeOnly = false, ): string { const parts: string[] = []; const defaultSpec = specs.find((s) => s.importType === "default"); @@ -23,7 +24,8 @@ export function buildImportText( ); parts.push(`{ ${specTexts.join(", ")} }`); } - return `import ${parts.join(", ")} from ${quoteChar}${sourcePath}${quoteChar};`; + const typeKeyword = typeOnly ? "type " : ""; + return `import ${typeKeyword}${parts.join(", ")} from ${quoteChar}${sourcePath}${quoteChar};`; } export function groupByPath( diff --git a/codemods/debarrel/scripts/utils/paths.ts b/codemods/debarrel/scripts/utils/paths.ts index df12297..18dba62 100644 --- a/codemods/debarrel/scripts/utils/paths.ts +++ b/codemods/debarrel/scripts/utils/paths.ts @@ -54,6 +54,34 @@ export function isBarrelFile(filename: string): boolean { return /^index\.(ts|tsx|js|jsx)$/.test(path.basename(filename)); } +const MODULE_EXTENSIONS = [".ts", ".tsx", ".js", ".jsx"] as const; + +/** + * Resolve a relative import path to the absolute file it points to on disk, + * mirroring Node/TS module resolution preferences: + * 1. `.` (file form — preferred when both exist) + * 2. `/index.` (directory's index form) + * + * Returns null when the import path isn't relative or no candidate exists. + */ +export function resolveImportPath( + importerFilename: string, + importPath: string, +): string | null { + if (!isLocalRelativePath(importPath)) return null; + const importerDir = path.dirname(importerFilename); + const resolved = path.resolve(importerDir, importPath); + for (const ext of MODULE_EXTENSIONS) { + const candidate = resolved + ext; + if (fileExists(candidate)) return candidate; + } + for (const ext of MODULE_EXTENSIONS) { + const candidate = path.join(resolved, `index${ext}`); + if (fileExists(candidate)) return candidate; + } + return null; +} + export function isInsideNodeModules(filename: string): boolean { return ( filename.includes("/node_modules/") || filename.includes("\\node_modules\\") diff --git a/codemods/debarrel/scripts/utils/specifiers.ts b/codemods/debarrel/scripts/utils/specifiers.ts index 2dd8da3..65ef057 100644 --- a/codemods/debarrel/scripts/utils/specifiers.ts +++ b/codemods/debarrel/scripts/utils/specifiers.ts @@ -1,4 +1,5 @@ import type { SgNode, SgRoot } from "codemod:ast-grep"; +import path from "path"; import type { Language } from "./language.ts"; import { getStringContent } from "./ast.ts"; import { @@ -7,8 +8,10 @@ import { isInsideNodeModules, isLocalRelativePath, joinImportPaths, + resolveImportPath, } from "./paths.ts"; import { parseBarrelExport } from "./barrel.ts"; +import { findSymbolViaExportStar } from "./exportStar.ts"; function getImportPackageName(importPath: string): string | null { if (importPath.startsWith("@")) { @@ -36,8 +39,23 @@ export function resolveSpecifier( localBinding: SgNode, importPath: string, def: { kind: string; root: SgRoot; node: SgNode }, + importerFilename: string, + importerRelativeFilename: string, ): SpecRewrite | null { - if (def.kind !== "external") return null; + // When the semantic analyzer fully resolves the binding to a different + // file we go through the `external` branches below. When it punts (most + // commonly because the symbol flows through a bare `export *` re-export + // that the analyzer can't enumerate statically), `def.kind` is "import" + // and `def.root.filename()` is the importer itself — skip the + // external-only checks and head straight to the manual export-star walker. + if (def.kind !== "external") { + return resolveViaExportStarWalk( + localBinding, + importPath, + importerFilename, + importerRelativeFilename, + ); + } // Never rewrite imports that resolve into node_modules — those are // third-party or published workspace packages with potentially restricted @@ -93,9 +111,86 @@ export function resolveSpecifier( } // Semantic analyzer resolved all the way through to the actual source file - // (not a barrel). Only rewrite if the import actually went through a barrel - // that we can bypass. If the resolved file is already the direct target of + // (not a barrel). If the resolved file is already the direct target of // the import (e.g. @acme/api/models/utils/ratelimiter → ratelimiter.ts), // the import is already correct — don't rewrite. return null; } + +/** + * Compute the workspace-relative path of a barrel `index.` from an + * importer's relative filename and the import specifier that points at it. + * Hand-rolled because the runtime exposes only a partial `path` polyfill + * (no `path.posix`). + */ +function barrelToRelativeFilename( + importerRelativeFilename: string, + importPath: string, + barrelExt: string, +): string { + const importerDir = importerRelativeFilename + .replace(/\\/g, "/") + .split("/") + .slice(0, -1); + const joined = importerDir.concat(importPath.replace(/\\/g, "/").split("/")); + const resolved: string[] = []; + for (const seg of joined) { + if (seg === "" || seg === ".") continue; + if (seg === ".." && resolved.length > 0 && resolved[resolved.length - 1] !== "..") { + resolved.pop(); + } else { + resolved.push(seg); + } + } + if (resolved.length > 0 && resolved[resolved.length - 1] === "index") { + resolved.pop(); + } + return resolved.length === 0 + ? `index${barrelExt}` + : `${resolved.join("/")}/index${barrelExt}`; +} + +/** + * Walk the barrel pointed to by `importPath` and look for which file in its + * `export * from "./y"` chain declares `localBinding`'s name. Used when the + * semantic analyzer can't tell us — bare `export *` re-exports don't carry + * named bindings the analyzer can chase. + */ +function resolveViaExportStarWalk( + localBinding: SgNode, + importPath: string, + importerFilename: string, + importerRelativeFilename: string, +): SpecRewrite | null { + if (!isLocalRelativePath(importPath)) return null; + const barrelFile = resolveImportPath(importerFilename, importPath); + if (!barrelFile || !isBarrelFile(barrelFile)) return null; + + const targetFile = findSymbolViaExportStar(barrelFile, localBinding.text()); + if (!targetFile) return null; + if (targetFile === barrelFile) return null; + + const barrelDir = path.dirname(barrelFile); + let rel = path.relative(barrelDir, targetFile); + const ext = path.extname(rel); + if (ext) rel = rel.slice(0, -ext.length); + rel = rel.replace(/\/index$/, "") || "."; + const fromBarrel = rel.startsWith(".") ? rel : `./${rel}`; + + // Mirror the barrel's workspace-relative path for the metric, so the + // `filePath` cardinality matches the named-reexport branches above. + const barrelExt = path.extname(barrelFile); + const barrelRelativeFilename = barrelToRelativeFilename( + importerRelativeFilename, + importPath, + barrelExt, + ); + + return { + consumerName: localBinding.text(), + newImportPath: joinImportPaths(importPath, fromBarrel), + localName: localBinding.text(), + importType: "named", + resolvedFilePath: barrelRelativeFilename, + }; +} diff --git a/codemods/debarrel/tests/export-star-reexport/expected/src/consumer.ts b/codemods/debarrel/tests/export-star-reexport/expected/src/consumer.ts new file mode 100644 index 0000000..faa88a8 --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/expected/src/consumer.ts @@ -0,0 +1,17 @@ +import { AppError, isAppError } from "./lib/appError"; +import { DEFAULT_CURRENCY } from "./lib/operations"; + +import type { AddressFragment, CurrencyFragment } from "./lib/operations"; + +export function describeAddress(addr: AddressFragment): string { + return `${addr.line1 ?? ""} (${addr.city ?? ""})`; +} + +export function defaultCurrency(): CurrencyFragment { + return DEFAULT_CURRENCY; +} + +export function wrapError(err: unknown): AppError { + if (isAppError(err)) return err; + return new AppError(String(err)); +} diff --git a/codemods/debarrel/tests/export-star-reexport/expected/src/lib/appError.ts b/codemods/debarrel/tests/export-star-reexport/expected/src/lib/appError.ts new file mode 100644 index 0000000..a2f1e13 --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/expected/src/lib/appError.ts @@ -0,0 +1,7 @@ +export class AppError extends Error { + name = "AppError"; +} + +export function isAppError(err: unknown): err is AppError { + return err instanceof AppError; +} diff --git a/codemods/debarrel/tests/export-star-reexport/expected/src/lib/index.ts b/codemods/debarrel/tests/export-star-reexport/expected/src/lib/index.ts new file mode 100644 index 0000000..0da0168 --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/expected/src/lib/index.ts @@ -0,0 +1,2 @@ +export { AppError, isAppError } from "./appError"; +export * from "./operations"; diff --git a/codemods/debarrel/tests/export-star-reexport/expected/src/lib/operations.ts b/codemods/debarrel/tests/export-star-reexport/expected/src/lib/operations.ts new file mode 100644 index 0000000..7b96cbc --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/expected/src/lib/operations.ts @@ -0,0 +1,14 @@ +export type AddressFragment = { + line1?: string; + city?: string; +}; + +export type CurrencyFragment = { + code: string; + symbol: string; +}; + +export const DEFAULT_CURRENCY: CurrencyFragment = { + code: "USD", + symbol: "$", +}; diff --git a/codemods/debarrel/tests/export-star-reexport/expected/src/metrics.json b/codemods/debarrel/tests/export-star-reexport/expected/src/metrics.json new file mode 100644 index 0000000..150970a --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/expected/src/metrics.json @@ -0,0 +1,11 @@ +{ + "barrel_import": [ + { + "cardinality": { + "filePath": "src/lib/index.ts", + "importer": "src/consumer.ts" + }, + "count": 2 + } + ] +} \ No newline at end of file diff --git a/codemods/debarrel/tests/export-star-reexport/expected/tsconfig.json b/codemods/debarrel/tests/export-star-reexport/expected/tsconfig.json new file mode 100644 index 0000000..459a96a --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/expected/tsconfig.json @@ -0,0 +1,9 @@ +{ + "compilerOptions": { + "target": "ES2020", + "module": "ESNext", + "moduleResolution": "bundler", + "baseUrl": "." + }, + "include": ["src"] +} diff --git a/codemods/debarrel/tests/export-star-reexport/input/src/consumer.ts b/codemods/debarrel/tests/export-star-reexport/input/src/consumer.ts new file mode 100644 index 0000000..7e74b89 --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/input/src/consumer.ts @@ -0,0 +1,23 @@ +import { + AppError, + isAppError, + DEFAULT_CURRENCY, +} from "./lib"; + +import type { + AddressFragment, + CurrencyFragment, +} from "./lib"; + +export function describeAddress(addr: AddressFragment): string { + return `${addr.line1 ?? ""} (${addr.city ?? ""})`; +} + +export function defaultCurrency(): CurrencyFragment { + return DEFAULT_CURRENCY; +} + +export function wrapError(err: unknown): AppError { + if (isAppError(err)) return err; + return new AppError(String(err)); +} diff --git a/codemods/debarrel/tests/export-star-reexport/input/src/lib/appError.ts b/codemods/debarrel/tests/export-star-reexport/input/src/lib/appError.ts new file mode 100644 index 0000000..a2f1e13 --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/input/src/lib/appError.ts @@ -0,0 +1,7 @@ +export class AppError extends Error { + name = "AppError"; +} + +export function isAppError(err: unknown): err is AppError { + return err instanceof AppError; +} diff --git a/codemods/debarrel/tests/export-star-reexport/input/src/lib/index.ts b/codemods/debarrel/tests/export-star-reexport/input/src/lib/index.ts new file mode 100644 index 0000000..0da0168 --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/input/src/lib/index.ts @@ -0,0 +1,2 @@ +export { AppError, isAppError } from "./appError"; +export * from "./operations"; diff --git a/codemods/debarrel/tests/export-star-reexport/input/src/lib/operations.ts b/codemods/debarrel/tests/export-star-reexport/input/src/lib/operations.ts new file mode 100644 index 0000000..7b96cbc --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/input/src/lib/operations.ts @@ -0,0 +1,14 @@ +export type AddressFragment = { + line1?: string; + city?: string; +}; + +export type CurrencyFragment = { + code: string; + symbol: string; +}; + +export const DEFAULT_CURRENCY: CurrencyFragment = { + code: "USD", + symbol: "$", +}; diff --git a/codemods/debarrel/tests/export-star-reexport/input/src/metrics.json b/codemods/debarrel/tests/export-star-reexport/input/src/metrics.json new file mode 100644 index 0000000..150970a --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/input/src/metrics.json @@ -0,0 +1,11 @@ +{ + "barrel_import": [ + { + "cardinality": { + "filePath": "src/lib/index.ts", + "importer": "src/consumer.ts" + }, + "count": 2 + } + ] +} \ No newline at end of file diff --git a/codemods/debarrel/tests/export-star-reexport/input/tsconfig.json b/codemods/debarrel/tests/export-star-reexport/input/tsconfig.json new file mode 100644 index 0000000..459a96a --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/input/tsconfig.json @@ -0,0 +1,9 @@ +{ + "compilerOptions": { + "target": "ES2020", + "module": "ESNext", + "moduleResolution": "bundler", + "baseUrl": "." + }, + "include": ["src"] +} diff --git a/codemods/debarrel/tests/export-star-reexport/metrics.json b/codemods/debarrel/tests/export-star-reexport/metrics.json new file mode 100644 index 0000000..150970a --- /dev/null +++ b/codemods/debarrel/tests/export-star-reexport/metrics.json @@ -0,0 +1,11 @@ +{ + "barrel_import": [ + { + "cardinality": { + "filePath": "src/lib/index.ts", + "importer": "src/consumer.ts" + }, + "count": 2 + } + ] +} \ No newline at end of file From 3469c06298cf36fc0544d707e5206f33e57d2218 Mon Sep 17 00:00:00 2001 From: Mohamad Mohebifar Date: Mon, 18 May 2026 10:10:27 -0700 Subject: [PATCH 2/2] refactor: improve type-only import detection and enhance export star resolution --- codemods/debarrel/scripts/codemod.ts | 2 +- codemods/debarrel/scripts/utils/exportStar.ts | 184 +++++++++++------- codemods/debarrel/scripts/utils/specifiers.ts | 62 +++--- 3 files changed, 136 insertions(+), 112 deletions(-) diff --git a/codemods/debarrel/scripts/codemod.ts b/codemods/debarrel/scripts/codemod.ts index c63fd2d..fad92de 100644 --- a/codemods/debarrel/scripts/codemod.ts +++ b/codemods/debarrel/scripts/codemod.ts @@ -45,7 +45,7 @@ const codemod: Codemod = async (root, options) => { // we don't downgrade a type-only import to a value import (which can // break under --verbatimModuleSyntax / --isolatedModules when the // resolved declarations are `export type` aliases). - const isTypeOnlyImport = /^\s*import\s+type\b/.test(importStmt.text()); + const isTypeOnlyImport = importStmt.children().some((c) => c.is("type")); const rewrites: SpecRewrite[] = []; let totalSpecifiers = 0; diff --git a/codemods/debarrel/scripts/utils/exportStar.ts b/codemods/debarrel/scripts/utils/exportStar.ts index 199e701..fb58edf 100644 --- a/codemods/debarrel/scripts/utils/exportStar.ts +++ b/codemods/debarrel/scripts/utils/exportStar.ts @@ -1,75 +1,123 @@ import fs from "fs"; +import path from "path"; +import { parse, type SgNode, type SgRoot } from "codemod:ast-grep"; +import type { Language } from "./language.ts"; +import { getStringContent } from "./ast.ts"; import { isLocalRelativePath, resolveImportPath } from "./paths.ts"; -// Note on parsing strategy: -// -// We deliberately use regex-driven parsing here rather than re-entering -// ast-grep. The semantic analyzer's `definition()` does not chase through -// bare `export * from "./y"` in this jssg runtime — for those specifiers -// `def.kind` is "import" and `def.root.filename()` is the importer itself -// — so to find the symbol's actual source we have to walk the barrel -// chain by reading the files off disk ourselves. Limiting that walk to -// regex matching keeps the helper self-contained and avoids needing a -// second ast-grep entry point per file. +// The semantic analyzer's `definition()` does not chase through bare +// `export * from "./y"` re-exports in this jssg runtime — for those +// specifiers `def.kind` is "import" and `def.root.filename()` is the +// importer itself. To find the symbol's actual source we walk the barrel +// chain ourselves: read each file off disk, parse it with ast-grep, and +// scan its top-level `export_statement` nodes. -// Matches bare `export * from "./y"` and `export type * from "./y"`, but -// NOT `export * as Ns from "./y"` — namespace re-exports wrap their target -// in a single binding, which the semantic analyzer already resolves on its -// own through the namespace import. -const EXPORT_STAR_RE = - /^\s*export\s+(?:type\s+)?\*\s+from\s+["']([^"']+)["']/gm; +function langForFile(filename: string): string { + const ext = path.extname(filename).toLowerCase(); + if (ext === ".tsx" || ext === ".jsx") return "tsx"; + if (ext === ".js" || ext === ".mjs" || ext === ".cjs") return "javascript"; + return "typescript"; +} + +function parseFile(filename: string): SgRoot | null { + let source: string; + try { + source = fs.readFileSync(filename, "utf8"); + } catch { + return null; + } + try { + return parse(langForFile(filename), source); + } catch { + return null; + } +} + +interface ExportStatementShape { + hasNamespaceExport: boolean; + hasExportClause: boolean; + exportClause: SgNode | undefined; + sourceNode: SgNode | undefined; + declaration: SgNode | undefined; +} + +function inspectExportStatement(stmt: SgNode): ExportStatementShape { + const children = stmt.children(); + return { + hasNamespaceExport: children.some((c) => c.is("namespace_export")), + hasExportClause: children.some((c) => c.is("export_clause")), + exportClause: children.find((c) => c.is("export_clause")), + sourceNode: children.find((c) => c.is("string")), + declaration: children.find( + (c) => + c.is("lexical_declaration") || + c.is("function_declaration") || + c.is("class_declaration") || + c.is("type_alias_declaration") || + c.is("interface_declaration") || + c.is("enum_declaration"), + ), + }; +} -function findExportStarSources(source: string): string[] { +// `export * from "./y"` (and `export type * from "./y"`), but NOT +// `export * as Ns from "./y"` — namespace re-exports wrap their target in a +// single binding the semantic analyzer already resolves on its own. +function findExportStarSources(root: SgRoot): string[] { const results = new Set(); - EXPORT_STAR_RE.lastIndex = 0; - let m: RegExpExecArray | null; - while ((m = EXPORT_STAR_RE.exec(source))) { - if (m[1]) results.add(m[1]); + for (const stmt of root.root().children()) { + if (!stmt.is("export_statement")) continue; + const shape = inspectExportStatement(stmt); + if (!shape.sourceNode) continue; + if (shape.hasNamespaceExport || shape.hasExportClause) continue; + const sourcePath = getStringContent(shape.sourceNode); + if (sourcePath) results.add(sourcePath); } return [...results]; } -function escapeRegex(s: string): string { - return s.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&"); +function declarationNameMatches(decl: SgNode, name: string): boolean { + if (decl.is("lexical_declaration")) { + for (const declarator of decl.findAll({ + rule: { kind: "variable_declarator" }, + })) { + const ident = declarator + .children() + .find((c) => c.is("identifier") || c.is("shorthand_property_identifier_pattern")); + if (ident && ident.text() === name) return true; + } + return false; + } + const ident = decl + .children() + .find((c) => c.is("identifier") || c.is("type_identifier")); + return ident !== undefined && ident.text() === name; } -/** - * Returns true if `source` exposes `name` as a top-level export — either - * a direct declaration (`export const NAME`, `export type NAME`, …) or - * inside an export clause (`export { …, NAME, … }`, possibly aliased, - * possibly re-exported from another module). - */ -function fileDeclaresName(source: string, name: string): boolean { - const escaped = escapeRegex(name); - - // Direct declarations. - const declRe = new RegExp( - `\\bexport\\s+(?:async\\s+)?(?:abstract\\s+)?(?:const|let|var|function|class|enum|interface|type)\\s+${escaped}\\b`, - ); - if (declRe.test(source)) return true; +// True when `root` exposes `name` as a top-level export — either as a direct +// declaration (`export const NAME`, `export type NAME`, …) or inside an +// export clause (`export { …, NAME, … }`, possibly aliased, possibly +// re-exported from another module). +function fileDeclaresName(root: SgRoot, name: string): boolean { + for (const stmt of root.root().children()) { + if (!stmt.is("export_statement")) continue; + const shape = inspectExportStatement(stmt); - // `export default function NAME`/`export default class NAME` are rare but - // worth catching for symmetry; named default-aliased re-exports happen - // elsewhere. - const defaultDeclRe = new RegExp( - `\\bexport\\s+default\\s+(?:async\\s+)?(?:function|class)\\s+${escaped}\\b`, - ); - if (defaultDeclRe.test(source)) return true; + if (shape.exportClause) { + for (const spec of shape.exportClause.findAll({ + rule: { kind: "export_specifier" }, + })) { + const idents = spec.findAll({ rule: { kind: "identifier" } }); + // `X` -> [X]; `X as Y` -> [X, Y]; exported name is the last identifier. + const exportedName = idents[idents.length - 1]?.text(); + if (exportedName === name) return true; + } + continue; + } - // Export clauses: `export { … }` or `export type { … }`, with or without - // a trailing `from "..."`. We strip block comments + line comments before - // scanning the clause so commented-out names don't produce false positives. - const clauseRe = /\bexport\s+(?:type\s+)?\{([^}]*)\}/g; - let m: RegExpExecArray | null; - while ((m = clauseRe.exec(source))) { - const inside = (m[1] ?? "") - .replace(/\/\*[\s\S]*?\*\//g, "") - .replace(/\/\/[^\n]*/g, ""); - // Match either "NAME" or "X as NAME" between commas / clause boundaries. - const nameInClauseRe = new RegExp( - `(^|,)\\s*(?:type\\s+)?(?:[A-Za-z_$][\\w$]*\\s+as\\s+)?${escaped}\\s*(,|$)`, - ); - if (nameInClauseRe.test(inside)) return true; + if (shape.declaration && declarationNameMatches(shape.declaration, name)) { + return true; + } } return false; } @@ -100,24 +148,16 @@ function walk( if (visited.has(file)) return null; visited.add(file); - let source: string; - try { - source = fs.readFileSync(file, "utf8"); - } catch { - return null; - } + const root = parseFile(file); + if (!root) return null; - for (const subPath of findExportStarSources(source)) { + for (const subPath of findExportStarSources(root)) { if (!isLocalRelativePath(subPath)) continue; const targetFile = resolveImportPath(file, subPath); if (!targetFile) continue; - let targetSource: string; - try { - targetSource = fs.readFileSync(targetFile, "utf8"); - } catch { - continue; - } - if (fileDeclaresName(targetSource, name)) return targetFile; + const targetRoot = parseFile(targetFile); + if (!targetRoot) continue; + if (fileDeclaresName(targetRoot, name)) return targetFile; // The first re-export hop didn't declare the symbol directly — keep // walking that file's own `export *` chain. We don't try to follow diff --git a/codemods/debarrel/scripts/utils/specifiers.ts b/codemods/debarrel/scripts/utils/specifiers.ts index 65ef057..5ea1959 100644 --- a/codemods/debarrel/scripts/utils/specifiers.ts +++ b/codemods/debarrel/scripts/utils/specifiers.ts @@ -117,39 +117,6 @@ export function resolveSpecifier( return null; } -/** - * Compute the workspace-relative path of a barrel `index.` from an - * importer's relative filename and the import specifier that points at it. - * Hand-rolled because the runtime exposes only a partial `path` polyfill - * (no `path.posix`). - */ -function barrelToRelativeFilename( - importerRelativeFilename: string, - importPath: string, - barrelExt: string, -): string { - const importerDir = importerRelativeFilename - .replace(/\\/g, "/") - .split("/") - .slice(0, -1); - const joined = importerDir.concat(importPath.replace(/\\/g, "/").split("/")); - const resolved: string[] = []; - for (const seg of joined) { - if (seg === "" || seg === ".") continue; - if (seg === ".." && resolved.length > 0 && resolved[resolved.length - 1] !== "..") { - resolved.pop(); - } else { - resolved.push(seg); - } - } - if (resolved.length > 0 && resolved[resolved.length - 1] === "index") { - resolved.pop(); - } - return resolved.length === 0 - ? `index${barrelExt}` - : `${resolved.join("/")}/index${barrelExt}`; -} - /** * Walk the barrel pointed to by `importPath` and look for which file in its * `export * from "./y"` chain declares `localBinding`'s name. Used when the @@ -167,8 +134,7 @@ function resolveViaExportStarWalk( if (!barrelFile || !isBarrelFile(barrelFile)) return null; const targetFile = findSymbolViaExportStar(barrelFile, localBinding.text()); - if (!targetFile) return null; - if (targetFile === barrelFile) return null; + if (!targetFile || targetFile === barrelFile) return null; const barrelDir = path.dirname(barrelFile); let rel = path.relative(barrelDir, targetFile); @@ -179,11 +145,10 @@ function resolveViaExportStarWalk( // Mirror the barrel's workspace-relative path for the metric, so the // `filePath` cardinality matches the named-reexport branches above. - const barrelExt = path.extname(barrelFile); - const barrelRelativeFilename = barrelToRelativeFilename( + const barrelRelativeFilename = toWorkspaceRelative( + importerFilename, importerRelativeFilename, - importPath, - barrelExt, + barrelFile, ); return { @@ -194,3 +159,22 @@ function resolveViaExportStarWalk( resolvedFilePath: barrelRelativeFilename, }; } + +/** + * Convert an absolute path inside the workspace back into a workspace-relative + * path, using the importer's own absolute+relative pair to derive the + * workspace root. Falls back to the absolute path if the root can't be + * inferred (importerFilename doesn't end with importerRelativeFilename). + */ +function toWorkspaceRelative( + importerFilename: string, + importerRelativeFilename: string, + absolutePath: string, +): string { + if (!importerFilename.endsWith(importerRelativeFilename)) return absolutePath; + const workspaceRoot = importerFilename.slice( + 0, + importerFilename.length - importerRelativeFilename.length, + ); + return path.relative(workspaceRoot, absolutePath).replace(/\\/g, "/"); +}