From 7117fcc557bb99dcfc7ea6daf8290b213029b040 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Sat, 2 Mar 2024 20:41:10 -0800 Subject: [PATCH 1/3] WIP --- nx.json | 1 + .../src/util/collectUnusedVariables.ts | 44 ++++++- .../no-unused-vars/no-unused-vars.test.ts | 107 ++++++++++++++++++ 3 files changed, 151 insertions(+), 1 deletion(-) diff --git a/nx.json b/nx.json index f18f497922fe..2c9781390b3e 100644 --- a/nx.json +++ b/nx.json @@ -1,5 +1,6 @@ { "$schema": "./node_modules/nx/schemas/nx-schema.json", + "neverConnectToCloud": true, "nxCloudAccessToken": "YjIzMmMxMWItMjhiMS00NWY2LTk1NWYtYWU3YWQ0YjE4YjBlfHJlYWQ=", "release": { "changelog": { diff --git a/packages/eslint-plugin/src/util/collectUnusedVariables.ts b/packages/eslint-plugin/src/util/collectUnusedVariables.ts index dbe749e62900..2617caf69019 100644 --- a/packages/eslint-plugin/src/util/collectUnusedVariables.ts +++ b/packages/eslint-plugin/src/util/collectUnusedVariables.ts @@ -1,6 +1,7 @@ import { ImplicitLibVariable, ScopeType, + Variable, Visitor, } from '@typescript-eslint/scope-manager'; import type { TSESTree } from '@typescript-eslint/utils'; @@ -10,6 +11,7 @@ import { ESLintUtils, TSESLint, } from '@typescript-eslint/utils'; +import { isDefinitionFile } from './misc'; class UnusedVarsVisitor< MessageIds extends string, @@ -22,6 +24,7 @@ class UnusedVarsVisitor< readonly #scopeManager: TSESLint.Scope.ScopeManager; // readonly #unusedVariables = new Set(); + readonly #isDefinitionFile: boolean; private constructor(context: TSESLint.RuleContext) { super({ @@ -32,6 +35,8 @@ class UnusedVarsVisitor< context.sourceCode.scopeManager, 'Missing required scope manager', ); + + this.#isDefinitionFile = isDefinitionFile(context.filename); } public static collectUnusedVariables< @@ -60,7 +65,13 @@ class UnusedVarsVisitor< scope: TSESLint.Scope.Scope, unusedVariables = new Set(), ): ReadonlySet { - for (const variable of scope.variables) { + const implicitlyExported = allVariablesImplicitlyExported( + scope, + this.#isDefinitionFile, + ); + + // TODO: do this better than the ternary + for (const variable of implicitlyExported ? [] : scope.variables) { if ( // skip function expression names, scope.functionExpressionScope || @@ -438,6 +449,37 @@ function isExported(variable: TSESLint.Scope.Variable): boolean { }); } +function allVariablesImplicitlyExported( + scope: TSESLint.Scope.Scope, + isDefinitionFile: boolean, +): boolean { + // TODO: does this also happen in ambient module declarations? + if (!isDefinitionFile || scope.type !== ScopeType.tsModule) { + return false; + } + + function isExportImportEquals(variable: Variable): boolean { + for (const def of variable.defs) { + if ( + def.type === TSESLint.Scope.DefinitionType.ImportBinding && + def.node.type === AST_NODE_TYPES.TSImportEqualsDeclaration && + def.node.parent.type === AST_NODE_TYPES.ExportNamedDeclaration + ) { + return true; + } + } + return false; + } + + for (const variable of scope.variables) { + if (isExported(variable) && !isExportImportEquals(variable)) { + return false; + } + } + + return true; +} + const LOGICAL_ASSIGNMENT_OPERATORS = new Set(['&&=', '||=', '??=']); /** diff --git a/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts b/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts index 04c218b7631e..fb6b5cf2b6eb 100644 --- a/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts @@ -1123,6 +1123,63 @@ export namespace Bar { export import TheFoo = Foo; } `, + ` +const foo = 1; +export = foo; + `, + ` +const Foo = 1; +interface Foo { + bar: string; +} +export = Foo; + `, + ` +interface Foo { + bar: string; +} +export = Foo; + `, + ` +type Foo = 1; +export = Foo; + `, + ` +type Foo = 1; +export = {} as Foo; + `, + ` +declare module 'foo' { + type Foo = 1; + export = Foo; +} + `, + ` +namespace Foo { + export const foo = 1; +} +export namespace Bar { + export import TheFoo = Foo; +} + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/2867 + { + code: ` +export namespace Foo { + const foo: 1234; +} + `, + filename: 'foo.d.ts', + }, + { + code: ` +export namespace Foo { + export import Bar = Something.Bar; + const foo: 1234; +} + `, + filename: 'foo.d.ts', + }, ], invalid: [ @@ -1950,5 +2007,55 @@ export namespace Bar { }, ], }, + // https://github.com/typescript-eslint/typescript-eslint/issues/2867 + { + code: ` +export namespace Foo { + const foo: 1234; + export const bar: string; + export namespace NS { + const baz: 1234; + } +} + `, + filename: 'foo.d.ts', + errors: [ + { + messageId: 'unusedVar', + line: 3, + column: 9, + data: { + varName: 'foo', + action: 'defined', + additional: '', + }, + }, + ], + }, + { + code: ` +export namespace Foo { + export import Bar = Something.Bar; + const foo: 1234; + export const bar: string; + export namespace NS { + const baz: 1234; + } +} + `, + filename: 'foo.d.ts', + errors: [ + { + messageId: 'unusedVar', + line: 4, + column: 9, + data: { + varName: 'foo', + action: 'defined', + additional: '', + }, + }, + ], + }, ], }); From 3d2fc3bf095be8edc5e16967fde098997ff9d591 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Sat, 2 Mar 2024 21:58:48 -0800 Subject: [PATCH 2/3] WIP --- .../src/util/collectUnusedVariables.ts | 13 +++++-- .../no-unused-vars/no-unused-vars.test.ts | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/util/collectUnusedVariables.ts b/packages/eslint-plugin/src/util/collectUnusedVariables.ts index 2617caf69019..cd215124d64c 100644 --- a/packages/eslint-plugin/src/util/collectUnusedVariables.ts +++ b/packages/eslint-plugin/src/util/collectUnusedVariables.ts @@ -70,7 +70,6 @@ class UnusedVarsVisitor< this.#isDefinitionFile, ); - // TODO: do this better than the ternary for (const variable of implicitlyExported ? [] : scope.variables) { if ( // skip function expression names, @@ -454,10 +453,20 @@ function allVariablesImplicitlyExported( isDefinitionFile: boolean, ): boolean { // TODO: does this also happen in ambient module declarations? - if (!isDefinitionFile || scope.type !== ScopeType.tsModule) { + if ( + !isDefinitionFile || + !( + scope.type === ScopeType.tsModule || + scope.type === ScopeType.module || + scope.type === ScopeType.global + ) + ) { return false; } + // TODO: test modules, globals + // TODO: look for `export {}` + function isExportImportEquals(variable: Variable): boolean { for (const def of variable.defs) { if ( diff --git a/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts b/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts index fb6b5cf2b6eb..8827b82f30e2 100644 --- a/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts @@ -1176,6 +1176,15 @@ export namespace Foo { export namespace Foo { export import Bar = Something.Bar; const foo: 1234; +} + `, + filename: 'foo.d.ts', + }, + { + code: ` +declare module 'foo' { + export import Bar = Something.Bar; + const foo: 1234; } `, filename: 'foo.d.ts', @@ -2041,6 +2050,31 @@ export namespace Foo { export namespace NS { const baz: 1234; } +} + `, + filename: 'foo.d.ts', + errors: [ + { + messageId: 'unusedVar', + line: 4, + column: 9, + data: { + varName: 'foo', + action: 'defined', + additional: '', + }, + }, + ], + }, + { + code: ` +declare module 'foo' { + export import Bar = Something.Bar; + const foo: 1234; + export const bar: string; + export namespace NS { + const baz: 1234; + } } `, filename: 'foo.d.ts', From f9f18c54c8315294588e328848dcb15d42eec81c Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 6 Mar 2024 15:08:56 -0800 Subject: [PATCH 3/3] revert nx --- nx.json | 1 - 1 file changed, 1 deletion(-) diff --git a/nx.json b/nx.json index 2c9781390b3e..f18f497922fe 100644 --- a/nx.json +++ b/nx.json @@ -1,6 +1,5 @@ { "$schema": "./node_modules/nx/schemas/nx-schema.json", - "neverConnectToCloud": true, "nxCloudAccessToken": "YjIzMmMxMWItMjhiMS00NWY2LTk1NWYtYWU3YWQ0YjE4YjBlfHJlYWQ=", "release": { "changelog": {