From ac7032eb68ac049da91580aa1ca5da1717245ae6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 15:58:30 +0000 Subject: [PATCH 1/6] Initial plan From 5fbfd811cedc0dc1430326b84ce02284c2dbb1d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 16:03:57 +0000 Subject: [PATCH 2/6] Refactor scope analysis to periscopic --- package-lock.json | 30 +++++- package.json | 3 +- src/format.ts | 262 ++++++++++------------------------------------ test/updateSrc.ts | 19 ++++ 4 files changed, 107 insertions(+), 207 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9feb481..68b4c08 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,8 @@ "license": "MIT", "dependencies": { "magic-string": "^0.30.21", - "oxc-parser": "^0.132.0" + "oxc-parser": "^0.132.0", + "periscopic": "^4.0.3" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -2915,7 +2916,6 @@ "version": "1.0.9", "resolved": "https://registry.npmjs.org/@types/estree/-/estree-1.0.9.tgz", "integrity": "sha512-GhdPgy1el4/ImP05X05Uw4cw2/M93BCUmnEvWZNStlCzEKME4Fkk+YpoA5OiHNQmoS7Cafb8Xa3Pya8m1Qrzeg==", - "dev": true, "license": "MIT" }, "node_modules/@types/istanbul-lib-coverage": { @@ -4342,6 +4342,15 @@ "node": ">=0.10.0" } }, + "node_modules/is-reference": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/is-reference/-/is-reference-3.0.3.tgz", + "integrity": "sha512-ixkJoqQvAP88E6wLydLGGqCJsrFUnqoH6HnaczB8XmDH1oaWU+xxdptvikTgaEhtZ53Ky6YXiBuUI2WXLMCwjw==", + "license": "MIT", + "dependencies": { + "@types/estree": "^1.0.6" + } + }, "node_modules/isexe": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz", @@ -4898,6 +4907,17 @@ "node": "20 || >=22" } }, + "node_modules/periscopic": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/periscopic/-/periscopic-4.0.3.tgz", + "integrity": "sha512-iD/CnjEI6TJYwqLXmEVByEbT1RwUGSW3W51t/uWf+2Ag7FMHLqJ1XNhawGaXBf9/gZr+DhF3bTGLOw+03GJnzg==", + "license": "MIT", + "dependencies": { + "@types/estree": "*", + "is-reference": "^3.0.2", + "zimmerframe": "^1.0.0" + } + }, "node_modules/picocolors": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/picocolors/-/picocolors-1.1.1.tgz", @@ -5699,6 +5719,12 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } + }, + "node_modules/zimmerframe": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/zimmerframe/-/zimmerframe-1.1.4.tgz", + "integrity": "sha512-B58NGBEoc8Y9MWWCQGl/gq9xBCe4IiKM0a2x7GZdQKOW5Exr8S1W24J6OgM1njK8xCRGvAJIL/MxXHf6SkmQKQ==", + "license": "MIT" } } } diff --git a/package.json b/package.json index cee1294..4301343 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,8 @@ }, "dependencies": { "magic-string": "^0.30.21", - "oxc-parser": "^0.132.0" + "oxc-parser": "^0.132.0", + "periscopic": "^4.0.3" }, "devDependencies": { "@eslint/js": "^10.0.1", diff --git a/src/format.ts b/src/format.ts index cce2225..27616a4 100644 --- a/src/format.ts +++ b/src/format.ts @@ -1,4 +1,5 @@ import MagicString from 'magic-string' +import { analyze, extract_names, Scope as PeriscopicScope } from 'periscopic' import type { ParseResult, Node, @@ -14,11 +15,7 @@ import { Visitor } from 'oxc-parser' import type { Callback } from './types.js' type BindingKind = 'other' | 'createRequireFactory' | 'requireAlias' -type Scope = { - parent: Scope | null - functionScope: boolean - bindings: Map -} +type PeriscopicNode = Parameters[0] type UnknownRecord = Record const isStringLiteral = (node: Node): node is StringLiteral => { @@ -53,129 +50,56 @@ const isRequireCallForModule = (node: CallExpression) => { return isStringLiteral(first) && isModuleCreateRequireSource(first.value) } -const collectBindingNames = (target: unknown): string[] => { - if (!isRecord(target)) { - return [] - } - - const nodeType = target.type - - if (isIdentifier(target)) { - return [target.name] - } - - const record = target - - if (nodeType === 'AssignmentPattern') { - return collectBindingNames(record.left) +const asPeriscopicNode = (node: Node | unknown) => node as PeriscopicNode +const getOrCreateKinds = ( + kindsByScope: WeakMap>, + scope: PeriscopicScope, +) => { + let kinds = kindsByScope.get(scope) + + if (!kinds) { + kinds = new Map() + kindsByScope.set(scope, kinds) } - if (nodeType === 'RestElement') { - return collectBindingNames(record.argument) - } - - if (nodeType === 'ArrayPattern') { - const elements = Array.isArray(record.elements) ? record.elements : [] - return elements.flatMap(element => collectBindingNames(element)) - } - - if (nodeType === 'ObjectPattern') { - const properties = (Array.isArray(record.properties) ? record.properties : []).filter( - isRecord, - ) - - return properties.flatMap(property => { - const propertyType = property.type - - const propertyRecord = property - - if (propertyType === 'Property') { - return collectBindingNames(propertyRecord.value) - } - - if (propertyType === 'RestElement') { - return collectBindingNames(propertyRecord.argument) - } - - return [] - }) - } - - return [] + return kinds } const format = async (src: string, ast: ParseResult, cb: Callback) => { const code = new MagicString(src) - let scope: Scope = { - parent: null, - functionScope: true, - bindings: new Map(), - } - const variableDeclarationKinds: string[] = [] - const pushScope = (options?: { functionScope?: boolean }) => { - scope = { - parent: scope, - functionScope: options?.functionScope ?? false, - bindings: new Map(), + const { map: scopeMap, scope: rootScope } = analyze(asPeriscopicNode(ast.program)) + const bindingKinds = new WeakMap>() + let scope = rootScope + const enterScope = (node: Node) => { + const nextScope = scopeMap.get(asPeriscopicNode(node)) + + if (nextScope) { + scope = nextScope } } - const popScope = () => { - if (scope.parent) { + const exitScope = (node: Node) => { + if (scopeMap.has(asPeriscopicNode(node)) && scope.parent) { scope = scope.parent } } - const getNearestFunctionScope = () => { - let cursor: Scope | null = scope - - while (cursor) { - if (cursor.functionScope) { - return cursor - } - - cursor = cursor.parent - } - - return scope - } - const defineBinding = ( - name: string, - kind: BindingKind = 'other', - options?: { varScoped?: boolean }, - ) => { - const targetScope = options?.varScoped ? getNearestFunctionScope() : scope - targetScope.bindings.set(name, kind) - } - const getBindingScope = (name: string) => { - let cursor: Scope | null = scope + const setBindingKind = (name: string, kind: BindingKind) => { + const owner = scope.find_owner(name) - while (cursor) { - if (cursor.bindings.has(name)) { - return cursor - } - - cursor = cursor.parent + if (!owner) { + return false } - return null + getOrCreateKinds(bindingKinds, owner).set(name, kind) + return true } const getBindingKind = (name: string) => { - const bindingScope = getBindingScope(name) + const owner = scope.find_owner(name) - if (!bindingScope) { + if (!owner) { return null } - return bindingScope.bindings.get(name) ?? null - } - const updateBinding = (name: string, kind: BindingKind) => { - const bindingScope = getBindingScope(name) - - if (!bindingScope) { - return false - } - - bindingScope.bindings.set(name, kind) - return true + return bindingKinds.get(owner)?.get(name) ?? 'other' } const isCreateRequireCall = (node: CallExpression) => { if (!isIdentifier(node.callee)) { @@ -210,13 +134,10 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { continue } - defineBinding(specifier.local.name, 'createRequireFactory') + setBindingKind(specifier.local.name, 'createRequireFactory') } } - const markRequireDestructureCreateRequire = ( - node: VariableDeclarator, - options?: { varScoped?: boolean }, - ) => { + const markRequireDestructureCreateRequire = (node: VariableDeclarator) => { if (node.id.type !== 'ObjectPattern' || node.init?.type !== 'CallExpression') { return } @@ -236,8 +157,8 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { continue } - for (const name of collectBindingNames(property.value)) { - defineBinding(name, 'createRequireFactory', options) + for (const name of extract_names(asPeriscopicNode(property.value))) { + setBindingKind(name, 'createRequireFactory') } } } @@ -329,52 +250,12 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { } const visitor = new Visitor({ - Program() { - pushScope({ functionScope: true }) - }, - 'Program:exit'() { - popScope() - }, - FunctionDeclaration(node) { - if (node.id && node.id.type === 'Identifier') { - defineBinding(node.id.name) - } - - pushScope({ functionScope: true }) - - for (const parameter of node.params) { - for (const name of collectBindingNames(parameter)) { - defineBinding(name) - } - } - }, - 'FunctionDeclaration:exit'() { - popScope() - }, - FunctionExpression(node) { - pushScope({ functionScope: true }) - - if (node.id && node.id.type === 'Identifier') { - defineBinding(node.id.name) - } - - for (const parameter of node.params) { - for (const name of collectBindingNames(parameter)) { - defineBinding(name) - } - } - }, - 'FunctionExpression:exit'() { - popScope() - }, + FunctionDeclaration: enterScope, + 'FunctionDeclaration:exit': exitScope, + FunctionExpression: enterScope, + 'FunctionExpression:exit': exitScope, ArrowFunctionExpression(node) { - pushScope({ functionScope: true }) - - for (const parameter of node.params) { - for (const name of collectBindingNames(parameter)) { - defineBinding(name) - } - } + enterScope(node) const { body } = node @@ -382,34 +263,20 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { formatExpression(body) } }, - 'ArrowFunctionExpression:exit'() { - popScope() - }, - BlockStatement() { - pushScope() - }, - 'BlockStatement:exit'() { - popScope() - }, - CatchClause(node) { - pushScope() - - if (node.param) { - for (const name of collectBindingNames(node.param)) { - defineBinding(name) - } - } - }, - 'CatchClause:exit'() { - popScope() - }, + 'ArrowFunctionExpression:exit': exitScope, + BlockStatement: enterScope, + 'BlockStatement:exit': exitScope, + CatchClause: enterScope, + 'CatchClause:exit': exitScope, + ForStatement: enterScope, + 'ForStatement:exit': exitScope, + ForInStatement: enterScope, + 'ForInStatement:exit': exitScope, + ForOfStatement: enterScope, + 'ForOfStatement:exit': exitScope, + SwitchStatement: enterScope, + 'SwitchStatement:exit': exitScope, ImportDeclaration(node) { - for (const specifier of node.specifiers) { - if ('local' in specifier && specifier.local.type === 'Identifier') { - defineBinding(specifier.local.name) - } - } - markModuleImportCreateRequire(node) const { source } = node @@ -427,28 +294,15 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { code.update(start + 1, end - 1, updated) } }, - VariableDeclaration(node) { - variableDeclarationKinds.push(node.kind) - }, - 'VariableDeclaration:exit'() { - variableDeclarationKinds.pop() - }, VariableDeclarator(node) { - const varScoped = variableDeclarationKinds.at(-1) === 'var' - const names = collectBindingNames(node.id) - - for (const name of names) { - defineBinding(name, 'other', { varScoped }) - } - - markRequireDestructureCreateRequire(node, { varScoped }) + markRequireDestructureCreateRequire(node) if (node.id.type !== 'Identifier' || node.init?.type !== 'CallExpression') { return } if (isCreateRequireCall(node.init)) { - defineBinding(node.id.name, 'requireAlias', { varScoped }) + setBindingKind(node.id.name, 'requireAlias') } }, AssignmentExpression(node) { @@ -459,12 +313,12 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { const { name } = node.left if (node.right.type === 'CallExpression' && isCreateRequireCall(node.right)) { - updateBinding(name, 'requireAlias') + setBindingKind(name, 'requireAlias') return } if (getBindingKind(name) === 'requireAlias') { - updateBinding(name, 'other') + setBindingKind(name, 'other') } }, ExpressionStatement(node) { diff --git a/test/updateSrc.ts b/test/updateSrc.ts index aa08dfe..01ee63b 100644 --- a/test/updateSrc.ts +++ b/test/updateSrc.ts @@ -329,4 +329,23 @@ describe('updateSrc', () => { assert.ok(update.indexOf("loader('./outside.mjs')") > -1) }) + + it('does not rewrite require shadowed by for-of scope', async () => { + const update = await specifier.updateSrc( + ` + for (const require of [value => value]) { + require('./skip.js') + } + + require('./rewrite.js') + `, + 'js', + spec => { + return spec.value.replace('.js', '.mjs') + }, + ) + + assert.ok(update.indexOf("require('./skip.js')") > -1) + assert.ok(update.indexOf("require('./rewrite.mjs')") > -1) + }) }) From 19ad3059f175c4063e6e4fbb5a70b6e6e5a77d9c Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 31 May 2026 16:00:20 -0500 Subject: [PATCH 3/6] refactor: remove periscopic while keeping parity. --- package-lock.json | 30 +---- package.json | 3 +- src/format.ts | 286 ++++++++++++++++++++++++++++++++++++---------- test/updateSrc.ts | 79 +++++++++++++ 4 files changed, 310 insertions(+), 88 deletions(-) diff --git a/package-lock.json b/package-lock.json index 68b4c08..9feb481 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,8 +10,7 @@ "license": "MIT", "dependencies": { "magic-string": "^0.30.21", - "oxc-parser": "^0.132.0", - "periscopic": "^4.0.3" + "oxc-parser": "^0.132.0" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -2916,6 +2915,7 @@ "version": "1.0.9", "resolved": "https://registry.npmjs.org/@types/estree/-/estree-1.0.9.tgz", "integrity": "sha512-GhdPgy1el4/ImP05X05Uw4cw2/M93BCUmnEvWZNStlCzEKME4Fkk+YpoA5OiHNQmoS7Cafb8Xa3Pya8m1Qrzeg==", + "dev": true, "license": "MIT" }, "node_modules/@types/istanbul-lib-coverage": { @@ -4342,15 +4342,6 @@ "node": ">=0.10.0" } }, - "node_modules/is-reference": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/is-reference/-/is-reference-3.0.3.tgz", - "integrity": "sha512-ixkJoqQvAP88E6wLydLGGqCJsrFUnqoH6HnaczB8XmDH1oaWU+xxdptvikTgaEhtZ53Ky6YXiBuUI2WXLMCwjw==", - "license": "MIT", - "dependencies": { - "@types/estree": "^1.0.6" - } - }, "node_modules/isexe": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz", @@ -4907,17 +4898,6 @@ "node": "20 || >=22" } }, - "node_modules/periscopic": { - "version": "4.0.3", - "resolved": "https://registry.npmjs.org/periscopic/-/periscopic-4.0.3.tgz", - "integrity": "sha512-iD/CnjEI6TJYwqLXmEVByEbT1RwUGSW3W51t/uWf+2Ag7FMHLqJ1XNhawGaXBf9/gZr+DhF3bTGLOw+03GJnzg==", - "license": "MIT", - "dependencies": { - "@types/estree": "*", - "is-reference": "^3.0.2", - "zimmerframe": "^1.0.0" - } - }, "node_modules/picocolors": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/picocolors/-/picocolors-1.1.1.tgz", @@ -5719,12 +5699,6 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } - }, - "node_modules/zimmerframe": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/zimmerframe/-/zimmerframe-1.1.4.tgz", - "integrity": "sha512-B58NGBEoc8Y9MWWCQGl/gq9xBCe4IiKM0a2x7GZdQKOW5Exr8S1W24J6OgM1njK8xCRGvAJIL/MxXHf6SkmQKQ==", - "license": "MIT" } } } diff --git a/package.json b/package.json index 4301343..cee1294 100644 --- a/package.json +++ b/package.json @@ -57,8 +57,7 @@ }, "dependencies": { "magic-string": "^0.30.21", - "oxc-parser": "^0.132.0", - "periscopic": "^4.0.3" + "oxc-parser": "^0.132.0" }, "devDependencies": { "@eslint/js": "^10.0.1", diff --git a/src/format.ts b/src/format.ts index 27616a4..dd2b4ee 100644 --- a/src/format.ts +++ b/src/format.ts @@ -1,5 +1,4 @@ import MagicString from 'magic-string' -import { analyze, extract_names, Scope as PeriscopicScope } from 'periscopic' import type { ParseResult, Node, @@ -15,7 +14,11 @@ import { Visitor } from 'oxc-parser' import type { Callback } from './types.js' type BindingKind = 'other' | 'createRequireFactory' | 'requireAlias' -type PeriscopicNode = Parameters[0] +type Scope = { + parent: Scope | null + functionScope: boolean + bindings: Map +} type UnknownRecord = Record const isStringLiteral = (node: Node): node is StringLiteral => { @@ -50,56 +53,129 @@ const isRequireCallForModule = (node: CallExpression) => { return isStringLiteral(first) && isModuleCreateRequireSource(first.value) } -const asPeriscopicNode = (node: Node | unknown) => node as PeriscopicNode -const getOrCreateKinds = ( - kindsByScope: WeakMap>, - scope: PeriscopicScope, -) => { - let kinds = kindsByScope.get(scope) - - if (!kinds) { - kinds = new Map() - kindsByScope.set(scope, kinds) +const collectBindingNames = (target: unknown): string[] => { + if (!isRecord(target)) { + return [] + } + + const nodeType = target.type + + if (isIdentifier(target)) { + return [target.name] + } + + const record = target + + if (nodeType === 'AssignmentPattern') { + return collectBindingNames(record.left) + } + + if (nodeType === 'RestElement') { + return collectBindingNames(record.argument) + } + + if (nodeType === 'ArrayPattern') { + const elements = Array.isArray(record.elements) ? record.elements : [] + return elements.flatMap(element => collectBindingNames(element)) } - return kinds + if (nodeType === 'ObjectPattern') { + const properties = (Array.isArray(record.properties) ? record.properties : []).filter( + isRecord, + ) + + return properties.flatMap(property => { + const propertyType = property.type + + const propertyRecord = property + + if (propertyType === 'Property') { + return collectBindingNames(propertyRecord.value) + } + + if (propertyType === 'RestElement') { + return collectBindingNames(propertyRecord.argument) + } + + return [] + }) + } + + return [] } const format = async (src: string, ast: ParseResult, cb: Callback) => { const code = new MagicString(src) - const { map: scopeMap, scope: rootScope } = analyze(asPeriscopicNode(ast.program)) - const bindingKinds = new WeakMap>() - let scope = rootScope - const enterScope = (node: Node) => { - const nextScope = scopeMap.get(asPeriscopicNode(node)) - - if (nextScope) { - scope = nextScope + let scope: Scope = { + parent: null, + functionScope: true, + bindings: new Map(), + } + const variableDeclarationKinds: string[] = [] + const pushScope = (options?: { functionScope?: boolean }) => { + scope = { + parent: scope, + functionScope: options?.functionScope ?? false, + bindings: new Map(), } } - const exitScope = (node: Node) => { - if (scopeMap.has(asPeriscopicNode(node)) && scope.parent) { + const popScope = () => { + if (scope.parent) { scope = scope.parent } } - const setBindingKind = (name: string, kind: BindingKind) => { - const owner = scope.find_owner(name) + const getNearestFunctionScope = () => { + let cursor: Scope | null = scope - if (!owner) { - return false + while (cursor) { + if (cursor.functionScope) { + return cursor + } + + cursor = cursor.parent } - getOrCreateKinds(bindingKinds, owner).set(name, kind) - return true + return scope + } + const defineBinding = ( + name: string, + kind: BindingKind = 'other', + options?: { varScoped?: boolean }, + ) => { + const targetScope = options?.varScoped ? getNearestFunctionScope() : scope + targetScope.bindings.set(name, kind) + } + const getBindingScope = (name: string) => { + let cursor: Scope | null = scope + + while (cursor) { + if (cursor.bindings.has(name)) { + return cursor + } + + cursor = cursor.parent + } + + return null } const getBindingKind = (name: string) => { - const owner = scope.find_owner(name) + const bindingScope = getBindingScope(name) - if (!owner) { + if (!bindingScope) { return null } - return bindingKinds.get(owner)?.get(name) ?? 'other' + return bindingScope.bindings.get(name) ?? null + } + const updateBinding = (name: string, kind: BindingKind) => { + const bindingScope = getBindingScope(name) + + if (!bindingScope) { + return false + } + + bindingScope.bindings.set(name, kind) + return true } const isCreateRequireCall = (node: CallExpression) => { if (!isIdentifier(node.callee)) { @@ -134,10 +210,13 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { continue } - setBindingKind(specifier.local.name, 'createRequireFactory') + defineBinding(specifier.local.name, 'createRequireFactory') } } - const markRequireDestructureCreateRequire = (node: VariableDeclarator) => { + const markRequireDestructureCreateRequire = ( + node: VariableDeclarator, + options?: { varScoped?: boolean }, + ) => { if (node.id.type !== 'ObjectPattern' || node.init?.type !== 'CallExpression') { return } @@ -157,8 +236,8 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { continue } - for (const name of extract_names(asPeriscopicNode(property.value))) { - setBindingKind(name, 'createRequireFactory') + for (const name of collectBindingNames(property.value)) { + defineBinding(name, 'createRequireFactory', options) } } } @@ -250,12 +329,52 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { } const visitor = new Visitor({ - FunctionDeclaration: enterScope, - 'FunctionDeclaration:exit': exitScope, - FunctionExpression: enterScope, - 'FunctionExpression:exit': exitScope, + Program() { + pushScope({ functionScope: true }) + }, + 'Program:exit'() { + popScope() + }, + FunctionDeclaration(node) { + if (node.id && node.id.type === 'Identifier') { + defineBinding(node.id.name) + } + + pushScope({ functionScope: true }) + + for (const parameter of node.params) { + for (const name of collectBindingNames(parameter)) { + defineBinding(name) + } + } + }, + 'FunctionDeclaration:exit'() { + popScope() + }, + FunctionExpression(node) { + pushScope({ functionScope: true }) + + if (node.id && node.id.type === 'Identifier') { + defineBinding(node.id.name) + } + + for (const parameter of node.params) { + for (const name of collectBindingNames(parameter)) { + defineBinding(name) + } + } + }, + 'FunctionExpression:exit'() { + popScope() + }, ArrowFunctionExpression(node) { - enterScope(node) + pushScope({ functionScope: true }) + + for (const parameter of node.params) { + for (const name of collectBindingNames(parameter)) { + defineBinding(name) + } + } const { body } = node @@ -263,20 +382,58 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { formatExpression(body) } }, - 'ArrowFunctionExpression:exit': exitScope, - BlockStatement: enterScope, - 'BlockStatement:exit': exitScope, - CatchClause: enterScope, - 'CatchClause:exit': exitScope, - ForStatement: enterScope, - 'ForStatement:exit': exitScope, - ForInStatement: enterScope, - 'ForInStatement:exit': exitScope, - ForOfStatement: enterScope, - 'ForOfStatement:exit': exitScope, - SwitchStatement: enterScope, - 'SwitchStatement:exit': exitScope, + 'ArrowFunctionExpression:exit'() { + popScope() + }, + BlockStatement() { + pushScope() + }, + 'BlockStatement:exit'() { + popScope() + }, + ForStatement() { + pushScope() + }, + 'ForStatement:exit'() { + popScope() + }, + ForInStatement() { + pushScope() + }, + 'ForInStatement:exit'() { + popScope() + }, + ForOfStatement() { + pushScope() + }, + 'ForOfStatement:exit'() { + popScope() + }, + SwitchStatement() { + pushScope() + }, + 'SwitchStatement:exit'() { + popScope() + }, + CatchClause(node) { + pushScope() + + if (node.param) { + for (const name of collectBindingNames(node.param)) { + defineBinding(name) + } + } + }, + 'CatchClause:exit'() { + popScope() + }, ImportDeclaration(node) { + for (const specifier of node.specifiers) { + if ('local' in specifier && specifier.local.type === 'Identifier') { + defineBinding(specifier.local.name) + } + } + markModuleImportCreateRequire(node) const { source } = node @@ -294,15 +451,28 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { code.update(start + 1, end - 1, updated) } }, + VariableDeclaration(node) { + variableDeclarationKinds.push(node.kind) + }, + 'VariableDeclaration:exit'() { + variableDeclarationKinds.pop() + }, VariableDeclarator(node) { - markRequireDestructureCreateRequire(node) + const varScoped = variableDeclarationKinds.at(-1) === 'var' + const names = collectBindingNames(node.id) + + for (const name of names) { + defineBinding(name, 'other', { varScoped }) + } + + markRequireDestructureCreateRequire(node, { varScoped }) if (node.id.type !== 'Identifier' || node.init?.type !== 'CallExpression') { return } if (isCreateRequireCall(node.init)) { - setBindingKind(node.id.name, 'requireAlias') + defineBinding(node.id.name, 'requireAlias', { varScoped }) } }, AssignmentExpression(node) { @@ -313,12 +483,12 @@ const format = async (src: string, ast: ParseResult, cb: Callback) => { const { name } = node.left if (node.right.type === 'CallExpression' && isCreateRequireCall(node.right)) { - setBindingKind(name, 'requireAlias') + updateBinding(name, 'requireAlias') return } if (getBindingKind(name) === 'requireAlias') { - setBindingKind(name, 'other') + updateBinding(name, 'other') } }, ExpressionStatement(node) { diff --git a/test/updateSrc.ts b/test/updateSrc.ts index 01ee63b..30e9191 100644 --- a/test/updateSrc.ts +++ b/test/updateSrc.ts @@ -348,4 +348,83 @@ describe('updateSrc', () => { assert.ok(update.indexOf("require('./skip.js')") > -1) assert.ok(update.indexOf("require('./rewrite.mjs')") > -1) }) + + it('does not rewrite require shadowed by for-in scope', async () => { + const update = await specifier.updateSrc( + ` + for (const require in { value: 1 }) { + require('./skip.js') + } + + require('./rewrite.js') + `, + 'js', + spec => { + return spec.value.replace('.js', '.mjs') + }, + ) + + assert.ok(update.indexOf("require('./skip.js')") > -1) + assert.ok(update.indexOf("require('./rewrite.mjs')") > -1) + }) + + it('does not rewrite require shadowed by for scope', async () => { + const update = await specifier.updateSrc( + ` + for (let require = value => value; false; ) { + require('./skip.js') + } + + require('./rewrite.js') + `, + 'js', + spec => { + return spec.value.replace('.js', '.mjs') + }, + ) + + assert.ok(update.indexOf("require('./skip.js')") > -1) + assert.ok(update.indexOf("require('./rewrite.mjs')") > -1) + }) + + it('does not rewrite require shadowed by switch scope', async () => { + const update = await specifier.updateSrc( + ` + switch (0) { + default: + const require = value => value + require('./skip.js') + } + + require('./rewrite.js') + `, + 'js', + spec => { + return spec.value.replace('.js', '.mjs') + }, + ) + + assert.ok(update.indexOf("require('./skip.js')") > -1) + assert.ok(update.indexOf("require('./rewrite.mjs')") > -1) + }) + + it('keeps require scope in sync across a re-export scope', async () => { + const update = await specifier.updateSrc( + ` + export { foo } from './reexport.js' + + const { createRequire } = require('node:module') + const customRequire = createRequire(import.meta.url) + + customRequire('./rewrite.js') + `, + 'js', + spec => { + return spec.value.replace('.js', '.mjs') + }, + ) + + assert.ok(update.indexOf("from './reexport.mjs'") > -1) + assert.ok(update.indexOf("customRequire('./rewrite.mjs')") > -1) + }) }) From bd93d7a4107e2b2f10a79e7936906a0ae42fb6cb Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 31 May 2026 16:41:03 -0500 Subject: [PATCH 4/6] chore: bump version. --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9feb481..adbeaaf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@knighted/specifier", - "version": "3.1.0", + "version": "3.1.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@knighted/specifier", - "version": "3.1.0", + "version": "3.1.1", "license": "MIT", "dependencies": { "magic-string": "^0.30.21", diff --git a/package.json b/package.json index cee1294..9fc6db5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@knighted/specifier", - "version": "3.1.0", + "version": "3.1.1", "description": "Node.js tool for updating your ES module and CommonJS specifiers.", "type": "module", "main": "dist", From ab8c47e725e803ca6b6767b985610547e695c723 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 31 May 2026 16:42:57 -0500 Subject: [PATCH 5/6] test: better spec description. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- test/updateSrc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/updateSrc.ts b/test/updateSrc.ts index 30e9191..068dda6 100644 --- a/test/updateSrc.ts +++ b/test/updateSrc.ts @@ -408,7 +408,7 @@ describe('updateSrc', () => { assert.ok(update.indexOf("require('./rewrite.mjs')") > -1) }) - it('keeps require scope in sync across a re-export scope', async () => { + it('does not desync createRequire tracking after a re-export declaration', async () => { const update = await specifier.updateSrc( ` export { foo } from './reexport.js' From 53f6e434c3a7466127a2d657295098d682a1ff65 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 31 May 2026 16:43:28 -0500 Subject: [PATCH 6/6] chore: bump minor version. --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index adbeaaf..946a8fe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@knighted/specifier", - "version": "3.1.1", + "version": "3.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@knighted/specifier", - "version": "3.1.1", + "version": "3.2.0", "license": "MIT", "dependencies": { "magic-string": "^0.30.21", diff --git a/package.json b/package.json index 9fc6db5..f614468 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@knighted/specifier", - "version": "3.1.1", + "version": "3.2.0", "description": "Node.js tool for updating your ES module and CommonJS specifiers.", "type": "module", "main": "dist",