diff --git a/rewrite-javascript/rewrite/src/javascript/add-import.ts b/rewrite-javascript/rewrite/src/javascript/add-import.ts index 05d641a936..3acfa297eb 100644 --- a/rewrite-javascript/rewrite/src/javascript/add-import.ts +++ b/rewrite-javascript/rewrite/src/javascript/add-import.ts @@ -5,6 +5,7 @@ import {randomId} from "../uuid"; import {emptyMarkers, markers} from "../markers"; import {getStyle, PrettierStyle, SpacesStyle, StyleKind} from "./style"; import {Cursor} from "../tree"; +import {bindingNames, namesDeclaredIn} from "./scope"; export type QuoteChar = "'" | '"'; @@ -143,19 +144,18 @@ export function maybeAddImport( } const derived = derivedName(options); - const cu = compilationUnitOf(visitor); + const cursor = cursorOf(visitor); + const cu = cursor && compilationUnitOf(cursor); if (!cu) { visitor.afterVisit.push(new AddImport(options, derived)); return derived; } - const bindings = bindingsInScope(cu, cursorOf(visitor)); - // An import already serving this request answers it; queuing one would, on the next cycle, // derive a suffixed name from the binding this call just added. A caller that named a preference // takes whatever comes back; one that did not assumes the name it derived, so a binding under // any other name would leave the references it emits unbound. - for (const binding of bindings) { + for (const binding of moduleScopeBindings(cu)) { if (binding.module === module && binding.member === memberName(options.member) && binding.typeOnly === typeOnly && (options.preferredName !== undefined || anyNameAnswers(options) || @@ -164,7 +164,10 @@ export function maybeAddImport( } } - const name = deconflict(derived, takenNames(bindings, visitor)); + // Only the module scope answers for a name, but any scope in the file occupies one. The queue + // gives every later request for this module the name chosen here, so it has to clear the scopes + // those references will sit in, which are not known yet. + const name = deconflict(derived, takenNames(namesDeclaredIn(cu), visitor)); visitor.afterVisit.push(new AddImport(options, name)); return name; } @@ -230,73 +233,29 @@ interface ModuleScopeBinding { } function cursorOf(visitor: JavaScriptVisitor): Cursor | undefined { - return (visitor as unknown as { cursor?: Cursor }).cursor; -} - -function compilationUnitOf(visitor: JavaScriptVisitor): JS.CompilationUnit | undefined { // `cursor` is protected on `TreeVisitor`, and the `maybeAddImport`/`maybeRemoveImport` // API is free functions, so reaching it takes a cast. - return cursorOf(visitor)?.firstEnclosing((v): v is JS.CompilationUnit => v?.kind === JS.Kind.CompilationUnit); + return (visitor as unknown as { cursor?: Cursor }).cursor; } -/** Every name in scope at `cursor`: what the file binds, and what each block enclosing it declares. */ -function bindingsInScope(cu: JS.CompilationUnit, cursor: Cursor | undefined): ModuleScopeBinding[] { - const bindings = statementBindings(cu.statements); - // A local shadows an import for the code the template lands in, so a name a block declares is - // taken there even though the module never answers for it. - for (let c = cursor; c && c.value !== cu; c = c.parent) { - for (const name of scopeNames(c.value)) { - bindings.push({name}); - } - } - return bindings; +function compilationUnitOf(cursor: Cursor): JS.CompilationUnit | undefined { + return cursor.firstEnclosing((v): v is JS.CompilationUnit => v?.kind === JS.Kind.CompilationUnit); } -/** The names a scope introduces directly: a block's declarations, or a function's parameters. */ -function scopeNames(scope: unknown): string[] { - const node = scope as J | undefined; - if (node?.kind === J.Kind.Block) { - return statementBindings((node as J.Block).statements).map(binding => binding.name); - } - if (node?.kind === J.Kind.MethodDeclaration) { - return (node as J.MethodDeclaration).parameters.elements - .flatMap(param => param.element?.kind === J.Kind.VariableDeclarations - ? (param.element as J.VariableDeclarations).variables.flatMap(v => patternNames(v.element?.name)) - : patternNames(param.element)) - .map(bound => bound.name); - } - if (node?.kind === J.Kind.Lambda) { - return (node as J.Lambda).parameters.parameters - .flatMap(param => param.element?.kind === J.Kind.VariableDeclarations - ? (param.element as J.VariableDeclarations).variables.flatMap(v => patternNames(v.element?.name)) - : patternNames(param.element)) - .map(bound => bound.name); - } - return []; -} - -/** Imports are top-level statements and so is anything that can shadow one, so a flat scan sees every name. */ -function statementBindings(statements: J.RightPadded[]): ModuleScopeBinding[] { +/** What the file's imports and `require`s bind at module scope, and the module each name comes from. */ +function moduleScopeBindings(cu: JS.CompilationUnit): ModuleScopeBinding[] { const bindings: ModuleScopeBinding[] = []; - const declaredBy = (name: J | undefined): void => { - for (const bound of patternNames(name)) { - bindings.push({name: bound.name}); - } - }; - const declaredByVariables = (varDecl: J.VariableDeclarations): void => { for (const variable of varDecl.variables) { const required = requiredModule(variable.element?.initializer?.element); if (required !== undefined) { bindings.push(...requireBindings(variable.element?.name, required)); - } else { - declaredBy(variable.element?.name); } } }; - for (const stmt of statements) { + for (const stmt of cu.statements) { const statement = stmt.element; switch (statement?.kind) { case JS.Kind.Import: @@ -312,18 +271,6 @@ function statementBindings(statements: J.RightPadded[]): ModuleScopeB } } break; - case J.Kind.MethodDeclaration: - declaredBy((statement as J.MethodDeclaration).name); - break; - case J.Kind.ClassDeclaration: - declaredBy((statement as J.ClassDeclaration).name); - break; - case JS.Kind.NamespaceDeclaration: - declaredBy((statement as JS.NamespaceDeclaration).name.element); - break; - case JS.Kind.TypeDeclaration: - declaredBy((statement as JS.TypeDeclaration).name.element); - break; } } @@ -357,55 +304,13 @@ function requiredModuleOf(methodInv: J.MethodInvocation): string | undefined { : undefined; } -/** - * Every name a binding pattern introduces. `member` is the property a name takes its value from, - * and only a name bound directly by the pattern has one — anything deeper reads a property of a - * property, so it occupies its name without binding a member of the module. - */ -function patternNames(pattern: J | undefined): { name: string; member?: string }[] { - if (pattern?.kind === J.Kind.Identifier) { - return [{name: (pattern as J.Identifier).simpleName}]; - } - // An array pattern binds by position, so its elements name no member of what they destructure. - if (pattern?.kind === JS.Kind.ArrayBindingPattern) { - return (pattern as JS.ArrayBindingPattern).elements.elements - .flatMap(elem => elem.element?.kind === JS.Kind.BindingElement - ? patternNames((elem.element as JS.BindingElement).name) - : patternNames(elem.element)) - .map(bound => ({name: bound.name})); - } - if (pattern?.kind !== JS.Kind.ObjectBindingPattern) { - return []; - } - const names: { name: string; member?: string }[] = []; - for (const elem of (pattern as JS.ObjectBindingPattern).bindings.elements) { - if (elem.element?.kind !== JS.Kind.BindingElement) { - continue; - } - const bindingElem = elem.element as JS.BindingElement; - if (bindingElem.name?.kind === J.Kind.Identifier) { - const name = (bindingElem.name as J.Identifier).simpleName; - const propertyName = bindingElem.propertyName?.element; - names.push({ - name, - member: propertyName?.kind === J.Kind.Identifier - ? (propertyName as J.Identifier).simpleName - : name - }); - } else { - names.push(...patternNames(bindingElem.name).map(bound => ({name: bound.name}))); - } - } - return names; -} - /** A `require` binds a module the way an import does, so the pool records it the same way. */ function requireBindings(pattern: J | undefined, module: string): ModuleScopeBinding[] { // A whole-module require binds no member, exactly as a default import does. if (pattern?.kind === J.Kind.Identifier) { return [{name: (pattern as J.Identifier).simpleName, module, member: undefined, typeOnly: false}]; } - return patternNames(pattern).map(bound => bound.member === undefined + return bindingNames(pattern).map(bound => bound.member === undefined ? {name: bound.name} : {name: bound.name, module, member: bound.member, typeOnly: false}); } @@ -465,12 +370,12 @@ function importBindings(jsImport: JS.Import): ModuleScopeBinding[] { } /** - * Names the file binds, plus those pending `AddImport`s on the `afterVisit` queue have claimed. A - * queued `RemoveImport` does not free one: it removes only what the file leaves unused, and binding - * a name it keeps is an error, where an unnecessary suffix merely reads oddly. + * Names in scope, plus those pending `AddImport`s on the `afterVisit` queue have claimed. A queued + * `RemoveImport` does not free one: it removes only what the file leaves unused, and binding a name + * it keeps is an error, where an unnecessary suffix merely reads oddly. */ -function takenNames(bindings: ModuleScopeBinding[], visitor: JavaScriptVisitor): Set { - const taken = new Set(); +function takenNames(inScope: ReadonlySet, visitor: JavaScriptVisitor): Set { + const taken = new Set(inScope); for (const v of visitor.afterVisit || []) { if (v instanceof AddImport && v.bindingName) { @@ -478,10 +383,6 @@ function takenNames(bindings: ModuleScopeBinding[], visitor: JavaScriptVisitor extends JavaScriptVisitor

{ shouldRemove = !usedIdentifiers.has(name) && !usedTypes.has(name); } else { // Regular case: check if the import name matches the removal criteria - shouldRemove = this.shouldRemoveImport(name, usedIdentifiers, usedTypes); + shouldRemove = this.shouldRemoveImport(name, usedIdentifiers, usedTypes, name); } if (shouldRemove) { @@ -325,7 +326,7 @@ export class RemoveImport

extends JavaScriptVisitor

{ }, p); } // Namespace is used, we can't remove individual members from it - } else if (this.shouldRemoveImport(name, usedIdentifiers, usedTypes)) { + } else if (this.shouldRemoveImport(name, usedIdentifiers, usedTypes, name)) { // If there's no default import, remove the entire import if (!importClause.name) { return undefined; @@ -355,7 +356,7 @@ export class RemoveImport

extends JavaScriptVisitor

{ }, p); } // Namespace is used, we can't remove individual members from it - } else if (this.shouldRemoveImport(aliasName, usedIdentifiers, usedTypes)) { + } else if (this.shouldRemoveImport(aliasName, usedIdentifiers, usedTypes, aliasName)) { // If there's no default import, remove the entire import if (!importClause.name) { return undefined; @@ -490,7 +491,7 @@ export class RemoveImport

extends JavaScriptVisitor

{ return true; // Keep imports that don't match the member } else { // We're removing based on the import name itself - return !this.shouldRemoveImport(importName, usedIdentifiers, usedTypes); + return !this.shouldRemoveImport(importName, usedIdentifiers, usedTypes, importName); } } return true; // Keep non-ImportSpecifier elements @@ -612,14 +613,10 @@ export class RemoveImport

extends JavaScriptVisitor

{ const {filtered, allRemoved} = await this.filterElementsWithPrefixPreservation( pattern.bindings.elements, (elem: J) => { - if (elem.kind === JS.Kind.BindingElement) { - const name = this.getBindingElementName(elem as JS.BindingElement); - return !this.shouldRemoveImport(name, usedIdentifiers, new Set()); - } else if (elem.kind === J.Kind.Identifier) { - const name = (elem as J.Identifier).simpleName; - return !this.shouldRemoveImport(name, usedIdentifiers, new Set()); - } - return true; // Keep other element types + const bound = this.boundByPatternElement(elem); + // An element whose names cannot be read is left alone. + return bound.length === 0 || + bound.some(b => !this.shouldRemoveImport(b.name, usedIdentifiers, new Set(), b.member)); }, async (elem: J, prefix: J.Space) => { if (elem.kind === J.Kind.Identifier) { @@ -687,23 +684,25 @@ export class RemoveImport

extends JavaScriptVisitor

{ return undefined; } - private getBindingElementName(bindingElement: JS.BindingElement): string { - const name = bindingElement.name; - if (name?.kind === J.Kind.Identifier) { - return (name as J.Identifier).simpleName; - } - return ''; + /** The names a pattern element binds, and for each the member of the module it reads. */ + private boundByPatternElement(elem: J): { name: string; member?: string }[] { + // Shorthand, so the name it binds is the member it reads. + return elem.kind === J.Kind.Identifier + ? [{name: (elem as J.Identifier).simpleName, member: (elem as J.Identifier).simpleName}] + : bindingNames(elem); } private shouldRemoveImport( name: string, usedIdentifiers: Set, - usedTypes: Set + usedTypes: Set, + member: string | undefined ): boolean { // If member is specified, we're removing a specific member from the module if (this.member !== undefined) { - // Only remove if this is the specific member we're looking for - if (this.member !== name) { + // A name bound under an alias reads one member and is referenced by another, and a name + // a nested pattern binds reads a property of a property, so it reads no member at all. + if (this.member !== member) { return false; } } diff --git a/rewrite-javascript/rewrite/src/javascript/scope.ts b/rewrite-javascript/rewrite/src/javascript/scope.ts new file mode 100644 index 0000000000..5c95ea0acb --- /dev/null +++ b/rewrite-javascript/rewrite/src/javascript/scope.ts @@ -0,0 +1,290 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import {Cursor, isTree} from "../tree"; +import {J} from "../java"; +import {JS} from "./tree"; + +/** The names code at some position can reach unqualified. */ +export interface Scope { + /** Whether this scope or one enclosing it binds `name`. */ + declares(name: string): boolean; + + names(): ReadonlySet; +} + +/** + * What the code at `cursor` can name: what every enclosing scope binds, plus the declarations that + * hoist into those scopes from blocks that do not enclose it. Where a declaration's shape leaves its + * reach unreadable the answer counts it rather than miss it, so a name reported here may not truly + * reach the cursor. + */ +export function scopeOf(cursor: Cursor): Scope { + let resolved: Set | undefined; + const names = () => resolved ??= namesInScope(cursor); + return {declares: name => names().has(name), names}; +} + +/** + * Every name the file declares, wherever it sits. A binding the whole file shares is referenced from + * sites that are not known when it is named, so it has to steer clear of every name that could + * shadow it at one of them. + */ +export function namesDeclaredIn(cu: JS.CompilationUnit): ReadonlySet { + const cached = declared.get(cu); + if (cached) { + return cached; + } + + const names = new Set(); + const collect = (node: any): boolean => { + declarationNames(node).forEach(name => names.add(name)); + if (node.kind !== J.Kind.ClassDeclaration) { + return true; + } + // A member's name is not one the file binds, though the code inside one still declares names. + for (const member of (node as J.ClassDeclaration).body?.statements ?? []) { + const element = unwrap(member); + walk(element, node => node === element || collect(node)); + } + return false; + }; + walk(cu.statements, collect); + + declared.set(cu, names); + return names; +} + +/** + * Every name a binding pattern introduces. `member` is the property a name takes its value from, + * which only a name an object pattern binds directly has: anything deeper reads a property of a + * property, an array element is chosen by position, and a rest name gathers what nothing claimed. + */ +export function bindingNames(pattern: J | undefined): { name: string; member?: string }[] { + switch (pattern?.kind) { + case J.Kind.Identifier: { + const simpleName = (pattern as J.Identifier).simpleName; + return simpleName ? [{name: simpleName}] : []; + } + case JS.Kind.Spread: + return bindingNames((pattern as JS.Spread).expression); + case JS.Kind.ArrayBindingPattern: + return unnamedMembers((pattern as JS.ArrayBindingPattern).elements.elements + .flatMap(element => bindingNames(unwrap(element)))); + case JS.Kind.ObjectBindingPattern: + return (pattern as JS.ObjectBindingPattern).bindings.elements + .flatMap(element => bindingNames(unwrap(element))); + case JS.Kind.BindingElement: { + const element = pattern as JS.BindingElement; + if (element.name?.kind !== J.Kind.Identifier) { + return unnamedMembers(bindingNames(element.name as J)); + } + const name = (element.name as J.Identifier).simpleName; + const propertyName = unwrap(element.propertyName); + return [{ + name, + member: propertyName?.kind === J.Kind.Identifier ? (propertyName as J.Identifier).simpleName : name + }]; + } + default: + return []; + } +} + +function unnamedMembers(bound: { name: string }[]): { name: string }[] { + return bound.map(({name}) => ({name})); +} + +function namesInScope(cursor: Cursor): Set { + const names = new Set(); + for (let c: Cursor | undefined = cursor; c; c = c.parent) { + for (const name of frameBindings(c.value, c.parent?.value)) { + names.add(name); + } + } + return names; +} + +/** What one node on the cursor path binds, `parent` being the node it hangs from. */ +function frameBindings(node: any, parent: any): string[] { + switch (node?.kind) { + case JS.Kind.CompilationUnit: { + const statements = (node as JS.CompilationUnit).statements; + return [...declaredNames(statements), ...hoistedNames(statements)]; + } + case J.Kind.Block: + // A class body holds members, which are reached through an instance rather than by name. + return parent?.kind === J.Kind.ClassDeclaration ? [] : declaredNames((node as J.Block).statements); + case J.Kind.MethodDeclaration: { + const method = node as J.MethodDeclaration; + // A function expression is the only function whose own name its body reaches: a + // declaration's name belongs to the enclosing block, a method's to an instance. + const self = parent?.kind === JS.Kind.StatementExpression ? bindingNames(method.name) : []; + return [ + ...self.map(bound => bound.name), + ...declaredNames(method.parameters.elements), + ...hoistedNames(method.body) + ]; + } + case J.Kind.Lambda: { + const lambda = node as J.Lambda; + return [...declaredNames(lambda.parameters.parameters), ...hoistedNames(lambda.body)]; + } + case J.Kind.ClassDeclaration: + return bindingNames((node as J.ClassDeclaration).name).map(bound => bound.name); + case J.Kind.TryCatch: + return declaredNames([(node as J.Try.Catch).parameter.tree]); + case J.Kind.ForLoop: + return declaredNames((node as J.ForLoop).control.init); + case J.Kind.ForEachLoop: + return declaredNames([(node as J.ForEachLoop).control.variable]); + case JS.Kind.ForInLoop: + return declaredNames([(node as JS.ForInLoop).control.variable]); + default: + return []; + } +} + +/** The names statements declare directly in the scope holding them. */ +function declaredNames(statements: any[]): string[] { + return statements.flatMap(statement => declarationNames(unwrap(statement))); +} + +function declarationNames(statement: any): string[] { + switch (statement?.kind) { + case JS.Kind.Import: + return importNames(statement as JS.Import); + case J.Kind.VariableDeclarations: + return (statement as J.VariableDeclarations).variables + .flatMap(variable => bindingNames(unwrap(variable)?.name)) + .map(bound => bound.name); + case JS.Kind.ScopedVariableDeclarations: + return declaredNames((statement as JS.ScopedVariableDeclarations).variables); + case J.Kind.MethodDeclaration: + return bindingNames((statement as J.MethodDeclaration).name).map(bound => bound.name); + case J.Kind.ClassDeclaration: + return bindingNames((statement as J.ClassDeclaration).name).map(bound => bound.name); + case JS.Kind.NamespaceDeclaration: + return bindingNames(unwrap((statement as JS.NamespaceDeclaration).name)).map(bound => bound.name); + case JS.Kind.TypeDeclaration: + return bindingNames(unwrap((statement as JS.TypeDeclaration).name)).map(bound => bound.name); + case J.Kind.Case: + // The cases of a switch share the block it opens, so each one's declarations bind in all. + return declaredNames((statement as J.Case).statements.elements); + default: + return []; + } +} + +/** The names an import binds, which for an aliased or namespace specifier is the alias. */ +function importNames(jsImport: JS.Import): string[] { + const importClause = jsImport.importClause; + if (!importClause) { + return []; + } + const names = bindingNames(unwrap(importClause.name)).map(bound => bound.name); + const namedBindings = importClause.namedBindings; + if (namedBindings?.kind === JS.Kind.NamedImports) { + for (const element of (namedBindings as JS.NamedImports).elements.elements) { + const specifier = unwrap(element); + if (specifier?.kind === JS.Kind.ImportSpecifier) { + names.push(...aliasedName((specifier as JS.ImportSpecifier).specifier)); + } + } + } else { + names.push(...aliasedName(namedBindings)); + } + return names; +} + +function aliasedName(specifier: J | undefined): string[] { + const name = specifier?.kind === JS.Kind.Alias ? (specifier as JS.Alias).alias : specifier; + return bindingNames(name).map(bound => bound.name); +} + +const blockScoped = new Set(['let', 'const', 'using']); + +// A walk of an immutable subtree has one answer, so it runs once. Keyed on the subtree itself, a +// replaced one is walked afresh: every call site in a function asks what that body hoists, and every +// import added to a file asks what that file declares. +const hoisted = new WeakMap(); +const declared = new WeakMap>(); + +/** + * The names blocks under `scope` hoist out to it. A `var` or function declaration reaches the whole + * function it sits in, so one nested in a block is in scope outside that block; only a `let`, `const` + * or `using` keyword says otherwise. + */ +function hoistedNames(scope: any): string[] { + if (typeof scope !== 'object' || scope === null) { + return []; + } + const cached = hoisted.get(scope); + if (cached) { + return cached; + } + + const names: string[] = []; + const collect = (node: any): boolean => { + switch (node.kind) { + case J.Kind.MethodDeclaration: + names.push(...declarationNames(node)); + return false; + case J.Kind.VariableDeclarations: + case JS.Kind.ScopedVariableDeclarations: + if (!((node.modifiers ?? []) as J.Modifier[]).some(m => blockScoped.has(m.keyword!))) { + names.push(...declarationNames(node)); + } + return false; + case J.Kind.TryCatch: + // A catch parameter carries no keyword to read, and binds only in the catch block. + walk((node as J.Try.Catch).body, collect); + return false; + case JS.Kind.StatementExpression: + // A function or class used as an expression names itself for its own body alone. + return false; + case J.Kind.Lambda: + case J.Kind.ClassDeclaration: + return false; + default: + return true; + } + }; + walk(scope, collect); + hoisted.set(scope, names); + return names; +} + +/** Visits every LST node under `node`, leaving a subtree unvisited where `visit` returns false. */ +function walk(node: unknown, visit: (node: any) => boolean): void { + if (Array.isArray(node)) { + node.forEach(child => walk(child, visit)); + return; + } + const kind = (node as any)?.kind; + if (kind === J.Kind.RightPadded || kind === J.Kind.LeftPadded) { + walk((node as J.RightPadded).element, visit); + } else if (kind === J.Kind.Container) { + walk((node as J.Container).elements, visit); + } else if (isTree(node) && visit(node)) { + // Markers hang off a node rather than being part of the code it holds. + Object.entries(node).forEach(([key, value]) => key !== 'markers' && walk(value, visit)); + } +} + +/** The element a padding wrapper holds, or the node itself. */ +function unwrap(node: any): any { + return node?.kind === J.Kind.RightPadded || node?.kind === J.Kind.LeftPadded ? unwrap(node.element) : node; +} diff --git a/rewrite-javascript/rewrite/test/javascript/add-import.test.ts b/rewrite-javascript/rewrite/test/javascript/add-import.test.ts index 94b7448578..a7d45238b1 100644 --- a/rewrite-javascript/rewrite/test/javascript/add-import.test.ts +++ b/rewrite-javascript/rewrite/test/javascript/add-import.test.ts @@ -3016,8 +3016,7 @@ describe('AddImport visitor', () => { const spec = new RecipeSpec(); const bound: string[] = []; spec.recipe = fromVisitor(new class extends JavaScriptVisitor { - // Each anchor asks for a different module, so the queue never answers one from - // another and every scope is resolved on its own. + // Each anchor asks for a different module, so the queue never answers one from another. override async visitMethodInvocation(method: J.MethodInvocation, p: any): Promise { if ((method.name as J.Identifier)?.simpleName === 'anchor') { bound.push(maybeAddImport(this, @@ -3040,11 +3039,19 @@ describe('AddImport visitor', () => { } const byLambda = (merge) => anchor(); + + function byHoistedVar() { + if (x) { + var merge = 1; + } + anchor(); + } `, ` import {merge as merge_1} from 'm0'; import {merge as merge_2} from 'm1'; import {merge as merge_3} from 'm2'; + import {merge as merge_4} from 'm3'; function byParameter(merge) { anchor(); @@ -3056,11 +3063,61 @@ describe('AddImport visitor', () => { } const byLambda = (merge) => anchor(); + + function byHoistedVar() { + if (x) { + var merge = 1; + } + anchor(); + } + ` + ) + ); + + expect(bound).toEqual(['merge_1', 'merge_2', 'merge_3', 'merge_4']); + }); + + test('one binding serves every site, so a name any of them shadows is taken', async () => { + const spec = new RecipeSpec(); + const bound: string[] = []; + spec.recipe = fromVisitor(new class extends JavaScriptVisitor { + override async visitMethodInvocation(method: J.MethodInvocation, p: any): Promise { + if ((method.name as J.Identifier)?.simpleName === 'anchor') { + bound.push(maybeAddImport(this, + {module: 'sap/base/util/merge', member: 'merge', onlyIfReferenced: false})); + } + return super.visitMethodInvocation(method, p); + } + }); + + await spec.rewriteRun( + typescript( + ` + function free() { + anchor(); + } + + function shadowed(merge) { + anchor(); + } + `, + ` + import {merge as merge_1} from 'sap/base/util/merge'; + + function free() { + anchor(); + } + + function shadowed(merge) { + anchor(); + } ` ) ); - expect(bound).toEqual(['merge_1', 'merge_2', 'merge_3']); + // `free` would take plain `merge`, but the one import both sites reference has to clear + // the parameter too, or the reference in `shadowed` binds to it. + expect(bound).toEqual(['merge_1', 'merge_1']); }); test('a request that named nothing takes the name the file already gives the module', async () => { diff --git a/rewrite-javascript/rewrite/test/javascript/remove-import.test.ts b/rewrite-javascript/rewrite/test/javascript/remove-import.test.ts index bf4a2b7aef..b663100af8 100644 --- a/rewrite-javascript/rewrite/test/javascript/remove-import.test.ts +++ b/rewrite-javascript/rewrite/test/javascript/remove-import.test.ts @@ -585,6 +585,40 @@ describe('RemoveImport visitor', () => { ); }); + test('should remove a destructured require member bound under another name', async () => { + const spec = new RecipeSpec(); + spec.recipe = fromVisitor(new RemoveImport("fs", "readFile")); + + //language=typescript + await spec.rewriteRun( + typescript( + ` + const {readFile: rf, writeFile} = require('fs'); + + function example() { + writeFile('test.txt', 'content', () => { + }); + } + `, + ` + const {writeFile} = require('fs'); + + function example() { + writeFile('test.txt', 'content', () => { + }); + } + ` + ), + // `readFile` here reads a property of `a`, not a member of the module, so nothing + // the module exports is bound and there is nothing to remove even though it is unused. + typescript( + ` + const {a: {readFile}} = require('fs'); + ` + ) + ); + }); + test('should remove destructured require', async () => { const spec = new RecipeSpec(); spec.recipe = fromVisitor(new RemoveImport("fs", "readFile")); diff --git a/rewrite-javascript/rewrite/test/javascript/scope.test.ts b/rewrite-javascript/rewrite/test/javascript/scope.test.ts new file mode 100644 index 0000000000..c72de73b64 --- /dev/null +++ b/rewrite-javascript/rewrite/test/javascript/scope.test.ts @@ -0,0 +1,224 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + bindingNames, + JavaScriptParser, + JavaScriptVisitor, + JS, + namesDeclaredIn, + Scope, + scopeOf, + sourceFileCache +} from "../../src/javascript"; +import {J} from "../../src/java"; + +const parser = new JavaScriptParser({sourceFileCache}); + +async function parse(source: string, sourcePath = 'test.ts'): Promise { + return (await parser.parse({text: source, sourcePath}).next()).value as JS.CompilationUnit; +} + +/** The scope where the source calls `anchor()`. */ +async function scopeAtAnchor(source: string, sourcePath?: string): Promise { + const found: Scope[] = []; + await new class extends JavaScriptVisitor { + override async visitMethodInvocation(method: J.MethodInvocation, p: undefined): Promise { + if ((method.name as J.Identifier)?.simpleName === 'anchor') { + found.push(scopeOf(this.cursor)); + } + return super.visitMethodInvocation(method, p); + } + }().visit(await parse(source, sourcePath), undefined); + expect(found).toHaveLength(1); + return found[0]; +} + +async function namesAtAnchor(source: string, sourcePath?: string): Promise { + return [...(await scopeAtAnchor(source, sourcePath)).names()].sort(); +} + +describe('scopeOf', () => { + test('a module binds every form of import', async () => { + expect(await namesAtAnchor(` + import def from 'a'; + import {named, other as renamed} from 'b'; + import * as ns from 'c'; + import 'd'; + anchor(); + `)).toEqual(['def', 'named', 'ns', 'renamed']); + }); + + test('a module binds what its declarations name', async () => { + expect(await namesAtAnchor(` + const c = 1; + let l = 2; + var v = 3, w = 4; + function f() {} + class K {} + namespace N {} + type T = string; + const {r} = require('m'); + anchor(); + `)).toEqual(['K', 'N', 'T', 'c', 'f', 'l', 'r', 'v', 'w']); + }); + + test('a var reaches the anchor from a sibling block, a let does not', async () => { + const names = await namesAtAnchor(` + function f() { + if (x) { + var hoisted = 1; + let confined = 2; + } + anchor(); + } + `); + expect(names).toContain('hoisted'); + expect(names).not.toContain('confined'); + }); + + test('a block binds its declarations wherever the anchor sits in it', async () => { + // `const` binds its whole block, so referencing it above its declaration is a temporal dead + // zone error rather than a reference to whatever the name means outside. + expect(await namesAtAnchor(` + function f() { + anchor(); + const later = 1; + } + `)).toContain('later'); + }); + + test("a switch's cases share one scope", async () => { + expect(await namesAtAnchor(` + switch (x) { + case 1: + let s = 1; + break; + case 2: + anchor(); + } + `)).toContain('s'); + }); + + test('a function binds its parameters, destructured and rest included', async () => { + expect(await namesAtAnchor(` + function f(plain, {prop, other: renamed, nested: {deep}}, [first], ...rest) { + anchor(); + } + `)).toEqual(['deep', 'f', 'first', 'plain', 'prop', 'renamed', 'rest']); + }); + + test('an arrow function binds its parameters', async () => { + expect(await namesAtAnchor(`const f = (a, {b}) => anchor();`)).toEqual(['a', 'b', 'f']); + }); + + test('a catch binds its parameter in its block alone', async () => { + expect(await namesAtAnchor(`try {} catch ({message}) { anchor(); }`)).toContain('message'); + + expect(await namesAtAnchor(`try {} catch (err) {} anchor();`)).not.toContain('err'); + }); + + test('a loop binds what its control declares', async () => { + expect(await namesAtAnchor(`for (let i = 0; i < 2; i++) { anchor(); }`)).toContain('i'); + + expect(await namesAtAnchor(`for (const [key, value] of pairs) { anchor(); }`)) + .toEqual(expect.arrayContaining(['key', 'value'])); + + expect(await namesAtAnchor(`for (const prop in obj) { anchor(); }`)).toContain('prop'); + }); + + test('a class binds its own name, not its members', async () => { + // Only a class expression's name is bound solely inside the class — the block holding a + // declaration binds its name too. + const names = await namesAtAnchor(` + const C = class K { + member() { anchor(); } + }; + `); + expect(names).toContain('K'); + expect(names).not.toContain('member'); + }); + + test('a nested function keeps its declarations to itself', async () => { + const names = await namesAtAnchor(` + function outer() { + function inner(param) { + var hidden = 1; + } + anchor(); + } + `); + expect(names).toEqual(['inner', 'outer']); + }); + + test('a function expression binds the name it gives itself, and only there', async () => { + expect(await namesAtAnchor(`const f = function named() { anchor(); };`)).toContain('named'); + + expect(await namesAtAnchor(`function f() { (function named() {})(); anchor(); }`)).not.toContain('named'); + }); + + test('a scope reaches through the JSX between it and the cursor', async () => { + // A handler prop is where a template most often lands in a component, and JSX is all the + // cursor crosses to get from the lambda binding `evt` out to the module. + expect(await namesAtAnchor( + `import R from 'r';\nfunction App() { return ; }`, + 'test.tsx' + )).toEqual(['App', 'R', 'evt']); + }); + + test('a binding pattern names the property each name reads', async () => { + const cu = await parse(`const {plain, other: renamed, nested: {deep}, ...rest} = o;`); + const declaration = cu.statements[0].element as J.VariableDeclarations; + expect(bindingNames(declaration.variables[0].element.name)).toEqual([ + {name: 'plain', member: 'plain'}, + {name: 'renamed', member: 'other'}, + // Neither reads a property of the object the pattern destructures. + {name: 'deep'}, + {name: 'rest'} + ]); + }); + + test('declares answers for a name any enclosing scope binds', async () => { + const scope = await scopeAtAnchor(`import {merge} from 'm';\nfunction f(param) { anchor(); }`); + expect(scope.declares('merge')).toBe(true); + expect(scope.declares('param')).toBe(true); + expect(scope.declares('absent')).toBe(false); + }); +}); + +describe('namesDeclaredIn', () => { + test('a name declared in any scope is a name the file has', async () => { + expect([...namesDeclaredIn(await parse(` + import imported from 'm'; + const top = 1; + function fn(param) { + if (c) { var nested = 2; } + try {} catch (caught) {} + for (const [element] of pairs) {} + return (inline) => inline; + } + `))].sort()).toEqual(['caught', 'element', 'fn', 'imported', 'inline', 'nested', 'param', 'top']); + }); + + test('a class member is reached through an instance, so its name is not the file\'s', async () => { + // What a member's own code declares is still a name the file has. + expect([...namesDeclaredIn(await parse(` + class K { + merge(param) { const local = 1; } + field = (bound) => bound; + } + `))].sort()).toEqual(['K', 'bound', 'local', 'param']); + }); +});