From cf9e2e5667630889d4762e6a7437ba264ca6bedb Mon Sep 17 00:00:00 2001 From: David Michon Date: Sat, 28 Dec 2024 21:00:12 +0000 Subject: [PATCH 1/3] feat(scope-manager): Only add used lib globals --- .../src/referencer/Referencer.ts | 156 ++++++++++++++++-- packages/scope-manager/tests/lib.test.ts | 24 ++- 2 files changed, 161 insertions(+), 19 deletions(-) diff --git a/packages/scope-manager/src/referencer/Referencer.ts b/packages/scope-manager/src/referencer/Referencer.ts index 06c19b67e455..ea1bf9e7ecc9 100644 --- a/packages/scope-manager/src/referencer/Referencer.ts +++ b/packages/scope-manager/src/referencer/Referencer.ts @@ -3,6 +3,11 @@ import type { Lib, TSESTree } from '@typescript-eslint/types'; import { AST_NODE_TYPES } from '@typescript-eslint/types'; import type { GlobalScope, Scope } from '../scope'; +import { + ImplicitLibVariable, + type ImplicitLibVariableOptions, + type Variable, +} from '../variable'; import type { ScopeManager } from '../ScopeManager'; import type { ReferenceImplicitGlobal } from './Reference'; import type { VisitorOptions } from './Visitor'; @@ -19,6 +24,7 @@ import { VariableDefinition, } from '../definition'; import { lib as TSLibraries } from '../lib'; +import { TYPE, VALUE, TYPE_VALUE } from '../lib/base-config'; import { ClassVisitor } from './ClassVisitor'; import { ExportVisitor } from './ExportVisitor'; import { ImportVisitor } from './ImportVisitor'; @@ -33,6 +39,16 @@ interface ReferencerOptions extends VisitorOptions { lib: Lib[]; } +type ImplicitVariableMap = Map; + +// The `TSLibraries` object should have this type instead, but that's outside the scope of this prototype +const entriesByLib: Map = + new Map(); +// This object caches by the serialized JSON representation of the file's libs array +const variablesFromSerializedLibs: Map = new Map(); +// This object caches by reference from the file's libs array +const variablesFromRawLibs: WeakMap = new WeakMap(); + // Referencing variables and creating bindings. class Referencer extends Visitor { #hasReferencedJsxFactory = false; @@ -51,23 +67,125 @@ class Referencer extends Visitor { } private populateGlobalsFromLib(globalScope: GlobalScope): void { - for (const lib of this.#lib) { - const variables = TSLibraries[lib]; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - /* istanbul ignore if */ if (!variables) { - throw new Error(`Invalid value for lib provided: ${lib}`); + let implicitVariablesFromLibs: ImplicitVariableMap | undefined = + variablesFromRawLibs.get(this.#lib); + if (!implicitVariablesFromLibs) { + // Try to find out if this is a different object but same content we've seen before. + const libs: Lib[] = this.#lib.slice().sort(); + const serialized: string = JSON.stringify(libs); + implicitVariablesFromLibs = variablesFromSerializedLibs.get(serialized); + if (!implicitVariablesFromLibs) { + // No match, need to create it + implicitVariablesFromLibs = new Map(); + for (const lib of libs) { + let entriesForLib: + | [string, ImplicitLibVariableOptions][] + | undefined = entriesByLib.get(lib); + if (!entriesForLib) { + const variables = TSLibraries[lib]; + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + /* istanbul ignore if */ if (!variables) { + throw new Error(`Invalid value for lib provided: ${lib}`); + } + entriesForLib = Object.entries(variables); + entriesByLib.set(lib, entriesForLib); + } + + for (const [name, variable] of entriesForLib) { + const existing = implicitVariablesFromLibs.get(name); + // Since a variable can be a type, a value, or both, and it isn't + // guaranteed to be the same between libs, merges + if (existing) { + if (existing === TYPE_VALUE || existing === variable) { + continue; + } else if ( + (existing === VALUE && variable === TYPE) || + (existing === TYPE && variable === VALUE) + ) { + implicitVariablesFromLibs.set(name, TYPE_VALUE); + continue; + } + } + // variable is either net-new, or is upgrading to TYPE_VALUE + implicitVariablesFromLibs.set(name, variable); + } + } + variablesFromSerializedLibs.set(serialized, implicitVariablesFromLibs); } - for (const [name, variable] of Object.entries(variables)) { - globalScope.defineImplicitVariable(name, variable); + variablesFromRawLibs.set(this.#lib, implicitVariablesFromLibs); + } + + // Collect all names that are defined or referenced. + const allScopes: Set = new Set([globalScope]); + const allNames: Set = new Set(); + + for (const scope of allScopes) { + for (const childScope of scope.childScopes) { + allScopes.add(childScope); + } + + for (const reference of scope.references) { + allNames.add(reference.identifier.name); + } + + for (const variable of scope.variables) { + allNames.add(variable.name); } } - // for const assertions (`{} as const` / `{}`) - globalScope.defineImplicitVariable('const', { - eslintImplicitGlobalSetting: 'readonly', - isTypeVariable: true, - isValueVariable: false, - }); + // Rules only care about implicit globals if they are referenced or overlap with a local declaration, + // so add only those implicit globals. + const replacements: Map = new Map(); + const arraysToProcess: Set = new Set([globalScope.variables]); + const { declaredVariables } = this.scopeManager; + for (const name of allNames) { + const libVariable: ImplicitLibVariableOptions | undefined = + implicitVariablesFromLibs.get(name); + if (libVariable) { + const existingVariable = globalScope.set.get(name); + if (existingVariable) { + // This should be cleaned up + const newVariable = new ImplicitLibVariable( + globalScope, + name, + libVariable, + ); + replacements.set(existingVariable, newVariable); + globalScope.set.set(name, newVariable); + + // Copy over information about the conflicting declaration + for (const def of existingVariable.defs) { + newVariable.defs.push(def); + const parentDeclarations = + def.parent && declaredVariables.get(def.parent); + const nodeDeclarations = declaredVariables.get(def.node); + if (parentDeclarations) { + arraysToProcess.add(parentDeclarations); + } + if (nodeDeclarations) { + arraysToProcess.add(nodeDeclarations); + } + } + for (const identifier of existingVariable.identifiers) { + newVariable.identifiers.push(identifier); + } + // References should not have been bound yet, so no action should be needed. + } else { + globalScope.defineImplicitVariable(name, libVariable); + } + } + } + + if (replacements.size > 0) { + for (const array of arraysToProcess) { + for (let i = array.length - 1; i >= 0; i--) { + const replacement = replacements.get(array[i]); + if (replacement) { + array[i] = replacement; + } + } + } + } } public close(node: TSESTree.Node): void { while (this.currentScope(true) && node === this.currentScope().block) { @@ -569,7 +687,14 @@ class Referencer extends Visitor { protected Program(node: TSESTree.Program): void { const globalScope = this.scopeManager.nestGlobalScope(node); - this.populateGlobalsFromLib(globalScope); + + // Keep this first since it reduces the amount of snapshot changes significantly + // for const assertions (`{} as const` / `{}`) + globalScope.defineImplicitVariable('const', { + eslintImplicitGlobalSetting: 'readonly', + isTypeVariable: true, + isValueVariable: false, + }); if (this.scopeManager.isGlobalReturn()) { // Force strictness of GlobalScope to false when using node.js scope. @@ -586,6 +711,9 @@ class Referencer extends Visitor { } this.visitChildren(node); + // Need to call this after all references and variables are defined, + // but before we bind the global scope's references. + this.populateGlobalsFromLib(globalScope); this.close(node); } diff --git a/packages/scope-manager/tests/lib.test.ts b/packages/scope-manager/tests/lib.test.ts index eb558f3eb6e9..82f4a29f3563 100644 --- a/packages/scope-manager/tests/lib.test.ts +++ b/packages/scope-manager/tests/lib.test.ts @@ -11,15 +11,29 @@ describe('implicit lib definitions', () => { expect(variables).toHaveLength(1); }); - it('should define implicit variables', () => { - const { scopeManager } = parseAndAnalyze('', { + it('should define an implicit variable if there is a reference', () => { + const { scopeManager } = parseAndAnalyze('new ArrayBuffer();', { lib: ['es2015'], }); const variables = scopeManager.variables; - expect(variables.length).toBeGreaterThan(1); + expect( + variables.some( + v => v instanceof ImplicitLibVariable && v.name === 'ArrayBuffer', + ), + ).toEqual(true); + }); + + it('should define an implicit variable if there is a collision', () => { + const { scopeManager } = parseAndAnalyze('var Symbol = {};', { + lib: ['es2015'], + }); - const variable = variables[0]; - expect(variable).toBeInstanceOf(ImplicitLibVariable); + const variables = scopeManager.variables; + expect( + variables.some( + v => v instanceof ImplicitLibVariable && v.name === 'Symbol', + ), + ).toEqual(true); }); }); From 00d74a311d3783893731ebac937e78ad8108986b Mon Sep 17 00:00:00 2001 From: David Michon Date: Mon, 30 Dec 2024 19:58:28 +0000 Subject: [PATCH 2/3] chore(scope-manager): Fix lint issues --- .../src/referencer/Referencer.ts | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/scope-manager/src/referencer/Referencer.ts b/packages/scope-manager/src/referencer/Referencer.ts index ea1bf9e7ecc9..4d582baf6d9e 100644 --- a/packages/scope-manager/src/referencer/Referencer.ts +++ b/packages/scope-manager/src/referencer/Referencer.ts @@ -3,12 +3,8 @@ import type { Lib, TSESTree } from '@typescript-eslint/types'; import { AST_NODE_TYPES } from '@typescript-eslint/types'; import type { GlobalScope, Scope } from '../scope'; -import { - ImplicitLibVariable, - type ImplicitLibVariableOptions, - type Variable, -} from '../variable'; import type { ScopeManager } from '../ScopeManager'; +import type { ImplicitLibVariableOptions, Variable } from '../variable'; import type { ReferenceImplicitGlobal } from './Reference'; import type { VisitorOptions } from './Visitor'; @@ -25,6 +21,7 @@ import { } from '../definition'; import { lib as TSLibraries } from '../lib'; import { TYPE, VALUE, TYPE_VALUE } from '../lib/base-config'; +import { ImplicitLibVariable } from '../variable'; import { ClassVisitor } from './ClassVisitor'; import { ExportVisitor } from './ExportVisitor'; import { ImportVisitor } from './ImportVisitor'; @@ -42,12 +39,11 @@ interface ReferencerOptions extends VisitorOptions { type ImplicitVariableMap = Map; // The `TSLibraries` object should have this type instead, but that's outside the scope of this prototype -const entriesByLib: Map = - new Map(); +const entriesByLib = new Map(); // This object caches by the serialized JSON representation of the file's libs array -const variablesFromSerializedLibs: Map = new Map(); +const variablesFromSerializedLibs = new Map(); // This object caches by reference from the file's libs array -const variablesFromRawLibs: WeakMap = new WeakMap(); +const variablesFromRawLibs = new WeakMap(); // Referencing variables and creating bindings. class Referencer extends Visitor { @@ -71,7 +67,7 @@ class Referencer extends Visitor { variablesFromRawLibs.get(this.#lib); if (!implicitVariablesFromLibs) { // Try to find out if this is a different object but same content we've seen before. - const libs: Lib[] = this.#lib.slice().sort(); + const libs: Lib[] = [...this.#lib].sort(); const serialized: string = JSON.stringify(libs); implicitVariablesFromLibs = variablesFromSerializedLibs.get(serialized); if (!implicitVariablesFromLibs) { @@ -116,8 +112,8 @@ class Referencer extends Visitor { } // Collect all names that are defined or referenced. - const allScopes: Set = new Set([globalScope]); - const allNames: Set = new Set(); + const allScopes = new Set([globalScope]); + const allNames = new Set(); for (const scope of allScopes) { for (const childScope of scope.childScopes) { @@ -135,8 +131,8 @@ class Referencer extends Visitor { // Rules only care about implicit globals if they are referenced or overlap with a local declaration, // so add only those implicit globals. - const replacements: Map = new Map(); - const arraysToProcess: Set = new Set([globalScope.variables]); + const replacements = new Map(); + const arraysToProcess = new Set([globalScope.variables]); const { declaredVariables } = this.scopeManager; for (const name of allNames) { const libVariable: ImplicitLibVariableOptions | undefined = @@ -156,20 +152,25 @@ class Referencer extends Visitor { // Copy over information about the conflicting declaration for (const def of existingVariable.defs) { newVariable.defs.push(def); + // These arrays were created during defineVariable invocation const parentDeclarations = def.parent && declaredVariables.get(def.parent); - const nodeDeclarations = declaredVariables.get(def.node); if (parentDeclarations) { arraysToProcess.add(parentDeclarations); } + + const nodeDeclarations = declaredVariables.get(def.node); if (nodeDeclarations) { arraysToProcess.add(nodeDeclarations); } } + + // This mapping is unidirectional, so just copy for (const identifier of existingVariable.identifiers) { newVariable.identifiers.push(identifier); } - // References should not have been bound yet, so no action should be needed. + + // References should not have been bound yet, so no action needed } else { globalScope.defineImplicitVariable(name, libVariable); } From 88e6818350e6a04f852482469a89e0d3c51b1391 Mon Sep 17 00:00:00 2001 From: David Michon Date: Mon, 30 Dec 2024 20:17:13 +0000 Subject: [PATCH 3/3] test(scope-manager): Add more lib tests --- packages/scope-manager/tests/lib.test.ts | 107 ++++++++++++++++++++--- 1 file changed, 94 insertions(+), 13 deletions(-) diff --git a/packages/scope-manager/tests/lib.test.ts b/packages/scope-manager/tests/lib.test.ts index 82f4a29f3563..1774374094cf 100644 --- a/packages/scope-manager/tests/lib.test.ts +++ b/packages/scope-manager/tests/lib.test.ts @@ -11,29 +11,110 @@ describe('implicit lib definitions', () => { expect(variables).toHaveLength(1); }); - it('should define an implicit variable if there is a reference', () => { + it('should define an implicit variable if there is a value reference', () => { const { scopeManager } = parseAndAnalyze('new ArrayBuffer();', { lib: ['es2015'], }); const variables = scopeManager.variables; - expect( - variables.some( - v => v instanceof ImplicitLibVariable && v.name === 'ArrayBuffer', - ), - ).toEqual(true); + const arrayBufferVariables = variables.filter( + v => v.name === 'ArrayBuffer', + ); + expect(arrayBufferVariables).toHaveLength(1); + expect(arrayBufferVariables[0]).toBeInstanceOf(ImplicitLibVariable); }); - it('should define an implicit variable if there is a collision', () => { - const { scopeManager } = parseAndAnalyze('var Symbol = {};', { + it('should define an implicit variable if there is a type reference', () => { + const { scopeManager } = parseAndAnalyze('type T = ArrayBuffer;', { lib: ['es2015'], }); const variables = scopeManager.variables; - expect( - variables.some( - v => v instanceof ImplicitLibVariable && v.name === 'Symbol', - ), - ).toEqual(true); + const arrayBufferVariables = variables.filter( + v => v.name === 'ArrayBuffer', + ); + expect(arrayBufferVariables).toHaveLength(1); + expect(arrayBufferVariables[0]).toBeInstanceOf(ImplicitLibVariable); + }); + + it('should define an implicit variable if there is a nested value reference', () => { + const { scopeManager } = parseAndAnalyze( + 'var f = () => new ArrayBuffer();', + { + lib: ['es2015'], + }, + ); + + const variables = scopeManager.variables; + const arrayBufferVariables = variables.filter( + v => v.name === 'ArrayBuffer', + ); + expect(arrayBufferVariables).toHaveLength(1); + expect(arrayBufferVariables[0]).toBeInstanceOf(ImplicitLibVariable); + }); + + it('should define an implicit variable if there is a nested type reference', () => { + const { scopeManager } = parseAndAnalyze( + 'var f = (): T => undefined as T;', + { + lib: ['es2015'], + }, + ); + + const variables = scopeManager.variables; + const arrayBufferVariables = variables.filter( + v => v.name === 'ArrayBuffer', + ); + expect(arrayBufferVariables).toHaveLength(1); + expect(arrayBufferVariables[0]).toBeInstanceOf(ImplicitLibVariable); + }); + + it('should define an implicit variable if there is a value collision', () => { + const { scopeManager } = parseAndAnalyze('var Symbol = 1;', { + lib: ['es2015'], + }); + + const variables = scopeManager.variables; + const symbolVariables = variables.filter(v => v.name === 'Symbol'); + expect(symbolVariables).toHaveLength(1); + expect(symbolVariables[0]).toBeInstanceOf(ImplicitLibVariable); + }); + + it('should define an implicit variable if there is a type collision', () => { + const { scopeManager } = parseAndAnalyze('type Symbol = 1;', { + lib: ['es2015'], + }); + + const variables = scopeManager.variables; + const symbolVariables = variables.filter(v => v.name === 'Symbol'); + expect(symbolVariables).toHaveLength(1); + expect(symbolVariables[0]).toBeInstanceOf(ImplicitLibVariable); + }); + + it('should define an implicit variable if there is a nested value collision', () => { + const { scopeManager } = parseAndAnalyze('var f = (Symbol) => Symbol;', { + lib: ['es2015'], + }); + + const variables = scopeManager.variables; + const symbolVariables = variables.filter(v => v.name === 'Symbol'); + expect(symbolVariables).toHaveLength(2); + expect(symbolVariables.some(v => v instanceof ImplicitLibVariable)).toBe( + true, + ); + expect(symbolVariables.some(v => !(v instanceof ImplicitLibVariable))).toBe( + true, + ); + }); + + it('should define an implicit variable if there is a nested type collision', () => { + const { scopeManager } = parseAndAnalyze('var f = (a: Symbol) => a;', { + lib: ['es2015'], + }); + + const variables = scopeManager.variables; + const symbolVariables = variables.filter(v => v.name === 'Symbol'); + expect(symbolVariables).toHaveLength(1); + expect(symbolVariables[0]).toBeInstanceOf(ImplicitLibVariable); }); });